diff --git a/plugins/providers/virtualbox/cap/cleanup_disks.rb b/plugins/providers/virtualbox/cap/cleanup_disks.rb index deb4073d1..c6c0ce466 100644 --- a/plugins/providers/virtualbox/cap/cleanup_disks.rb +++ b/plugins/providers/virtualbox/cap/cleanup_disks.rb @@ -44,7 +44,7 @@ module VagrantPlugins LOGGER.warn("Disk '#{d["name"]}' not attached to guest, but still exists.") else # TODO: write test for sata controller with another name - machine.provider.driver.remove_disk(disk_info[:port], disk_info[:device]) + machine.provider.driver.remove_disk(disk_info[:port], disk_info[:device], controller.name) end machine.provider.driver.close_medium(d["uuid"]) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 148dd880b..3413ee0ad 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -263,9 +263,10 @@ module VagrantPlugins begin # Danger Zone + controller = machine.provider.driver.get_controller('SATA') # remove and close original volume - machine.provider.driver.remove_disk(disk_info[:port], disk_info[:device]) + machine.provider.driver.remove_disk(disk_info[:port], disk_info[:device], controller.name) # Create a backup of the original disk if something goes wrong LOGGER.warn("Making a backup of the original disk at #{defined_disk["Location"]}") FileUtils.mv(defined_disk["Location"], backup_disk_location) diff --git a/plugins/providers/virtualbox/driver/version_5_0.rb b/plugins/providers/virtualbox/driver/version_5_0.rb index 207c989c5..c994669c1 100644 --- a/plugins/providers/virtualbox/driver/version_5_0.rb +++ b/plugins/providers/virtualbox/driver/version_5_0.rb @@ -28,17 +28,16 @@ module VagrantPlugins # @param [String] type - type of disk to attach # @param [Hash] opts - additional options def attach_disk(port, device, file, type="hdd", **opts) - # Maybe only support SATA Controller for `:disk`??? if type == "hdd" - controller = "SATA Controller" + controller = get_controller('SATA') else - controller = "IDE Controller" + controller = get_controller('IDE') end comment = "This disk is managed externally by Vagrant. Removing or adjusting settings could potentially cause issues with Vagrant." execute('storageattach', @uuid, - '--storagectl', controller, + '--storagectl', controller.name, '--port', port.to_s, '--device', device.to_s, '--type', type, @@ -234,10 +233,10 @@ module VagrantPlugins # @param [String] port - port on device to attach disk to # @param [String] device - device on controller for disk - # @param [Hash] opts - additional options - def remove_disk(port, device, controller="SATA Controller") + # @param [String] controller_name - controller name to remove disk from + def remove_disk(port, device, controller_name) execute('storageattach', @uuid, - '--storagectl', controller, + '--storagectl', controller_name, '--port', port.to_s, '--device', device.to_s, '--medium', "none") 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 0094c5c52..a505a1d1b 100644 --- a/test/unit/plugins/providers/virtualbox/cap/cleanup_disks_test.rb +++ b/test/unit/plugins/providers/virtualbox/cap/cleanup_disks_test.rb @@ -88,7 +88,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::CleanupDisks do with("67890"). and_return(device_info) - expect(driver).to receive(:remove_disk).with("1", "0").and_return(true) + expect(driver).to receive(:remove_disk).with("1", "0", sata_controller.name).and_return(true) expect(driver).to receive(:close_medium).with("67890").and_return(true) subject.handle_cleanup_disk(machine, defined_disks, disk_meta_file[: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 ce1128f1e..9cc5fa682 100644 --- a/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb +++ b/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb @@ -361,7 +361,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do expect(driver).to receive(:resize_disk).with(vdi_disk_file, disk_config.size.to_i). and_return(true) - expect(driver).to receive(:remove_disk).with(attach_info[:port], attach_info[:device]). + expect(driver).to receive(:remove_disk).with(attach_info[:port], attach_info[:device], sata_controller.name). and_return(true) expect(driver).to receive(:close_medium).with("12345") @@ -393,7 +393,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do expect(driver).to receive(:resize_disk).with(vdi_disk_file, disk_config.size.to_i). and_return(true) - expect(driver).to receive(:remove_disk).with(attach_info[:port], attach_info[:device]). + expect(driver).to receive(:remove_disk).with(attach_info[:port], attach_info[:device], sata_controller.name). and_return(true) expect(driver).to receive(:close_medium).with("12345") diff --git a/test/unit/plugins/providers/virtualbox/driver/version_5_0_test.rb b/test/unit/plugins/providers/virtualbox/driver/version_5_0_test.rb index b6b23e0d2..f441afbf6 100644 --- a/test/unit/plugins/providers/virtualbox/driver/version_5_0_test.rb +++ b/test/unit/plugins/providers/virtualbox/driver/version_5_0_test.rb @@ -92,6 +92,12 @@ OUTPUT end describe "#attach_disk" do + let(:controller) { double("controller", name: "IDE Controller", storage_bus: "IDE") } + + before do + allow(subject).to receive(:get_controller).with(controller.storage_bus).and_return(controller) + end + it "attaches a dvddrive device to the IDE controller" do expect(subject).to receive(:execute) do |*args| storagectl = args[args.index("--storagectl") + 1] @@ -102,15 +108,7 @@ OUTPUT end describe "#remove_disk" do - it "removes a disk from the SATA Controller by default" do - expect(subject).to receive(:execute) do |*args| - storagectl = args[args.index("--storagectl") + 1] - expect(storagectl).to eq("SATA Controller") - end - subject.remove_disk(anything, anything) - end - - it "can remove a disk from the specified controller" do + it "removes a disk from the specified controller" do expect(subject).to receive(:execute) do |*args| storagectl = args[args.index("--storagectl") + 1] expect(storagectl).to eq("IDE Controller")