diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index df6aa54e2..bef7053aa 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -9,8 +9,8 @@ module VagrantPlugins module ConfigureDisks LOGGER = Log4r::Logger.new("vagrant::plugins::virtualbox::configure_disks") - # The max amount of disks that can be attached to a single device in a controller - MAX_DISK_NUMBER = 30.freeze + SATA_CONTROLLER_TYPES = ["IntelAhci"].freeze + IDE_CONTROLLER_TYPES = ["PIIX4", "PIIX3", "ICH6"].freeze # @param [Vagrant::Machine] machine # @param [VagrantPlugins::Kernel_V2::VagrantConfigDisk] defined_disks @@ -20,8 +20,15 @@ module VagrantPlugins return {} if !Vagrant::Util::Experimental.feature_enabled?("disks") - if defined_disks.size > MAX_DISK_NUMBER - # you can only attach up to 30 disks per controller, INCLUDING the primary disk + disks_defined = defined_disks.select { |d| d.type == :disk } + controller = sata_controller(machine) + if disks_defined.any? && controller && disks_defined.size > controller[:maxportcount] + raise Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit + end + + dvds_defined = defined_disks.select { |d| d.type == :dvd } + controller = ide_controller(machine) + if dvds_defined.any? && controller && dvds_defined.size > controller[:maxportcount] raise Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit end @@ -59,8 +66,9 @@ module VagrantPlugins # Ensure we grab the proper primary disk # We can't rely on the order of `all_disks`, as they will not # always come in port order, but primary is always Port 0 Device 0. - vm_info = machine.provider.driver.show_vm_info - primary_uuid = vm_info["SATA Controller-ImageUUID-0-0"] + controller = sata_controller(machine) + primary = controller[:attachments].detect { |a| a[:port] == "0" && a[:device] == "0" } + primary_uuid = primary[:uuid] current_disk = all_disks.select { |d| d["UUID"] == primary_uuid }.first else @@ -108,6 +116,14 @@ module VagrantPlugins disk_metadata end + def self.sata_controller(machine) + machine.provider.driver.storage_controllers.detect { |c| SATA_CONTROLLER_TYPES.include?(c[:type]) } + end + + def self.ide_controller(machine) + machine.provider.driver.storage_controllers.detect { |c| IDE_CONTROLLER_TYPES.include?(c[:type]) } + end + # Helper method to get the UUID of a specific controller attachment # # @param [Vagrant::Machine] machine - the current machine @@ -125,26 +141,21 @@ module VagrantPlugins # @param [Config::Disk] dvd - the current disk to configure # @return [Hash] - dvd_metadata def self.handle_configure_dvd(machine, dvd) - controller = "IDE Controller" - vm_info = machine.provider.driver.show_vm_info + controller = ide_controller(machine) - # Check if this DVD file is already attached - (0..1).each do |port| - (0..1).each do |device| - if vm_info["#{controller}-#{port}-#{device}"] == dvd.file - LOGGER.warn("DVD '#{dvd.file}' is already connected to guest '#{machine.name}', skipping...") - return {uuid: vm_info["#{controller}-ImageUUID-#{port}-#{device}"], name: dvd.name} - end - end - end - - # New attachment - disk_info = get_next_port(machine, controller) + disk_info = get_next_port(machine, controller[:name]) machine.provider.driver.attach_disk(disk_info[:port], disk_info[:device], dvd.file, "dvddrive") - attachment_uuid = attachment(machine, controller, disk_info[:port], disk_info[:device]) + # Refresh the controller information + controller = ide_controller(machine) + attachment = controller[:attachments].detect { |c| c[:port] == disk_info[:port] && + c[:device] == disk_info[:device] } - {uuid: attachment_uuid, name: dvd.name} + if attachment + {uuid: attachment[:uuid], name: dvd.name} + else + {} + end end # Check to see if current disk is configured based on defined_disks @@ -208,29 +219,32 @@ module VagrantPlugins # @param [Vagrant::Machine] machine # @param [String] controller name (defaults to "SATA Controller") # @return [Hash] dsk_info - The next available port and device on a given controller - def self.get_next_port(machine, controller="SATA Controller") - vm_info = machine.provider.driver.show_vm_info + def self.get_next_port(machine, controller_name="SATA Controller") + controller = machine.provider.driver.storage_controllers.detect { |c| c[:name] == controller_name } + # definitely need an error for this dsk_info = {} - if controller == "SATA Controller" - disk_images = vm_info.select { |v| v.include?("ImageUUID") && v.include?(controller) } - used_ports = disk_images.keys.map { |k| k.split('-') }.map {|v| v[2].to_i} - next_available_port = ((0..(MAX_DISK_NUMBER-1)).to_a - used_ports).first + if controller[:type] == "IntelAhci" # SATA Controller + used_ports = controller[:attachments].map { |a| a[:port].to_i } + next_available_port = ((0..(controller[:maxportcount]-1)).to_a - used_ports).first dsk_info[:port] = next_available_port.to_s dsk_info[:device] = "0" else # IDE Controllers have primary/secondary devices, so find the first port # with an empty device - (0..1).each do |port| - break if dsk_info[:port] && dsk_info[:device] + (0..(controller[:maxportcount]-1)).each do |port| + # Skip this port if it's full + port_attachments = controller[:attachments].select { |a| a[:port] == port.to_s } + next if port_attachments.count == 2 - (0..1).each do |device| - if vm_info["#{controller}-ImageUUID-#{port}-#{device}"].to_s.empty? - dsk_info[:port] = port.to_s - dsk_info[:device] = device.to_s - break - end + dsk_info[:port] = port.to_s + + # Check for a free device + if port_attachments.detect { |a| a[:device] == "0" } + dsk_info[:device] = "1" + else + dsk_info[:device] = "0" end end end diff --git a/plugins/providers/virtualbox/driver/version_5_0.rb b/plugins/providers/virtualbox/driver/version_5_0.rb index 6de2552a8..25bf388e5 100644 --- a/plugins/providers/virtualbox/driver/version_5_0.rb +++ b/plugins/providers/virtualbox/driver/version_5_0.rb @@ -934,6 +934,35 @@ module VagrantPlugins destination end + # Helper method to get a list of storage controllers added to the + # current VM + def storage_controllers + vm_info = show_vm_info + count = vm_info.count { |key, value| key.match(/^storagecontrollername/) } + + (0..count - 1).map do |n| + # basic controller metadata + name = vm_info["storagecontrollername#{n}"] + type = vm_info["storagecontrollertype#{n}"] + maxportcount = vm_info["storagecontrollermaxportcount#{n}"].to_i + + # build attachments array + attachments = [] + vm_info.each do |k, v| + if /^#{name}-ImageUUID-(\d+)-(\d+)$/ =~ k + attachments << {port: $1.to_s, device: $2.to_s, uuid: v} + end + end + + { + name: name, + type: type, + maxportcount: maxportcount, + attachments: attachments + } + end + end + protected def valid_ip_address?(ip) 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 2f4317e1d..918846cf7 100644 --- a/test/unit/plugins/providers/virtualbox/cap/cleanup_disks_test.rb +++ b/test/unit/plugins/providers/virtualbox/cap/cleanup_disks_test.rb @@ -33,9 +33,13 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::CleanupDisks do let(:vm_info) { {"SATA Controller-ImageUUID-0-0" => "12345", "SATA Controller-ImageUUID-1-0" => "67890"} } + let(:storage_controllers) { + } + 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) end describe "#cleanup_disks" do 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 53390f9f0..0e250128b 100644 --- a/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb +++ b/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb @@ -25,9 +25,18 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do double(:state) end - let(:vm_info) { {"SATA Controller-ImageUUID-0-0" => "12345", + let(:vm_info) { {"storagecontrollername0" => "SATA Controller", + "storagecontrollertype0" => "IntelAhci", + "storagecontrollermaxportcount0" => "30", + "SATA Controller-ImageUUID-0-0" => "12345", "SATA Controller-ImageUUID-1-0" => "67890"} } + let(:storage_controllers) { [{name: "SATA Controller", + type: "IntelAhci", + maxportcount: 30, + attachments: [{port: "0", device: "0", uuid: "12345"}, + {port: "1", device: "0", uuid: "67890"}]}] } + 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), @@ -66,6 +75,7 @@ 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) end describe "#configure_disks" do @@ -85,7 +95,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do end context "with over the disk limit for a given device" do - let(:defined_disks) { (1..31).each { |i| double("disk-#{i}") }.to_a } + let(:defined_disks) { (1..31).map { |i| double("disk-#{i}", type: :disk) }.to_a } it "raises an exception if the disks defined exceed the limit for a SATA Controller" do expect{subject.configure_disks(machine, defined_disks)}. @@ -281,8 +291,11 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do end context "with an IDE controller" do - let(:vm_info) { {"IDE Controller-ImageUUID-0-0" => "00000aaaaa", - "IDE Controller-ImageUUID-0-1" => "11111bbbbb" } } + let(:storage_controllers) { [{name: "IDE Controller", + type: "PIIX4", + maxportcount: 2, + attachments: [{port: "0", device: "0", uuid: "12345"}, + {port: "0", device: "1", uuid: "67890"}]}] } it "determines the next available port and device to use" do dsk_info = subject.get_next_port(machine, "IDE Controller") @@ -292,10 +305,13 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do end context "with a full IDE controller" do - let(:vm_info) { {"IDE Controller-ImageUUID-0-0" => "00000aaaaa", - "IDE Controller-ImageUUID-0-1" => "11111bbbbb", - "IDE Controller-ImageUUID-1-0" => "22222ccccc", - "IDE Controller-ImageUUID-1-1" => "33333ddddd"} } + let(:storage_controllers) { [{name: "IDE Controller", + type: "PIIX4", + maxportcount: 2, + attachments: [{port: "0", device: "0", uuid: "11111"}, + {port: "0", device: "1", uuid: "22222"}, + {port: "1", device: "0", uuid: "33333"}, + {port: "1", device: "1", uuid: "44444"}]}] } it "raises an error" do expect { subject.get_next_port(machine, "IDE Controller") } @@ -399,9 +415,19 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do describe ".handle_configure_dvd" do let(:dvd_config) { double("dvd", file: "/tmp/untitled.iso", name: "dvd1") } + let(:storage_controllers) { [{name: "IDE Controller", + type: "PIIX4", + maxportcount: 2, + attachments: []}] } + + let(:single_attachment) { [{name: "IDE Controller", + type: "PIIX4", + maxportcount: 2, + attachments: [port: "0", device: "0", uuid: "12345"]}] } before do allow(subject).to receive(:get_next_port).with(machine, "IDE Controller").and_return({device: "0", port: "0"}) + allow(driver).to receive(:storage_controllers).and_return(storage_controllers) end it "attaches to the next available port and device" do @@ -411,20 +437,14 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do end it "returns the UUID of the newly-attached dvd" do - allow(subject).to receive(:attachment).with(machine, "IDE Controller", "0", "0").and_return("12345") + expect(driver).to receive(:storage_controllers).and_return( + storage_controllers, + single_attachment + ) expect(driver).to receive(:attach_disk).with("0", "0", "/tmp/untitled.iso", "dvddrive") disk_meta = subject.handle_configure_dvd(machine, dvd_config) expect(disk_meta[:uuid]).to eq("12345") end - - context "when an ISO file is already attached" do - let(:vm_info) { {"IDE Controller-0-0" => "/tmp/untitled.iso" } } - - it "skips the attachment" do - expect(driver).not_to receive(:attach_disk) - subject.handle_configure_dvd(machine, dvd_config) - end - end end end 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 5224800ab..c3b413206 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 @@ -118,4 +118,29 @@ OUTPUT subject.remove_disk(anything, anything, "IDE Controller") end end + + describe "#storage_controllers" do + before do + allow(subject).to receive(:show_vm_info).and_return( + {"storagecontrollername0" => "SATA Controller", + "storagecontrollertype0" => "IntelAhci", + "storagecontrollermaxportcount0" => "30", + "SATA Controller-ImageUUID-0-0" => "12345", + "SATA Controller-ImageUUID-1-0" => "67890"} + ) + end + + it "returns the storage controllers" do + expect(subject.storage_controllers.first[:name]).to eq("SATA Controller") + expect(subject.storage_controllers.first[:type]).to eq("IntelAhci") + expect(subject.storage_controllers.first[:maxportcount]).to eq(30) + end + + it "includes attachments for each storage controller" do + expect(subject.storage_controllers.first[:attachments]) + .to include(port: "0", device: "0", uuid: "12345") + expect(subject.storage_controllers.first[:attachments]) + .to include(port: "1", device: "0", uuid: "67890") + end + end end