diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index 910f64098..619803022 100644 --- a/lib/vagrant/errors.rb +++ b/lib/vagrant/errors.rb @@ -932,6 +932,10 @@ module Vagrant error_key(:virtualbox_disks_defined_exceed_limit) end + class VirtualBoxDisksControllerNotFound < VagrantError + error_key(:virtualbox_disks_controller_not_found) + end + class VirtualBoxGuestPropertyNotFound < VagrantError error_key(:virtualbox_guest_property_not_found) end diff --git a/plugins/providers/virtualbox/cap/cleanup_disks.rb b/plugins/providers/virtualbox/cap/cleanup_disks.rb index c02820d7b..9481b5dd9 100644 --- a/plugins/providers/virtualbox/cap/cleanup_disks.rb +++ b/plugins/providers/virtualbox/cap/cleanup_disks.rb @@ -29,7 +29,7 @@ module VagrantPlugins controller = machine.provider.driver.storage_controllers.detect { |c| c.sata_controller? } primary_disk = controller.attachments.detect { |a| a[:port] == "0" && a[:device] == "0" }[:uuid] - unless disk_meta.nil? + if disk_meta disk_meta.each do |d| dsk = defined_disks.select { |dk| dk.name == d["name"] } if !dsk.empty? || d["uuid"] == primary_disk diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 8cfa9cd96..a225ec092 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -18,15 +18,23 @@ module VagrantPlugins return {} if !Vagrant::Util::Experimental.feature_enabled?("disks") disks_defined = defined_disks.select { |d| d.type == :disk } - controller = machine.provider.driver.storage_controllers.detect { |c| c.sata_controller? } - if disks_defined.any? && controller && disks_defined.size > controller.maxportcount - raise Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit + if disks_defined.any? + controller = machine.provider.driver.storage_controllers.detect { |c| c.sata_controller? } + if controller.nil? + raise Vagrant::Errors::VirtualBoxDisksControllerNotFound, disk_type: ':disk', controller_type: 'SATA' + elsif disks_defined.size > controller.maxportcount + raise Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit + end end dvds_defined = defined_disks.select { |d| d.type == :dvd } - controller = machine.provider.driver.storage_controllers.detect { |c| c.ide_controller? } - if dvds_defined.any? && controller && dvds_defined.size > controller.maxportcount - raise Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit + if dvds_defined.any? + controller = machine.provider.driver.storage_controllers.detect { |c| c.ide_controller? } + if controller.nil? + raise Vagrant::Errors::VirtualBoxDisksControllerNotFound, disk_type: ':dvd', controller_type: 'IDE' + elsif dvds_defined.size > controller.maxportcount + raise Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit + end end machine.ui.info(I18n.t("vagrant.cap.configure_disks.start")) @@ -99,7 +107,8 @@ module VagrantPlugins disk_info = machine.provider.driver.get_port_and_device(current_disk["UUID"]) if disk_info.empty? LOGGER.warn("Disk '#{disk.name}' is not connected to guest '#{machine.name}', Vagrant will attempt to connect disk to guest") - dsk_info = get_next_port(machine) + controller = machine.provider.driver.storage_controllers.detect { |c| c.sata_controller? } + dsk_info = get_next_port(machine, controller.name) machine.provider.driver.attach_disk(dsk_info[:port], dsk_info[:device], current_disk["Location"]) @@ -195,9 +204,9 @@ module VagrantPlugins # device = disk_info[3] # # @param [Vagrant::Machine] machine - # @param [String] controller name (defaults to "SATA Controller") + # @param [String] controller name # @return [Hash] dsk_info - The next available port and device on a given controller - def self.get_next_port(machine, controller_name="SATA Controller") + def self.get_next_port(machine, controller_name) controller = machine.provider.driver.storage_controllers.detect { |c| c.name == controller_name } # TODO: definitely need an error for this dsk_info = {} diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 071b3fa94..d23c2935f 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -1678,6 +1678,10 @@ en: 4.2.14 contains a critical bug which prevents it from working with Vagrant. VirtualBox 4.2.16+ fixes this problem. Please upgrade VirtualBox. + virtualbox_disks_controller_not_found: |- + The current disk configuration requires a controller of type '%{controller_type}'. + Please add the proper controller to your guest using provider-specific + customizations, or remove the '%{disk_type}' disk configurations. virtualbox_disks_defined_exceed_limit: |- VirtualBox only allows up to 30 disks to be attached to a single guest using the SATA Controller, including the primray disk. diff --git a/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb b/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb index 1da64890b..6ea05d8b3 100644 --- a/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb +++ b/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb @@ -31,13 +31,21 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do "SATA Controller-ImageUUID-0-0" => "12345", "SATA Controller-ImageUUID-1-0" => "67890"} } - let(:controller) { double("controller", name: "SATA Controller", maxportcount: 30, sata_controller?: true, ide_controller?: false) } + let(:sata_controller) { double("sata_controller", + name: "SATA Controller", + maxportcount: 30, + sata_controller?: true, + ide_controller?: false) } + + let(:ide_controller) { double("ide_controller", + name: "IDE Controller", + maxportcount: 2, + ide_controller?: true, + sata_controller?: false) } let(:attachments) { [{port: "0", device: "0", uuid: "12345"}, {port: "1", device: "0", uuid: "67890"}]} - let(:storage_controllers) { [ controller ] } - let(:defined_disks) { [double("disk", name: "vagrant_primary", size: "5GB", primary: true, type: :disk), double("disk", name: "disk-0", size: "5GB", primary: false, type: :disk), double("disk", name: "disk-1", size: "5GB", primary: false, type: :disk), @@ -76,8 +84,8 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do before do allow(Vagrant::Util::Experimental).to receive(:feature_enabled?).and_return(true) allow(driver).to receive(:show_vm_info).and_return(vm_info) - allow(driver).to receive(:storage_controllers).and_return(storage_controllers) - allow(controller).to receive(:attachments).and_return(attachments) + allow(sata_controller).to receive(:attachments).and_return(attachments) + allow(driver).to receive(:storage_controllers).and_return([sata_controller]) end describe "#configure_disks" do @@ -105,7 +113,22 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do end end + context "missing SATA controller" do + before do + allow(driver).to receive(:storage_controllers).and_return([]) + end + + it "raises an error" do + expect { subject.configure_disks(machine, defined_disks) }. + to raise_error(Vagrant::Errors::VirtualBoxDisksControllerNotFound) + end + end + context "with dvd type" do + before do + allow(driver).to receive(:storage_controllers).and_return([ide_controller]) + end + let(:defined_disks) { [double("dvd", type: :dvd)] } let(:dvd_data) { {uuid: "1234", name: "dvd"} } @@ -114,6 +137,17 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do expect(subject).to receive(:handle_configure_dvd).and_return(dvd_data) subject.configure_disks(machine, defined_disks) end + + context "missing IDE controller" do + before do + allow(driver).to receive(:storage_controllers).and_return([]) + end + + it "raises an error" do + expect { subject.configure_disks(machine, defined_disks) }. + to raise_error(Vagrant::Errors::VirtualBoxDisksControllerNotFound) + end + end end end @@ -287,18 +321,22 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do describe ".get_next_port" do it "determines the next available port and device to use" do - dsk_info = subject.get_next_port(machine) + dsk_info = subject.get_next_port(machine, sata_controller.name) expect(dsk_info[:port]).to eq("2") expect(dsk_info[:device]).to eq("0") end - context "with an IDE controller" do - let(:controller) { double("controller", name: "IDE Controller", maxportcount: 2, sata_controller?: false, ide_controller?: true) } + context "guest with an IDE controller" do let(:attachments) { [{port: "0", device: "0", uuid: "12345"}, {port: "0", device: "1", uuid: "67890"}] } + before do + allow(ide_controller).to receive(:attachments).and_return(attachments) + allow(driver).to receive(:storage_controllers).and_return([ide_controller]) + end + it "determines the next available port and device to use" do - dsk_info = subject.get_next_port(machine, "IDE Controller") + dsk_info = subject.get_next_port(machine, ide_controller.name) expect(dsk_info[:port]).to eq("1") expect(dsk_info[:device]).to eq("0") end @@ -310,7 +348,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do {port: "1", device: "1", uuid: "44444"}] } it "raises an error" do - expect { subject.get_next_port(machine, "IDE Controller") } + expect { subject.get_next_port(machine, ide_controller.name) } .to raise_error(Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit) end end @@ -412,13 +450,13 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do describe ".handle_configure_dvd" do let(:dvd_config) { double("dvd", file: "/tmp/untitled.iso", name: "dvd1") } - let(:controller) { double("controller", name: "IDE Controller", maxportcount: 2, sata_controller?: false, ide_controller?: true) } before do - allow(subject).to receive(:get_next_port).with(machine, "IDE Controller").and_return({device: "0", port: "0"}) - allow(controller).to receive(:attachments).and_return( + allow(subject).to receive(:get_next_port).with(machine, ide_controller.name).and_return({device: "0", port: "0"}) + allow(ide_controller).to receive(:attachments).and_return( [port: "0", device: "0", uuid: "12345"] ) + allow(driver).to receive(:storage_controllers).and_return([ide_controller]) end it "returns the UUID of the newly-attached dvd" do