Require controller name for #remove_disk

Also use #get_controller methods when attaching a disk.

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"])

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)

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")

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])

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")
 

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")