diff --git a/plugins/providers/virtualbox/cap/cleanup_disks.rb b/plugins/providers/virtualbox/cap/cleanup_disks.rb index 9481b5dd9..deb4073d1 100644 --- a/plugins/providers/virtualbox/cap/cleanup_disks.rb +++ b/plugins/providers/virtualbox/cap/cleanup_disks.rb @@ -26,7 +26,7 @@ module VagrantPlugins # @param [VagrantPlugins::Kernel_V2::VagrantConfigDisk] defined_disks # @param [Hash] disk_meta - A hash of all the previously defined disks from the last configure_disk action def self.handle_cleanup_disk(machine, defined_disks, disk_meta) - controller = machine.provider.driver.storage_controllers.detect { |c| c.sata_controller? } + controller = machine.provider.driver.get_controller('SATA') primary_disk = controller.attachments.detect { |a| a[:port] == "0" && a[:device] == "0" }[:uuid] if disk_meta @@ -57,9 +57,8 @@ module VagrantPlugins # @param [VagrantPlugins::Kernel_V2::VagrantConfigDisk] defined_dvds # @param [Hash] dvd_meta - A hash of all the previously defined dvds from the last configure_disk action def self.handle_cleanup_dvd(machine, defined_dvds, dvd_meta) - controller = machine.provider.driver.storage_controllers.detect { |c| c.ide_controller? } - - unless dvd_meta.nil? + if dvd_meta + controller = machine.provider.driver.get_controller('IDE') dvd_meta.each do |d| dsk = defined_dvds.select { |dk| dk.name == d["name"] } if !dsk.empty? diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 1846a25ef..148dd880b 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -104,7 +104,7 @@ module VagrantPlugins if disk_info.empty? LOGGER.warn("Disk '#{disk.name}' is not connected to guest '#{machine.name}', Vagrant will attempt to connect disk to guest") controller = machine.provider.driver.get_controller('SATA') - dsk_info = get_next_port(machine, controller.name) + dsk_info = get_next_port(machine, controller) machine.provider.driver.attach_disk(dsk_info[:port], dsk_info[:device], current_disk["Location"]) @@ -126,7 +126,7 @@ module VagrantPlugins def self.handle_configure_dvd(machine, dvd) controller = machine.provider.driver.get_controller('IDE') - disk_info = get_next_port(machine, controller.name) + disk_info = get_next_port(machine, controller) machine.provider.driver.attach_disk(disk_info[:port], disk_info[:device], dvd.file, "dvddrive") # Refresh the controller information @@ -181,7 +181,8 @@ module VagrantPlugins disk_var = machine.provider.driver.create_disk(disk_file, disk_config.size, disk_ext.upcase) disk_metadata = {uuid: disk_var.split(':').last.strip, name: disk_config.name} - dsk_controller_info = get_next_port(machine) + controller = machine.provider.driver.get_controller('SATA') + dsk_controller_info = get_next_port(machine, controller) machine.provider.driver.attach_disk(dsk_controller_info[:port], dsk_controller_info[:device], disk_file) disk_metadata @@ -200,11 +201,9 @@ module VagrantPlugins # device = disk_info[3] # # @param [Vagrant::Machine] machine - # @param [String] controller name + # @param [VagrantPlugins::ProviderVirtualBox::Model::StorageController] controller # @return [Hash] dsk_info - The next available port and device on a given 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 + def self.get_next_port(machine, controller) dsk_info = {} if controller.storage_bus == 'SATA' diff --git a/plugins/providers/virtualbox/driver/meta.rb b/plugins/providers/virtualbox/driver/meta.rb index f6f1ad161..ea9d4e720 100644 --- a/plugins/providers/virtualbox/driver/meta.rb +++ b/plugins/providers/virtualbox/driver/meta.rb @@ -116,6 +116,7 @@ module VagrantPlugins :execute_command, :export, :forward_ports, + :get_controller, :get_port_and_device, :halt, :import, diff --git a/plugins/providers/virtualbox/driver/version_5_0.rb b/plugins/providers/virtualbox/driver/version_5_0.rb index e2a753b4c..207c989c5 100644 --- a/plugins/providers/virtualbox/driver/version_5_0.rb +++ b/plugins/providers/virtualbox/driver/version_5_0.rb @@ -960,7 +960,6 @@ module VagrantPlugins end end - # Get the controller that uses the specified storage bus. # # A VirtualBoxDisksControllerNotFound error is raised if a compatible diff --git a/test/unit/plugins/providers/virtualbox/cap/cleanup_disks_test.rb b/test/unit/plugins/providers/virtualbox/cap/cleanup_disks_test.rb index e97ebba84..0094c5c52 100644 --- a/test/unit/plugins/providers/virtualbox/cap/cleanup_disks_test.rb +++ b/test/unit/plugins/providers/virtualbox/cap/cleanup_disks_test.rb @@ -30,10 +30,8 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::CleanupDisks do let(:disk_meta_file) { {disk: [], floppy: [], dvd: []} } let(:defined_disks) { {} } - let(:vm_info) { {"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("controller", name: "SATA Controller", storage_bus: "SATA", maxportcount: 30) } + let(:ide_controller) { double("controller", name: "IDE Controller", storage_bus: "IDE", maxportcount: 2) } let(:attachments) { [{port: "0", device: "0", uuid: "12345"}, {port: "1", device: "0", uuid: "67890"}]} @@ -42,9 +40,11 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::CleanupDisks 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(:get_controller).with("IDE").and_return(ide_controller) + allow(driver).to receive(:get_controller).with("SATA").and_return(sata_controller) + allow(driver).to receive(:storage_controllers).and_return([ide_controller, sata_controller]) end describe "#cleanup_disks" do @@ -108,14 +108,15 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::CleanupDisks do end describe "#handle_cleanup_dvd" do - let(:vm_info) { {"IDE Controller-ImageUUID-0-0" => "1234" } } - - let(:controller) { double("controller", name: "IDE Controller", maxportcount: 2, sata_controller?: false, ide_controller?: true) } let(:attachments) { [{port: "0", device: "0", uuid: "1234"}] } let(:disk_meta_file) { {dvd: [{"uuid" => "1234", "name" => "iso"}]} } let(:defined_disks) { [] } + before do + allow(ide_controller).to receive(:attachments).and_return(attachments) + end + it "removes the medium from guest" do expect(driver).to receive(:remove_disk).with("0", "0", "IDE Controller").and_return(true) 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 68ca5be4e..ce1128f1e 100644 --- a/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb +++ b/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb @@ -25,12 +25,6 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do double(:state) end - let(:vm_info) { {"storagecontrollername0" => "SATA Controller", - "storagecontrollertype0" => "IntelAhci", - "storagecontrollermaxportcount0" => "30", - "SATA Controller-ImageUUID-0-0" => "12345", - "SATA Controller-ImageUUID-1-0" => "67890"} } - let(:sata_controller) { double("controller", name: "SATA Controller", storage_bus: "SATA", maxportcount: 30) } let(:ide_controller) { double("controller", name: "IDE Controller", storage_bus: "IDE", maxportcount: 2) } @@ -74,8 +68,6 @@ 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(sata_controller).to receive(:attachments).and_return(attachments) allow(driver).to receive(:get_controller).with("IDE").and_return(ide_controller) @@ -301,7 +293,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do expect(driver).to receive(:create_disk). with(disk_file, disk_config.size, "VDI").and_return(disk_data) - expect(subject).to receive(:get_next_port).with(machine). + expect(subject).to receive(:get_next_port).with(machine, sata_controller). and_return(port_and_device) expect(driver).to receive(:attach_disk).with(port_and_device[:port], @@ -314,7 +306,7 @@ 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, sata_controller.name) + dsk_info = subject.get_next_port(machine, sata_controller) expect(dsk_info[:port]).to eq("2") expect(dsk_info[:device]).to eq("0") end @@ -328,7 +320,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do end it "determines the next available port and device to use" do - dsk_info = subject.get_next_port(machine, ide_controller.name) + dsk_info = subject.get_next_port(machine, ide_controller) expect(dsk_info[:port]).to eq("1") expect(dsk_info[:device]).to eq("0") end @@ -340,7 +332,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.name) } + expect { subject.get_next_port(machine, ide_controller) } .to raise_error(Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit) end end @@ -444,7 +436,8 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do let(:dvd_config) { double("dvd", file: "/tmp/untitled.iso", name: "dvd1") } before do - allow(subject).to receive(:get_next_port).with(machine, ide_controller.name).and_return({device: "0", port: "0"}) + allow(subject).to receive(:get_next_port).with(machine, ide_controller). + and_return({device: "0", port: "0"}) allow(ide_controller).to receive(:attachments).and_return( [port: "0", device: "0", uuid: "12345"] )