diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index 910f64098..c3fa8595e 100644 --- a/lib/vagrant/errors.rb +++ b/lib/vagrant/errors.rb @@ -932,6 +932,22 @@ module Vagrant error_key(:virtualbox_disks_defined_exceed_limit) end + class VirtualBoxDisksControllerNotFound < VagrantError + error_key(:virtualbox_disks_controller_not_found) + end + + class VirtualBoxDisksNoSupportedControllers < VagrantError + error_key(:virtualbox_disks_no_supported_controllers) + end + + class VirtualBoxDisksPrimaryNotFound < VagrantError + error_key(:virtualbox_disks_primary_not_found) + end + + class VirtualBoxDisksUnsupportedController < VagrantError + error_key(:virtualbox_disks_unsupported_controller) + end + class VirtualBoxGuestPropertyNotFound < VagrantError error_key(:virtualbox_guest_property_not_found) end diff --git a/plugins/kernel_v2/config/disk.rb b/plugins/kernel_v2/config/disk.rb index 87c3d5741..dcce4d5df 100644 --- a/plugins/kernel_v2/config/disk.rb +++ b/plugins/kernel_v2/config/disk.rb @@ -41,7 +41,8 @@ module VagrantPlugins # @return [Integer,String] attr_accessor :size - # Path to the location of the disk file (Optional) + # Path to the location of the disk file (Optional for `:disk` type, + # required for `:dvd` type.) # # @return [String] attr_accessor :file @@ -171,10 +172,17 @@ module VagrantPlugins end end - if !@size + if !@size && type == :disk errors << I18n.t("vagrant.config.disk.invalid_size", name: @name, machine: machine.name) end + if @type == :dvd && !@file + errors << I18n.t("vagrant.config.disk.dvd_type_file_required", name: @name, machine: machine.name) + end + + if @type == :dvd && @primary + errors << I18n.t("vagrant.config.disk.dvd_type_primary", name: @name, machine: machine.name) + end if @file if !@file.is_a?(String) diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index 746896190..3f8e419b3 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -900,7 +900,7 @@ module VagrantPlugins end # Validate disks - # Check if there is more than one primrary disk defined and throw an error + # Check if there is more than one primary disk defined and throw an error primary_disks = @disks.select { |d| d.primary && d.type == :disk } if primary_disks.size > 1 errors << I18n.t("vagrant.config.vm.multiple_primary_disks_error", @@ -908,15 +908,24 @@ module VagrantPlugins end disk_names = @disks.map { |d| d.name } - duplicate_names = disk_names.detect{ |d| disk_names.count(d) > 1 } - if duplicate_names && duplicate_names.size + duplicate_names = disk_names.find_all { |d| disk_names.count(d) > 1 } + if duplicate_names.any? errors << I18n.t("vagrant.config.vm.multiple_disk_names_error", - name: duplicate_names) + name: machine.name, + disk_names: duplicate_names.uniq.join("\n")) + end + + disk_files = @disks.map { |d| d.file } + duplicate_files = disk_files.find_all { |d| d && disk_files.count(d) > 1 } + if duplicate_files.any? + errors << I18n.t("vagrant.config.vm.multiple_disk_files_error", + name: machine.name, + disk_files: duplicate_files.uniq.join("\n")) end @disks.each do |d| error = d.validate(machine) - errors.concat error if !error.empty? + errors.concat(error) if !error.empty? end # Validate clout_init_configs diff --git a/plugins/providers/virtualbox/cap/cleanup_disks.rb b/plugins/providers/virtualbox/cap/cleanup_disks.rb index 5d1029be5..b01a2b5ba 100644 --- a/plugins/providers/virtualbox/cap/cleanup_disks.rb +++ b/plugins/providers/virtualbox/cap/cleanup_disks.rb @@ -16,38 +16,69 @@ module VagrantPlugins return if !Vagrant::Util::Experimental.feature_enabled?("disks") handle_cleanup_disk(machine, defined_disks, disk_meta_file["disk"]) - # TODO: Floppy and DVD disks + handle_cleanup_dvd(machine, defined_disks, disk_meta_file["dvd"]) + # TODO: Floppy disks end protected # @param [Vagrant::Machine] machine # @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 + # @param [Array] disk_meta - An array of all the previously defined disks from the last configure_disk action def self.handle_cleanup_disk(machine, defined_disks, disk_meta) - vm_info = machine.provider.driver.show_vm_info - primary_disk = vm_info["SATA Controller-ImageUUID-0-0"] + raise TypeError, "Expected `Array` but received `#{disk_meta.class}`" if !disk_meta.is_a?(Array) + storage_controllers = machine.provider.driver.read_storage_controllers + + primary = storage_controllers.get_primary_attachment + primary_uuid = primary[:uuid] disk_meta.each do |d| dsk = defined_disks.select { |dk| dk.name == d["name"] } - if !dsk.empty? || d["uuid"] == primary_disk + if !dsk.empty? || d["uuid"] == primary_uuid next else LOGGER.warn("Found disk not in Vagrantfile config: '#{d["name"]}'. Removing disk from guest #{machine.name}") - disk_info = machine.provider.driver.get_port_and_device(d["uuid"]) - machine.ui.warn(I18n.t("vagrant.cap.cleanup_disks.disk_cleanup", name: d["name"]), prefix: true) - if disk_info.empty? + controller = storage_controllers.get_controller(d["controller"]) + attachment = controller.get_attachment(uuid: d["uuid"]) + + if !attachment LOGGER.warn("Disk '#{d["name"]}' not attached to guest, but still exists.") else - machine.provider.driver.remove_disk(disk_info[:port], disk_info[:device]) + machine.provider.driver.remove_disk(controller.name, attachment[:port], attachment[:device]) end machine.provider.driver.close_medium(d["uuid"]) end end end + + # @param [Vagrant::Machine] machine + # @param [VagrantPlugins::Kernel_V2::VagrantConfigDisk] defined_dvds + # @param [Array] dvd_meta - An array of all the previously defined dvds from the last configure_disk action + def self.handle_cleanup_dvd(machine, defined_dvds, dvd_meta) + raise TypeError, "Expected `Array` but received `#{dvd_meta.class}`" if !dvd_meta.is_a?(Array) + dvd_meta.each do |d| + dsk = defined_dvds.select { |dk| dk.name == d["name"] } + if !dsk.empty? + next + else + LOGGER.warn("Found dvd not in Vagrantfile config: '#{d["name"]}'. Removing dvd from guest #{machine.name}") + machine.ui.warn("DVD '#{d["name"]}' no longer exists in Vagrant config. Removing medium from guest...", prefix: true) + + storage_controllers = machine.provider.driver.read_storage_controllers + controller = storage_controllers.get_controller(d["controller"]) + attachment = controller.get_attachment(uuid: d["uuid"]) + + if !attachment + LOGGER.warn("DVD '#{d["name"]}' not attached to guest, but still exists.") + else + machine.provider.driver.remove_disk(controller.name, attachment[:port], attachment[:device]) + end + end + end + end end end end diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 5d6fe5f51..790592651 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -9,9 +9,6 @@ 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 - # @param [Vagrant::Machine] machine # @param [VagrantPlugins::Kernel_V2::VagrantConfigDisk] defined_disks # @return [Hash] configured_disks - A hash of all the current configured disks @@ -20,27 +17,68 @@ 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 - raise Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit - end - machine.ui.info(I18n.t("vagrant.cap.configure_disks.start")) + storage_controllers = machine.provider.driver.read_storage_controllers + + # Check to determine which controller we should attach disks to. + # If there is only one storage controller attached to the VM, use + # it. If there are multiple controllers (e.g. IDE/SATA), attach DVDs + # to the IDE controller and disks to the SATA controller. + if storage_controllers.size == 1 + controller = storage_controllers.first + + # The only way you can define up to the controller limit is if + # exactly one disk is a primary disk, otherwise we need to reserve + # a slot for the primary + if (defined_disks.any? { |d| d.primary } && defined_disks.size > controller.limit) || + defined_disks.size > controller.limit - 1 + raise Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit, + limit: controller.limit, + name: controller.name + else + disk_controller = controller + dvd_controller = controller + end + else + disks_defined = defined_disks.select { |d| d.type == :disk } + if disks_defined.any? + disk_controller = storage_controllers.get_primary_controller + + if (disks_defined.any? { |d| d.primary } && disks_defined.size > disk_controller.limit) || + disks_defined.size > disk_controller.limit - 1 + raise Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit, + limit: disk_controller.limit, + name: disk_controller.name + end + end + + dvds_defined = defined_disks.select { |d| d.type == :dvd } + if dvds_defined.any? + dvd_controller = storage_controllers.get_dvd_controller + + if dvds_defined.size > dvd_controller.limit + raise Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit, + limit: dvd_controller.limit, + name: dvd_controller.name + end + end + end + current_disks = machine.provider.driver.list_hdds - configured_disks = {disk: [], floppy: [], dvd: []} + configured_disks = { disk: [], floppy: [], dvd: [] } defined_disks.each do |disk| if disk.type == :disk - disk_data = handle_configure_disk(machine, disk, current_disks) + disk_data = handle_configure_disk(machine, disk, current_disks, disk_controller.name) configured_disks[:disk] << disk_data unless disk_data.empty? elsif disk.type == :floppy # TODO: Write me machine.ui.info(I18n.t("vagrant.cap.configure_disks.floppy_not_supported", name: disk.name)) elsif disk.type == :dvd - # TODO: Write me - machine.ui.info(I18n.t("vagrant.cap.configure_disks.dvd_not_supported", name: disk.name)) + dvd_data = handle_configure_dvd(machine, disk, dvd_controller.name) + configured_disks[:dvd] << dvd_data unless dvd_data.empty? end end @@ -56,15 +94,13 @@ module VagrantPlugins def self.get_current_disk(machine, disk, all_disks) current_disk = nil if disk.primary - # 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"] + storage_controllers = machine.provider.driver.read_storage_controllers + primary = storage_controllers.get_primary_attachment + primary_uuid = primary[:uuid] current_disk = all_disks.select { |d| d["UUID"] == primary_uuid }.first else - current_disk = all_disks.select { |d| d["Disk Name"] == disk.name}.first + current_disk = all_disks.detect { |d| d["Disk Name"] == disk.name } end current_disk @@ -75,8 +111,12 @@ module VagrantPlugins # @param [Vagrant::Machine] machine - the current machine # @param [Config::Disk] disk - the current disk to configure # @param [Array] all_disks - A list of all currently defined disks in VirtualBox + # @param [String] controller_name - the name of the storage controller to use # @return [Hash] - disk_metadata - def self.handle_configure_disk(machine, disk, all_disks) + def self.handle_configure_disk(machine, disk, all_disks, controller_name) + storage_controllers = machine.provider.driver.read_storage_controllers + controller = storage_controllers.get_controller(controller_name) + disk_metadata = {} # Grab the existing configured disk, if it exists @@ -85,29 +125,85 @@ module VagrantPlugins # Configure current disk if !current_disk # create new disk and attach - disk_metadata = create_disk(machine, disk) + disk_metadata = create_disk(machine, disk, controller) elsif compare_disk_size(machine, disk, current_disk) - disk_metadata = resize_disk(machine, disk, current_disk) + disk_metadata = resize_disk(machine, disk, current_disk, controller) else # TODO: What if it needs to be resized? 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) - machine.provider.driver.attach_disk(dsk_info[:port], + dsk_info = get_next_port(machine, controller) + machine.provider.driver.attach_disk(controller.name, + dsk_info[:port], dsk_info[:device], + "hdd", current_disk["Location"]) + disk_metadata[:port] = dsk_info[:port] + disk_metadata[:device] = dsk_info[:device] else LOGGER.info("No further configuration required for disk '#{disk.name}'") + disk_metadata[:port] = disk_info[:port] + disk_metadata[:device] = disk_info[:device] end - disk_metadata = {uuid: current_disk["UUID"], name: disk.name} + disk_metadata[:uuid] = current_disk["UUID"] + disk_metadata[:name] = disk.name + disk_metadata[:controller] = controller.name end disk_metadata end + # Handles all disk configs of type `:dvd` + # + # @param [Vagrant::Machine] machine - the current machine + # @param [Config::Disk] dvd - the current disk to configure + # @param [String] controller_name - the name of the storage controller to use + # @return [Hash] - dvd_metadata + def self.handle_configure_dvd(machine, dvd, controller_name) + storage_controllers = machine.provider.driver.read_storage_controllers + controller = storage_controllers.get_controller(controller_name) + + dvd_metadata = {} + + dvd_location = File.expand_path(dvd.file) + dvd_attached = controller.attachments.detect { |a| a[:location] == dvd_location } + + if dvd_attached + LOGGER.info("No further configuration required for dvd '#{dvd.name}'") + dvd_metadata[:name] = dvd.name + dvd_metadata[:port] = dvd_attached[:port] + dvd_metadata[:device] = dvd_attached[:device] + dvd_metadata[:uuid] = dvd_attached[:uuid] + dvd_metadata[:controller] = controller.name + else + LOGGER.warn("DVD '#{dvd.name}' is not connected to guest '#{machine.name}', Vagrant will attempt to connect dvd to guest") + dsk_info = get_next_port(machine, controller) + machine.provider.driver.attach_disk(controller.name, + dsk_info[:port], + dsk_info[:device], + "dvddrive", + dvd.file) + + # Refresh the controller information + storage_controllers = machine.provider.driver.read_storage_controllers + controller = storage_controllers.get_controller(controller_name) + + attachment = controller.attachments.detect { |a| a[:port] == dsk_info[:port] && + a[:device] == dsk_info[:device] } + + dvd_metadata[:name] = dvd.name + dvd_metadata[:port] = dsk_info[:port] + dvd_metadata[:device] = dsk_info[:device] + dvd_metadata[:uuid] = attachment[:uuid] + dvd_metadata[:controller] = controller.name + end + + dvd_metadata + end + # Check to see if current disk is configured based on defined_disks # # @param [Kernel_V2::VagrantConfigDisk] disk_config @@ -131,7 +227,9 @@ module VagrantPlugins # # @param [Vagrant::Machine] machine # @param [Kernel_V2::VagrantConfigDisk] disk_config - def self.create_disk(machine, disk_config) + # @param [VagrantPlugins::ProviderVirtualBox::Model::StorageController] controller - + # the storage controller to use + def self.create_disk(machine, disk_config, controller) machine.ui.detail(I18n.t("vagrant.cap.configure_disks.create_disk", name: disk_config.name)) # NOTE: At the moment, there are no provider specific configs for VirtualBox # but we grab it anyway for future use. @@ -146,10 +244,16 @@ module VagrantPlugins LOGGER.info("Attempting to create a new disk file '#{disk_file}' of size '#{disk_config.size}' bytes") 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.attach_disk(controller.name, + dsk_controller_info[:port], + dsk_controller_info[:device], + "hdd", + disk_file) - dsk_controller_info = get_next_port(machine) - machine.provider.driver.attach_disk(dsk_controller_info[:port], dsk_controller_info[:device], disk_file) + disk_metadata = { uuid: disk_var.split(":").last.strip, name: disk_config.name, + controller: controller.name, port: dsk_controller_info[:port], + device: dsk_controller_info[:device] } disk_metadata end @@ -167,20 +271,47 @@ module VagrantPlugins # device = disk_info[3] # # @param [Vagrant::Machine] machine + # @param [VagrantPlugins::ProviderVirtualBox::Model::StorageController] controller - + # the storage controller to use # @return [Hash] dsk_info - The next available port and device on a given controller - def self.get_next_port(machine) - vm_info = machine.provider.driver.show_vm_info - dsk_info = {device: "0", port: "0"} + def self.get_next_port(machine, controller) + dsk_info = {} - disk_images = vm_info.select { |v| v.include?("ImageUUID") && v.include?("SATA 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.devices_per_port == 1 + 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 - if dsk_info[:port].empty? + dsk_info[:port] = next_available_port.to_s + dsk_info[:device] = "0" + elsif controller.devices_per_port == 2 + # IDE Controllers have primary/secondary devices, so find the first port + # with an empty 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 == controller.devices_per_port + + dsk_info[:port] = port.to_s + + # Check for a free device + if port_attachments.any? { |a| a[:device] == "0" } + dsk_info[:device] = "1" + else + dsk_info[:device] = "0" + end + + break if dsk_info[:port] + end + else + raise Vagrant::Errors::VirtualBoxDisksUnsupportedController, controller_name: controller.name + end + + if dsk_info[:port].to_s.empty? # This likely only occurs if additional disks have been added outside of Vagrant configuration - LOGGER.warn("There are no more available ports to attach disks to for the SATA Controller. Clear up some space on the SATA controller to attach new disks.") - raise Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit + LOGGER.warn("There is no more available space to attach disks to for the controller '#{controller}'. Clear up some space on the controller '#{controller}' to attach new disks.") + raise Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit, + limit: controller.limit, + name: controller.name end dsk_info @@ -189,15 +320,18 @@ module VagrantPlugins # @param [Vagrant::Machine] machine # @param [Config::Disk] disk_config - the current disk to configure # @param [Hash] defined_disk - current disk as represented by VirtualBox + # @param [VagrantPlugins::ProviderVirtualBox::Model::StorageController] controller - + # the storage controller to use # @return [Hash] - disk_metadata - def self.resize_disk(machine, disk_config, defined_disk) + def self.resize_disk(machine, disk_config, defined_disk, controller) machine.ui.detail(I18n.t("vagrant.cap.configure_disks.resize_disk", name: disk_config.name), prefix: true) + # grab disk to be resized port and device number + disk_info = machine.provider.driver.get_port_and_device(defined_disk["UUID"]) + if defined_disk["Storage format"] == "VMDK" LOGGER.warn("Disk type VMDK cannot be resized in VirtualBox. Vagrant will convert disk to VDI format to resize first, and then convert resized disk back to VMDK format") - # grab disk to be resized port and device number - disk_info = machine.provider.driver.get_port_and_device(defined_disk["UUID"]) # original disk information in case anything goes wrong during clone/resize original_disk = defined_disk backup_disk_location = "#{original_disk["Location"]}.backup" @@ -209,9 +343,8 @@ module VagrantPlugins begin # Danger Zone - # remove and close original volume - machine.provider.driver.remove_disk(disk_info[:port], disk_info[:device]) + machine.provider.driver.remove_disk(controller.name, disk_info[:port], disk_info[:device]) # 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) @@ -222,14 +355,17 @@ module VagrantPlugins # clone back to original vmdk format and attach resized disk vmdk_disk_file = machine.provider.driver.vdi_to_vmdk(vdi_disk_file) - machine.provider.driver.attach_disk(disk_info[:port], disk_info[:device], vmdk_disk_file, "hdd") + machine.provider.driver.attach_disk(controller.name, + disk_info[:port], + disk_info[:device], + "hdd", + vmdk_disk_file) rescue ScriptError, SignalException, StandardError LOGGER.warn("Vagrant encountered an error while trying to resize a disk. Vagrant will now attempt to reattach and preserve the original disk...") machine.ui.error(I18n.t("vagrant.cap.configure_disks.recovery_from_resize", location: original_disk["Location"], name: machine.name)) - recover_from_resize(machine, disk_info, backup_disk_location, original_disk, vdi_disk_file) - + recover_from_resize(machine, disk_info, backup_disk_location, original_disk, vdi_disk_file, controller) raise ensure # Remove backup disk file if all goes well @@ -246,7 +382,8 @@ module VagrantPlugins machine.provider.driver.resize_disk(defined_disk["Location"], disk_config.size.to_i) end - disk_metadata = {uuid: defined_disk["UUID"], name: disk_config.name} + disk_metadata = { uuid: defined_disk["UUID"], name: disk_config.name, controller: controller.name, + port: disk_info[:port], device: disk_info[:device] } disk_metadata end @@ -261,13 +398,17 @@ module VagrantPlugins # @param [String] backup_disk_location - The place on disk where vagrant made a backup of the original disk being resized # @param [Hash] original_disk - The disk information from VirtualBox # @param [String] vdi_disk_file - The place on disk where vagrant made a clone of the original disk being resized - def self.recover_from_resize(machine, disk_info, backup_disk_location, original_disk, vdi_disk_file) + # @param [VagrantPlugins::ProviderVirtualBox::Model::StorageController] controller - the storage controller to use + def self.recover_from_resize(machine, disk_info, backup_disk_location, original_disk, vdi_disk_file, controller) begin # move backup to original name FileUtils.mv(backup_disk_location, original_disk["Location"], force: true) # Attach disk - machine.provider.driver. - attach_disk(disk_info[:port], disk_info[:device], original_disk["Location"], "hdd") + machine.provider.driver.attach_disk(controller.name, + disk_info[:port], + disk_info[:device], + "hdd", + original_disk["Location"]) # Remove cloned disk if still hanging around if vdi_disk_file diff --git a/plugins/providers/virtualbox/driver/meta.rb b/plugins/providers/virtualbox/driver/meta.rb index 7175112ba..54e817a17 100644 --- a/plugins/providers/virtualbox/driver/meta.rb +++ b/plugins/providers/virtualbox/driver/meta.rb @@ -117,6 +117,7 @@ module VagrantPlugins :export, :forward_ports, :get_port_and_device, + :get_storage_controller, :halt, :import, :list_snapshots, @@ -133,6 +134,7 @@ module VagrantPlugins :read_machine_folder, :read_network_interfaces, :read_state, + :read_storage_controllers, :read_used_ports, :read_vms, :reconfig_host_only, diff --git a/plugins/providers/virtualbox/driver/version_5_0.rb b/plugins/providers/virtualbox/driver/version_5_0.rb index b08576a8f..dc01e1f14 100644 --- a/plugins/providers/virtualbox/driver/version_5_0.rb +++ b/plugins/providers/virtualbox/driver/version_5_0.rb @@ -22,20 +22,22 @@ module VagrantPlugins # - Port: 0 # - Device: 0 # + # @param [String] controller_name - name of storage controller to attach disk to # @param [String] port - port on device to attach disk to # @param [String] device - device on controller for disk - # @param [String] file - disk file path # @param [String] type - type of disk to attach + # @param [String] file - disk file path # @param [Hash] opts - additional options - def attach_disk(port, device, file, type="hdd", **opts) - # Maybe only support SATA Controller for `:disk`??? - controller = "SATA Controller" - + def attach_disk(controller_name, port, device, type, file, **opts) comment = "This disk is managed externally by Vagrant. Removing or adjusting settings could potentially cause issues with Vagrant." - execute('storageattach', @uuid, '--storagectl', controller, '--port', - port.to_s, '--device', device.to_s, '--type', type, '--medium', - file, '--comment', comment) + execute('storageattach', @uuid, + '--storagectl', controller_name, + '--port', port.to_s, + '--device', device.to_s, + '--type', type, + '--medium', file, + '--comment', comment) end def clear_forwarded_ports @@ -224,12 +226,15 @@ module VagrantPlugins raise end + # @param [String] controller_name - controller name to remove disk from # @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" - execute('storageattach', @uuid, '--storagectl', controller, '--port', port.to_s, '--device', device.to_s, '--medium', "none") + def remove_disk(controller_name, port, device) + execute('storageattach', @uuid, + '--storagectl', controller_name, + '--port', port.to_s, + '--device', device.to_s, + '--medium', "none") end # @param [String] disk_file @@ -388,20 +393,21 @@ module VagrantPlugins # Returns port and device for an attached disk given a disk uuid. Returns # empty hash if disk is not attachd to guest # - # @param [Hash] vm_info - A guests information from vboxmanage # @param [String] disk_uuid - the UUID for the disk we are searching for # @return [Hash] disk_info - Contains a device and port number def get_port_and_device(disk_uuid) - vm_info = show_vm_info - disk = {} - disk_info_key = vm_info.key(disk_uuid) - return disk if !disk_info_key - disk_info = disk_info_key.split("-") - - disk[:port] = disk_info[2] - disk[:device] = disk_info[3] + storage_controllers = read_storage_controllers + storage_controllers.each do |controller| + controller.attachments.each do |attachment| + if disk_uuid == attachment[:uuid] + disk[:port] = attachment[:port] + disk[:device] = attachment[:device] + return disk + end + end + end return disk end @@ -923,6 +929,41 @@ module VagrantPlugins destination end + # Helper method to get a list of storage controllers added to the + # current VM + # + # @return [VagrantPlugins::ProviderVirtualBox::Model::StorageControllerArray] + def read_storage_controllers + vm_info = show_vm_info + count = vm_info.count { |key, value| key.match(/^storagecontrollername\d+$/) } + + storage_controllers = Model::StorageControllerArray.new + + (0..count - 1).each 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 + port = $1.to_s + device = $2.to_s + uuid = v + location = vm_info["#{name}-#{port}-#{device}"] + + attachments << {port: port, device: device, uuid: uuid, location: location} + end + end + + storage_controllers << Model::StorageController.new(name, type, maxportcount, attachments) + end + + storage_controllers + end + protected def valid_ip_address?(ip) diff --git a/plugins/providers/virtualbox/model/storage_controller.rb b/plugins/providers/virtualbox/model/storage_controller.rb new file mode 100644 index 000000000..bb9adefcb --- /dev/null +++ b/plugins/providers/virtualbox/model/storage_controller.rb @@ -0,0 +1,135 @@ +module VagrantPlugins + module ProviderVirtualBox + module Model + # Represents a storage controller for VirtualBox. Storage controllers + # have a type, a name, and can have hard disks or optical drives attached. + class StorageController + IDE_CONTROLLER_TYPES = ["PIIX4", "PIIX3", "ICH6"].map(&:freeze).freeze + SATA_CONTROLLER_TYPES = ["IntelAhci"].map(&:freeze).freeze + SCSI_CONTROLLER_TYPES = [ "LsiLogic", "BusLogic"].map(&:freeze).freeze + + IDE_DEVICES_PER_PORT = 2.freeze + SATA_DEVICES_PER_PORT = 1.freeze + SCSI_DEVICES_PER_PORT = 1.freeze + + IDE_BOOT_PRIORITY = 1.freeze + SATA_BOOT_PRIORITY = 2.freeze + SCSI_BOOT_PRIORITY = 3.freeze + + # The name of the storage controller. + # + # @return [String] + attr_reader :name + + # The specific type of controller. + # + # @return [String] + attr_reader :type + + # The maximum number of avilable ports for the storage controller. + # + # @return [Integer] + attr_reader :maxportcount + + # The number of devices that can be attached to each port. For SATA + # controllers, this will usually be 1, and for IDE controllers this + # will usually be 2. + # @return [Integer] + attr_reader :devices_per_port + + # The maximum number of individual disks that can be attached to the + # storage controller. For SATA controllers, this equals the maximum + # number of ports. For IDE controllers, this will be twice the max + # number of ports (primary/secondary). + # + # @return [Integer] + attr_reader :limit + + # The boot priority of the storage controller. This does not seem to + # depend on the controller number returned by `showvminfo`. + # Experimentation has determined that VirtualBox will try to boot from + # the first controller it finds with a hard disk, in this order: + # IDE, SATA, SCSI + # + # @return [Integer] + attr_reader :boot_priority + + # The list of disks/ISOs attached to each storage controller. + # + # @return [Array] + attr_reader :attachments + + def initialize(name, type, maxportcount, attachments) + @name = name + @type = type + + @maxportcount = maxportcount.to_i + + if IDE_CONTROLLER_TYPES.include?(@type) + @storage_bus = :ide + @devices_per_port = IDE_DEVICES_PER_PORT + @boot_priority = IDE_BOOT_PRIORITY + elsif SATA_CONTROLLER_TYPES.include?(@type) + @storage_bus = :sata + @devices_per_port = SATA_DEVICES_PER_PORT + @boot_priority = SATA_BOOT_PRIORITY + elsif SCSI_CONTROLLER_TYPES.include?(@type) + @storage_bus = :scsi + @devices_per_port = SCSI_DEVICES_PER_PORT + @boot_priority = SCSI_BOOT_PRIORITY + else + @storage_bus = :unknown + @devices_per_port = 1 + end + + @limit = @maxportcount * @devices_per_port + + attachments ||= [] + @attachments = attachments + end + + # Get a single storage device, either by port/device address or by + # UUID. + # + # @param [Hash] opts - A hash of options to match + # @return [Hash] attachment - Attachment information + def get_attachment(opts = {}) + if opts[:port] && opts[:device] + @attachments.detect { |a| a[:port] == opts[:port] && + a[:device] == opts[:device] } + elsif opts[:uuid] + @attachments.detect { |a| a[:uuid] == opts[:uuid] } + end + end + + # Returns true if the storage controller has a supported type. + # + # @return [Boolean] + def supported? + [:ide, :sata, :scsi].include?(@storage_bus) + end + + # Returns true if the storage controller is a IDE type controller. + # + # @return [Boolean] + def ide? + @storage_bus == :ide + end + + # Returns true if the storage controller is a SATA type controller. + # + # @return [Boolean] + def sata? + @storage_bus == :sata + end + + # Returns true if the storage controller is a SCSI type controller. + # + # @return [Boolean] + def scsi? + @storage_bus == :scsi + end + end + end + end +end diff --git a/plugins/providers/virtualbox/model/storage_controller_array.rb b/plugins/providers/virtualbox/model/storage_controller_array.rb new file mode 100644 index 000000000..452acf793 --- /dev/null +++ b/plugins/providers/virtualbox/model/storage_controller_array.rb @@ -0,0 +1,100 @@ +require_relative "../cap/validate_disk_ext" + +module VagrantPlugins + module ProviderVirtualBox + module Model + # A collection of storage controllers. Includes finder methods to look + # up a storage controller by given attributes. + class StorageControllerArray < Array + # Returns a storage controller with the given name. Raises an + # exception if a matching controller can't be found. + # + # @param [String] name - The name of the storage controller + # @return [VagrantPlugins::ProviderVirtualBox::Model::StorageController] + def get_controller(name) + controller = detect { |c| c.name == name } + if !controller + raise Vagrant::Errors::VirtualBoxDisksControllerNotFound, name: name + end + controller + end + + # Find the controller containing the primary disk (i.e. the boot + # disk). This is used to determine which controller virtual disks + # should be attached to. + # + # Raises an exception if no supported controllers are found. + # + # @return [VagrantPlugins::ProviderVirtualBox::Model::StorageController] + def get_primary_controller + ordered = sort { |a, b| a.boot_priority <=> b.boot_priority } + controller = ordered.detect do |c| + c.supported? && c.attachments.any? { |a| hdd?(a) } + end + + if !controller + raise Vagrant::Errors::VirtualBoxDisksNoSupportedControllers, + supported_types: supported_types.join(" ,") + end + + controller + end + + # Find the attachment representing the primary disk (i.e. the boot + # disk). We can't rely on the order of #list_hdds, as they will not + # always come in port order, but primary is always Port 0 Device 0. + # + # @return [Hash] attachment - Primary disk attachment information + def get_primary_attachment + attachment = nil + + controller = get_primary_controller + attachment = controller.get_attachment(port: "0", device: "0") + if !attachment + raise Vagrant::Errors::VirtualBoxDisksPrimaryNotFound + end + + attachment + end + + # Returns the first supported storage controller for attaching dvds. + # Will raise an exception if no suitable controller can be found. + # + # @return [VagrantPlugins::ProviderVirtualBox::Model::StorageController] + def get_dvd_controller + ordered = sort { |a, b| a.boot_priority <=> b.boot_priority } + controller = ordered.detect { |c| c.supported? } + if !controller + raise Vagrant::Errors::VirtualBoxDisksNoSupportedControllers, + supported_types: supported_types.join(" ,") + end + + controller + end + + private + + # Determine whether the given attachment is a hard disk. + # + # @param [Hash] attachment - Attachment information + # @return [Boolean] + def hdd?(attachment) + if !attachment + false + else + ext = File.extname(attachment[:location].to_s).downcase.split('.').last + VagrantPlugins::ProviderVirtualBox::Cap::ValidateDiskExt.validate_disk_ext(nil, ext) + end + end + + # Returns a list of all the supported controller types. + # + # @return [Array] + def supported_types + StorageController::SATA_CONTROLLER_TYPES + StorageController::IDE_CONTROLLER_TYPES + + StorageController::SCSI_CONTROLLER_TYPES + end + end + end + end +end diff --git a/plugins/providers/virtualbox/plugin.rb b/plugins/providers/virtualbox/plugin.rb index 63f6a0e21..8a44e613e 100644 --- a/plugins/providers/virtualbox/plugin.rb +++ b/plugins/providers/virtualbox/plugin.rb @@ -89,6 +89,8 @@ module VagrantPlugins module Model autoload :ForwardedPort, File.expand_path("../model/forwarded_port", __FILE__) + autoload :StorageController, File.expand_path("../model/storage_controller", __FILE__) + autoload :StorageControllerArray, File.expand_path("../model/storage_controller_array", __FILE__) end module Util diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 7ce0f57e3..6bdb40967 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -1678,11 +1678,30 @@ 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_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. + virtualbox_disks_controller_not_found: |- + Vagrant expected to find a storage controller called '%{name}', + but there is no controller with this name attached to the current VM. + If you have changed or removed any storage controllers, please restore + them to their previous configuration. + virtualbox_disks_no_supported_controllers: |- + Vagrant was unable to detect a disk controller with any of the + following supported types: - Please ensure only up to 30 disks are configured for your guest. + %{supported_types} + + Please add one of the supported controller types in order to use the + disk configuration feature. + virtualbox_disks_defined_exceed_limit: |- + VirtualBox only allows up to %{limit} disks to be attached to the + storage controller '%{name}'. Please remove some disks from your disk + configuration, or attach a new storage controller. + virtualbox_disks_primary_not_found: |- + Vagrant was not able to detect a primary disk attached to the main + controller. Vagrant Disks requires that the primary disk be attached + to the first port of the main controller in order to manage it. + virtualbox_disks_unsupported_controller: |- + An disk operation was attempted on the controller '%{controller_name}', + but Vagrant doesn't support this type of disk controller. virtualbox_guest_property_not_found: |- Could not find a required VirtualBox guest property: %{guest_property} @@ -1844,6 +1863,11 @@ en: #------------------------------------------------------------------------------- config: disk: + dvd_type_file_required: + A 'file' option is required when defining a disk of type ':dvd' for guest '%{machine}'. + dvd_type_primary: |- + Disks of type ':dvd' cannot be defined as a primary disks. Please + remove the 'primary' argument for disk '%{name}' on guest '%{machine}'. invalid_ext: |- Disk type '%{ext}' is not a valid disk extention for '%{name}'. Please pick one of the following supported disk types: %{exts} invalid_type: |- @@ -1970,8 +1994,20 @@ en: Ignoring provider config for validation... multiple_primary_disks_error: |- There are more than one primary disks defined for guest '%{name}'. Please ensure that only one disk has been defined as a primary disk. + multiple_disk_files_error: |- + The following disk files are used multiple times in the disk + configuration for the VM '%{name}': + + %{disk_files} + + Each disk file may only be attached to a VM once. multiple_disk_names_error: |- - Duplicate disk names defined: '%{name}'. Disk names must be unique. + The following disks names are defined multiple times in the disk + configuration for the VM '%{name}': + + %{disk_names} + + Disk names must be unique. multiple_networks_set_hostname: "Multiple networks have set `:hostname`" name_invalid: |- The sub-VM name '%{name}' is invalid. Please don't use special characters. @@ -2231,7 +2267,6 @@ en: configure_disks: start: "Configuring storage mediums..." floppy_not_supported: "Floppy disk configuration not yet supported. Skipping disk '%{name}'..." - dvd_not_supported: "DVD disk configuration not yet supported. Skipping disk '%{name}'..." shrink_size_not_supported: |- VirtualBox does not support shrinking disk sizes. Cannot shrink '%{name}' disks size. create_disk: |- diff --git a/test/unit/plugins/kernel_v2/config/disk_test.rb b/test/unit/plugins/kernel_v2/config/disk_test.rb index 967a9046d..48bec3b78 100644 --- a/test/unit/plugins/kernel_v2/config/disk_test.rb +++ b/test/unit/plugins/kernel_v2/config/disk_test.rb @@ -15,17 +15,16 @@ describe VagrantPlugins::Kernel_V2::VagrantConfigDisk do let(:machine) { double("machine", name: "name", provider: provider, env: env, provider_name: :virtualbox) } - def assert_invalid errors = subject.validate(machine) - if !errors.empty? { |v| !v.empty? } + if errors.empty? raise "No errors: #{errors.inspect}" end end def assert_valid errors = subject.validate(machine) - if !errors.empty? { |v| v.empty? } + if !errors.empty? raise "Errors: #{errors.inspect}" end end @@ -33,8 +32,6 @@ describe VagrantPlugins::Kernel_V2::VagrantConfigDisk do before do env = double("env") - subject.name = "foo" - subject.size = 100 allow(provider).to receive(:capability?).with(:validate_disk_ext).and_return(true) allow(provider).to receive(:capability).with(:validate_disk_ext, "vdi").and_return(true) allow(provider).to receive(:capability?).with(:set_default_disk_ext).and_return(true) @@ -42,6 +39,11 @@ describe VagrantPlugins::Kernel_V2::VagrantConfigDisk do end describe "with defaults" do + before do + subject.name = "foo" + subject.size = 100 + end + it "is valid with test defaults" do subject.finalize! assert_valid @@ -52,25 +54,31 @@ describe VagrantPlugins::Kernel_V2::VagrantConfigDisk do expect(subject.type).to eq(type) end - it "defaults to non-primray disk" do + it "defaults to non-primary disk" do subject.finalize! expect(subject.primary).to eq(false) end end describe "with an invalid config" do - let(:invalid_subject) { described_class.new(type) } + before do + subject.name = "bar" + end it "raises an error if size not set" do - invalid_subject.name = "bar" subject.finalize! assert_invalid end context "with an invalid disk extension" do before do + subject.size = 100 + subject.disk_ext = "fake" + allow(provider).to receive(:capability?).with(:validate_disk_ext).and_return(true) allow(provider).to receive(:capability).with(:validate_disk_ext, "fake").and_return(false) + allow(provider).to receive(:capability?).with(:default_disk_exts).and_return(true) + allow(provider).to receive(:capability).with(:default_disk_exts).and_return(["vdi", "vmdk"]) end it "raises an error" do @@ -79,4 +87,32 @@ describe VagrantPlugins::Kernel_V2::VagrantConfigDisk do end end end + + describe "config for dvd type" do + let(:iso_path) { "/tmp/untitled.iso" } + + before do + subject.type = :dvd + subject.name = "untitled" + allow(File).to receive(:file?).with(iso_path).and_return(true) + subject.file = iso_path + end + + it "is valid with test defaults" do + subject.finalize! + assert_valid + end + + it "is invalid if file path is unset" do + subject.file = nil + subject.finalize! + assert_invalid + end + + it "is invalid if primary" do + subject.primary = true + subject.finalize! + assert_invalid + end + end end diff --git a/test/unit/plugins/kernel_v2/config/vm_test.rb b/test/unit/plugins/kernel_v2/config/vm_test.rb index c6576df6a..914d3c74f 100644 --- a/test/unit/plugins/kernel_v2/config/vm_test.rb +++ b/test/unit/plugins/kernel_v2/config/vm_test.rb @@ -8,7 +8,7 @@ describe VagrantPlugins::Kernel_V2::VMConfig do subject { described_class.new } let(:provider) { double("provider") } - let(:machine) { double("machine", provider: provider, provider_name: "provider") } + let(:machine) { double("machine", provider: provider, provider_name: "provider", name: "default") } def assert_invalid errors = subject.validate(machine) @@ -622,6 +622,14 @@ describe VagrantPlugins::Kernel_V2::VMConfig do assert_invalid end + it "raises an error with duplicate disk files" do + allow(File).to receive(:file?).with("bar.vmdk").and_return(true) + subject.disk(:disk, size: 100, name: "foo1", file: "bar.vmdk") + subject.disk(:disk, size: 100, name: "foo2", file: "bar.vmdk") + subject.finalize! + assert_invalid + end + it "does not merge duplicate disks" do subject.disk(:disk, size: 1000, primary: false, name: "storage") subject.disk(:disk, size: 1000, primary: false, name: "backup") diff --git a/test/unit/plugins/kernel_v2/config/vm_trigger_test.rb b/test/unit/plugins/kernel_v2/config/vm_trigger_test.rb index 6735e219f..53708cb0c 100644 --- a/test/unit/plugins/kernel_v2/config/vm_trigger_test.rb +++ b/test/unit/plugins/kernel_v2/config/vm_trigger_test.rb @@ -13,14 +13,14 @@ describe VagrantPlugins::Kernel_V2::VagrantConfigTrigger do def assert_invalid errors = subject.validate(machine) - if !errors.empty? { |v| !v.empty? } + if errors.empty? raise "No errors: #{errors.inspect}" end end def assert_valid errors = subject.validate(machine) - if !errors.empty? { |v| v.empty? } + if !errors.empty? raise "Errors: #{errors.inspect}" end end 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 7a64c14d8..6dc9072cc 100644 --- a/test/unit/plugins/providers/virtualbox/cap/cleanup_disks_test.rb +++ b/test/unit/plugins/providers/virtualbox/cap/cleanup_disks_test.rb @@ -27,25 +27,35 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::CleanupDisks do let(:subject) { described_class } - let(:disk_meta_file) { {disk: [], floppy: [], dvd: []} } + 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(:attachments) { [{port: "0", device: "0", uuid: "12345"}, + {port: "1", device: "0", uuid: "67890"}]} + + let(:controller) { double("controller", name: "controller", limit: 30, maxportcount: 30) } + + let(:storage_controllers) { double("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(controller).to receive(:get_attachment).with(port: "0", device: "0").and_return(attachments[0]) + allow(controller).to receive(:get_attachment).with(uuid: "12345").and_return(attachments[0]) + allow(controller).to receive(:get_attachment).with(uuid: "67890").and_return(attachments[1]) + allow(storage_controllers).to receive(:get_controller).and_return(controller) + allow(storage_controllers).to receive(:get_primary_controller).and_return(controller) + allow(storage_controllers).to receive(:get_primary_attachment).and_return(attachments[0]) + allow(driver).to receive(:read_storage_controllers).and_return(storage_controllers) end - context "#cleanup_disks" do + describe "#cleanup_disks" do it "returns if there's no data in meta file" do subject.cleanup_disks(machine, defined_disks, disk_meta_file) expect(subject).not_to receive(:handle_cleanup_disk) end - describe "with disks to clean up" do - let(:disk_meta_file) { {disk: [{uuid: "1234", name: "storage"}], floppy: [], dvd: []} } + context "with disks to clean up" do + let(:disk_meta_file) { {"disk" => [{"uuid" => "1234", "name" => "storage"}], "floppy" => [], "dvd" => []} } it "calls the cleanup method if a disk_meta file is defined" do expect(subject).to receive(:handle_cleanup_disk). @@ -54,35 +64,79 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::CleanupDisks do subject.cleanup_disks(machine, defined_disks, disk_meta_file) end + + it "raises an error if primary disk can't be found" do + allow(storage_controllers).to receive(:get_primary_attachment).and_raise(Vagrant::Errors::VirtualBoxDisksPrimaryNotFound) + + expect { subject.cleanup_disks(machine, defined_disks, disk_meta_file) }. + to raise_error(Vagrant::Errors::VirtualBoxDisksPrimaryNotFound) + end + end + + context "with dvd attached" do + let(:disk_meta_file) { {"disk" => [], "floppy" => [], "dvd" => [{"uuid" => "12345", "name" => "iso"}] } } + + it "calls the cleanup method if a disk_meta file is defined" do + expect(subject).to receive(:handle_cleanup_dvd). + with(machine, defined_disks, disk_meta_file["dvd"]). + and_return(true) + + subject.cleanup_disks(machine, defined_disks, disk_meta_file) + end end end - context "#handle_cleanup_disk" do - let(:disk_meta_file) { {disk: [{"uuid"=>"67890", "name"=>"storage"}], floppy: [], dvd: []} } + describe "#handle_cleanup_disk" do + let(:disk_meta_file) { { disk: [{ "uuid" => "67890", "name" => "storage", "controller" => "controller", "port" => "1", "device" => "0" }], floppy: [], dvd: [] } } + let(:defined_disks) { [] } - let(:device_info) { {port: "1", device: "0"} } it "removes and closes medium from guest" do - allow(driver).to receive(:get_port_and_device). - 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("controller", "1", "0").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]) end - describe "when the disk isn't attached to a guest" do + context "when the disk isn't attached to a guest" do it "only closes the medium" do - allow(driver).to receive(:get_port_and_device). - with("67890"). - and_return({}) + allow(controller).to receive(:get_attachment).with(uuid: "67890").and_return(nil) + expect(driver).to receive(:close_medium).with("67890").and_return(true) + subject.handle_cleanup_disk(machine, defined_disks, disk_meta_file[:disk]) + end + end + + context "when attachment is not found at the expected device" do + it "removes the disk from the correct device" do + allow(controller).to receive(:get_attachment).with(uuid: "67890").and_return(port: "2", device: "0") + expect(driver).to receive(:remove_disk).with("controller", "2", "0").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]) end end end + + describe "#handle_cleanup_dvd" do + let(:disk_meta_file) { {dvd: [{"uuid" => "1234", "name" => "iso", "port" => "0", "device" => "0", "controller" => "controller" }]} } + + let(:defined_disks) { [] } + + it "removes the medium from guest" do + allow(controller).to receive(:get_attachment).with(uuid: "1234").and_return(port: "0", device: "0") + expect(driver).to receive(:remove_disk).with("controller", "0", "0").and_return(true) + + subject.handle_cleanup_dvd(machine, defined_disks, disk_meta_file[:dvd]) + end + + context "when attachment is not found at the expected device" do + it "removes the disk from the correct device" do + allow(controller).to receive(:get_attachment).with(uuid: "1234").and_return(port: "0", device: "1") + expect(driver).to receive(:remove_disk).with("controller", "0", "1").and_return(true) + + subject.handle_cleanup_dvd(machine, defined_disks, disk_meta_file[:dvd]) + end + end + end end 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 05ac77e68..c06debc3d 100644 --- a/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb +++ b/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb @@ -25,8 +25,12 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do double(:state) end - let(:vm_info) { {"SATA Controller-ImageUUID-0-0" => "12345", - "SATA Controller-ImageUUID-1-0" => "67890"} } + let(:storage_controllers) { double("storage controllers") } + + let(:controller) { double("controller", name: "controller", maxportcount: 30, devices_per_port: 1, limit: 30) } + + let(: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), @@ -65,41 +69,125 @@ 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(controller).to receive(:attachments).and_return(attachments) + allow(storage_controllers).to receive(:get_controller).with(controller.name).and_return(controller) + allow(storage_controllers).to receive(:first).and_return(controller) + allow(storage_controllers).to receive(:size).and_return(1) + allow(driver).to receive(:read_storage_controllers).and_return(storage_controllers) end - context "#configure_disks" do + describe "#configure_disks" do let(:dsk_data) { {uuid: "1234", name: "disk"} } - it "configures disks and returns the disks defined" do - allow(driver).to receive(:list_hdds).and_return([]) + let(:dvd) { double("dvd", type: :dvd, name: "dvd", primary: false) } - expect(subject).to receive(:handle_configure_disk).exactly(4).and_return(dsk_data) + before do + allow(driver).to receive(:list_hdds).and_return([]) + end + + it "configures disks and returns the disks defined" do + expect(subject).to receive(:handle_configure_disk).with(machine, anything, [], controller.name). + exactly(4).and_return(dsk_data) subject.configure_disks(machine, defined_disks) end - describe "with no disks to configure" do + it "configures dvd and returns the disks defined" do + defined_disks = [ dvd ] + + expect(subject).to receive(:handle_configure_dvd).with(machine, dvd, controller.name). + and_return({}) + subject.configure_disks(machine, defined_disks) + end + + context "with no disks to configure" do let(:defined_disks) { {} } + it "returns empty hash if no disks to configure" do expect(subject.configure_disks(machine, defined_disks)).to eq({}) end end - describe "with over the disk limit for a given device" do - let(:defined_disks) { (1..31).each { |i| double("disk-#{i}") }.to_a } + # NOTE: In this scenario, one slot must be reserved for the primary + # disk, so the controller limit goes down by 1 when there is no primary + # disk defined in the config. + context "with over the disk limit for a given device" do + let(:defined_disks) { (1..controller.limit).map { |i| double("disk-#{i}", type: :disk, primary: false) }.to_a } - it "raises an exception if the disks defined exceed the limit for a SATA Controller" do + it "raises an exception if the disks defined exceed the limit" do expect{subject.configure_disks(machine, defined_disks)}. to raise_error(Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit) end end + + # hashicorp/bionic64 + context "with more than one storage controller" do + let(:controller1) { double("controller1", name: "IDE Controller", maxportcount: 2, devices_per_port: 2, limit: 4) } + let(:controller2) { double("controller2", name: "SATA Controller", maxportcount: 30, devices_per_port: 1, limit: 30) } + + before do + allow(storage_controllers).to receive(:size).and_return(2) + allow(storage_controllers).to receive(:get_controller).with(controller1.name). + and_return(controller1) + allow(storage_controllers).to receive(:get_controller).with(controller2.name). + and_return(controller2) + allow(storage_controllers).to receive(:get_dvd_controller).and_return(controller1) + allow(storage_controllers).to receive(:get_primary_controller).and_return(controller2) + end + + it "attaches disks to the primary controller" do + expect(subject).to receive(:handle_configure_disk).with(machine, anything, [], controller2.name). + exactly(4).and_return(dsk_data) + subject.configure_disks(machine, defined_disks) + end + + it "attaches dvds to the secondary controller" do + defined_disks = [ dvd ] + + expect(subject).to receive(:handle_configure_dvd).with(machine, dvd, controller1.name). + and_return({}) + subject.configure_disks(machine, defined_disks) + end + + it "raises an error if there are more than 4 dvds configured" do + defined_disks = [ + double("dvd", name: "installer1", type: :dvd, file: "installer.iso", primary: false), + double("dvd", name: "installer2", type: :dvd, file: "installer.iso", primary: false), + double("dvd", name: "installer3", type: :dvd, file: "installer.iso", primary: false), + double("dvd", name: "installer4", type: :dvd, file: "installer.iso", primary: false), + double("dvd", name: "installer5", type: :dvd, file: "installer.iso", primary: false) + ] + + expect { subject.configure_disks(machine, defined_disks) }. + to raise_error(Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit) + end + + it "attaches multiple dvds" do + defined_disks = [ + double("dvd", name: "installer1", type: :dvd, file: "installer.iso", primary: false), + double("dvd", name: "installer2", type: :dvd, file: "installer.iso", primary: false), + double("dvd", name: "installer3", type: :dvd, file: "installer.iso", primary: false), + double("dvd", name: "installer4", type: :dvd, file: "installer.iso", primary: false), + ] + + expect(subject).to receive(:handle_configure_dvd).exactly(4).times.and_return({}) + + subject.configure_disks(machine, defined_disks) + end + end end - context "#get_current_disk" do + describe "#get_current_disk" do it "gets primary disk uuid if disk to configure is primary" do + allow(storage_controllers).to receive(:get_primary_attachment).and_return(attachments[0]) primary_disk = subject.get_current_disk(machine, defined_disks.first, all_disks) expect(primary_disk).to eq(all_disks.first) end + it "raises an error if primary disk can't be found" do + allow(storage_controllers).to receive(:get_primary_attachment).and_raise(Vagrant::Errors::VirtualBoxDisksPrimaryNotFound) + expect { subject.get_current_disk(machine, defined_disks.first, all_disks) }. + to raise_error(Vagrant::Errors::VirtualBoxDisksPrimaryNotFound) + end + it "finds the disk to configure" do disk = subject.get_current_disk(machine, defined_disks[1], all_disks) expect(disk).to eq(all_disks[1]) @@ -111,8 +199,8 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do end end - context "#handle_configure_disk" do - describe "when creating a new disk" do + describe "#handle_configure_disk" do + context "when creating a new disk" do let(:all_disks) { [{"UUID"=>"12345", "Parent UUID"=>"base", "State"=>"created", @@ -123,17 +211,28 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do "Capacity"=>"65536 MBytes", "Encryption"=>"disabled"}] } - let(:disk_meta) { {uuid: "67890", name: "disk-0"} } + let(:disk_meta) { {uuid: "67890", name: "disk-0", controller: "controller", port: "1", device: "1"} } it "creates a new disk if it doesn't yet exist" do - expect(subject).to receive(:create_disk).with(machine, defined_disks[1]) + expect(subject).to receive(:create_disk).with(machine, defined_disks[1], controller) .and_return(disk_meta) - subject.handle_configure_disk(machine, defined_disks[1], all_disks) + subject.handle_configure_disk(machine, defined_disks[1], all_disks, controller.name) + end + + it "includes disk attachment info in metadata" do + expect(subject).to receive(:create_disk).with(machine, defined_disks[1], controller) + .and_return(disk_meta) + + disk_metadata = subject.handle_configure_disk(machine, defined_disks[1], all_disks, controller.name) + expect(disk_metadata).to have_key(:controller) + expect(disk_metadata).to have_key(:port) + expect(disk_metadata).to have_key(:device) + expect(disk_metadata).to have_key(:name) end end - describe "when a disk needs to be resized" do + context "when a disk needs to be resized" do let(:all_disks) { [{"UUID"=>"12345", "Parent UUID"=>"base", "State"=>"created", @@ -161,13 +260,13 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do with(machine, defined_disks[1], all_disks[1]).and_return(true) expect(subject).to receive(:resize_disk). - with(machine, defined_disks[1], all_disks[1]).and_return(true) + with(machine, defined_disks[1], all_disks[1], controller).and_return({}) - subject.handle_configure_disk(machine, defined_disks[1], all_disks) + subject.handle_configure_disk(machine, defined_disks[1], all_disks, controller.name) end end - describe "if no additional disk configuration is required" do + context "if no additional disk configuration is required" do let(:all_disks) { [{"UUID"=>"12345", "Parent UUID"=>"base", "State"=>"created", @@ -199,11 +298,13 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do expect(driver).to receive(:get_port_and_device).with("67890"). and_return({}) - expect(driver).to receive(:attach_disk).with((disk_info[:port].to_i + 1).to_s, + expect(driver).to receive(:attach_disk).with(controller.name, + (disk_info[:port].to_i + 1).to_s, disk_info[:device], + "hdd", all_disks[1]["Location"]) - subject.handle_configure_disk(machine, defined_disks[1], all_disks) + subject.handle_configure_disk(machine, defined_disks[1], all_disks, controller.name) end it "does nothing if all disks are properly configured" do @@ -216,12 +317,12 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do expect(driver).to receive(:get_port_and_device).with("67890"). and_return(disk_info) - subject.handle_configure_disk(machine, defined_disks[1], all_disks) + subject.handle_configure_disk(machine, defined_disks[1], all_disks, controller.name) end end end - context "#compare_disk_size" do + describe "#compare_disk_size" do let(:disk_config_small) { double("disk", name: "disk-0", size: 1073741824.0, primary: false, type: :disk) } let(:disk_config_large) { double("disk", name: "disk-0", size: 68719476736.0, primary: false, type: :disk) } @@ -235,7 +336,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do end end - context "#create_disk" do + describe "#create_disk" do let(:disk_config) { double("disk", name: "disk-0", size: 1073741824.0, primary: false, type: :disk, disk_ext: "vdi", provider_config: nil) } @@ -251,27 +352,67 @@ 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, controller). and_return(port_and_device) - expect(driver).to receive(:attach_disk).with(port_and_device[:port], + expect(driver).to receive(:attach_disk).with(controller.name, + port_and_device[:port], port_and_device[:device], + "hdd", disk_file) - subject.create_disk(machine, disk_config) + subject.create_disk(machine, disk_config, controller) end end - context "#get_next_port" do - it "determines the next available port to use" do - dsk_info = subject.get_next_port(machine) - expect(dsk_info[:device]).to eq("0") + describe ".get_next_port" do + it "determines the next available port and device to use" do + dsk_info = subject.get_next_port(machine, controller) expect(dsk_info[:port]).to eq("2") + expect(dsk_info[:device]).to eq("0") + end + + context "with IDE controller" do + let(:controller) { + double("controller", name: "IDE", maxportcount: 2, devices_per_port: 2, limit: 4) + } + + let(:attachments) { [] } + + it "attaches to port 0, device 0" do + dsk_info = subject.get_next_port(machine, controller) + expect(dsk_info[:port]).to eq("0") + expect(dsk_info[:device]).to eq("0") + end + + context "with 1 device" do + let(:attachments) { [{port:"0", device: "0"}] } + + it "attaches to the next device on that port" do + dsk_info = subject.get_next_port(machine, controller) + expect(dsk_info[:port]).to eq("0") + expect(dsk_info[:device]).to eq("1") + end + end + end + + context "with SCSI controller" do + let(:controller) { + double("controller", name: "SCSI", maxportcount: 16, devices_per_port: 1, limit: 16) + } + + let(:attachments) { [] } + + it "determines the next available port and device to use" do + dsk_info = subject.get_next_port(machine, controller) + expect(dsk_info[:port]).to eq("0") + expect(dsk_info[:device]).to eq("0") + end end end - context "#resize_disk" do - describe "when a disk is vmdk format" do + describe "#resize_disk" do + context "when a disk is vmdk format" do let(:disk_config) { double("disk", name: "vagrant_primary", size: 1073741824.0, primary: false, type: :disk, disk_ext: "vmdk", provider_config: nil) } @@ -289,10 +430,9 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do expect(driver).to receive(:vmdk_to_vdi).with(all_disks[0]["Location"]). and_return(vdi_disk_file) - expect(driver).to receive(:resize_disk).with(vdi_disk_file, disk_config.size.to_i). - and_return(true) + 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(controller.name, attach_info[:port], attach_info[:device]). and_return(true) expect(driver).to receive(:close_medium).with("12345") @@ -300,7 +440,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do and_return(vmdk_disk_file) expect(driver).to receive(:attach_disk). - with(attach_info[:port], attach_info[:device], vmdk_disk_file, "hdd").and_return(true) + with(controller.name, attach_info[:port], attach_info[:device], "hdd", vmdk_disk_file).and_return(true) expect(driver).to receive(:close_medium).with(vdi_disk_file).and_return(true) expect(driver).to receive(:list_hdds).and_return(all_disks) @@ -308,7 +448,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do expect(FileUtils).to receive(:remove).with("#{vmdk_disk_file}.backup", force: true). and_return(true) - subject.resize_disk(machine, disk_config, all_disks[0]) + subject.resize_disk(machine, disk_config, all_disks[0], controller) end it "reattaches original disk if something goes wrong" do @@ -321,45 +461,87 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do expect(driver).to receive(:vmdk_to_vdi).with(all_disks[0]["Location"]). and_return(vdi_disk_file) - expect(driver).to receive(:resize_disk).with(vdi_disk_file, disk_config.size.to_i). - and_return(true) + 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(controller.name, attach_info[:port], attach_info[:device]). and_return(true) expect(driver).to receive(:close_medium).with("12345") allow(driver).to receive(:vdi_to_vmdk).and_raise(StandardError) - expect(FileUtils).to receive(:mv).with("#{vmdk_disk_file}.backup", vmdk_disk_file, force: true). - and_return(true) + expect(subject).to receive(:recover_from_resize).with(machine, attach_info, "#{vmdk_disk_file}.backup", all_disks[0], vdi_disk_file, controller) - expect(driver).to receive(:attach_disk). - with(attach_info[:port], attach_info[:device], vmdk_disk_file, "hdd").and_return(true) - expect(driver).to receive(:close_medium).with(vdi_disk_file).and_return(true) - - expect{subject.resize_disk(machine, disk_config, all_disks[0])}.to raise_error(Exception) + expect{subject.resize_disk(machine, disk_config, all_disks[0], controller)}.to raise_error(Exception) end end - describe "when a disk is vdi format" do + context "when a disk is vdi format" do let(:disk_config) { double("disk", name: "disk-0", size: 1073741824.0, primary: false, type: :disk, disk_ext: "vdi", provider_config: nil) } it "resizes the disk" do expect(driver).to receive(:resize_disk).with(all_disks[1]["Location"], disk_config.size.to_i) - subject.resize_disk(machine, disk_config, all_disks[1]) + expect(driver).to receive(:get_port_and_device).with(all_disks[1]["UUID"]). + and_return({port: "1", device: "0"}) + + subject.resize_disk(machine, disk_config, all_disks[1], controller) end end end - context "#vmdk_to_vdi" do + describe "#vmdk_to_vdi" do it "converts a disk from vmdk to vdi" do end end - context "#vdi_to_vmdk" do + describe "#vdi_to_vmdk" do it "converts a disk from vdi to vmdk" do end end + + describe ".recover_from_resize" do + let(:disk_config) { double("disk", name: "vagrant_primary", size: 1073741824.0, + primary: false, type: :disk, disk_ext: "vmdk", + provider_config: nil) } + let(:attach_info) { {port: "0", device: "0"} } + let(:vdi_disk_file) { "/home/vagrant/VirtualBox VMs/ubuntu-18.04-amd64-disk001.vdi" } + let(:vmdk_disk_file) { "/home/vagrant/VirtualBox VMs/ubuntu-18.04-amd64-disk001.vmdk" } + let(:vmdk_backup_file) { "/home/vagrant/VirtualBox VMs/ubuntu-18.04-amd64-disk001.vmdk.backup" } + + it "reattaches the original disk file and closes the cloned medium" do + expect(FileUtils).to receive(:mv).with(vmdk_backup_file, vmdk_disk_file, force: true). + and_return(true) + + expect(driver).to receive(:attach_disk). + with(controller.name, attach_info[:port], attach_info[:device], "hdd", vmdk_disk_file).and_return(true) + + expect(driver).to receive(:close_medium).with(vdi_disk_file).and_return(true) + + subject.recover_from_resize(machine, attach_info, vmdk_backup_file, all_disks[0], vdi_disk_file, controller) + end + end + + describe ".handle_configure_dvd" do + let(:dvd_config) { double("dvd", file: "/tmp/untitled.iso", name: "dvd1", primary: false) } + + before do + allow(subject).to receive(:get_next_port).with(machine, controller). + and_return({device: "0", port: "0"}) + allow(controller).to receive(:attachments).and_return( + [port: "0", device: "0", uuid: "12345"] + ) + end + + it "includes disk attachment info in metadata" do + expect(driver).to receive(:attach_disk).with(controller.name, "0", "0", "dvddrive", "/tmp/untitled.iso") + + dvd_metadata = subject.handle_configure_dvd(machine, dvd_config, controller.name) + expect(dvd_metadata[:uuid]).to eq("12345") + expect(dvd_metadata).to have_key(:controller) + expect(dvd_metadata).to have_key(:port) + expect(dvd_metadata).to have_key(:device) + expect(dvd_metadata).to have_key(:name) + 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 d094028ca..d10bfb4bc 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 @@ -5,6 +5,7 @@ describe VagrantPlugins::ProviderVirtualBox::Driver::Version_5_0 do include_context "virtualbox" let(:vbox_version) { "5.0.0" } + let(:controller_name) { "controller" } subject { VagrantPlugins::ProviderVirtualBox::Driver::Version_5_0.new(uuid) } @@ -90,4 +91,53 @@ OUTPUT end end end + + describe "#attach_disk" do + it "attaches a device to the specified controller" do + expect(subject).to receive(:execute) do |*args| + storagectl = args[args.index("--storagectl") + 1] + expect(storagectl).to eq(controller_name) + end + subject.attach_disk(controller_name, anything, anything, anything, anything) + end + end + + describe "#remove_disk" 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(controller_name) + end + subject.remove_disk(controller_name, anything, anything) + end + end + + describe "#read_storage_controllers" do + before do + allow(subject).to receive(:show_vm_info).and_return( + { "storagecontrollername0" => "SATA Controller", + "storagecontrollertype0" => "IntelAhci", + "storagecontrollermaxportcount0" => "30", + "SATA Controller-0-0" => "/tmp/primary.vdi", + "SATA Controller-ImageUUID-0-0" => "12345", + "SATA Controller-1-0" => "/tmp/secondary.vdi", + "SATA Controller-ImageUUID-1-0" => "67890" } + ) + end + + it "returns a list of storage controllers" do + storage_controllers = subject.read_storage_controllers + + expect(storage_controllers.first.name).to eq("SATA Controller") + expect(storage_controllers.first.type).to eq("IntelAhci") + expect(storage_controllers.first.maxportcount).to eq(30) + end + + it "includes attachments for each storage controller" do + storage_controllers = subject.read_storage_controllers + + expect(storage_controllers.first.attachments).to include(port: "0", device: "0", uuid: "12345", location: "/tmp/primary.vdi") + expect(storage_controllers.first.attachments).to include(port: "1", device: "0", uuid: "67890", location: "/tmp/secondary.vdi") + end + end end diff --git a/test/unit/plugins/providers/virtualbox/model/storage_controller_array_test.rb b/test/unit/plugins/providers/virtualbox/model/storage_controller_array_test.rb new file mode 100644 index 000000000..a0b6d66af --- /dev/null +++ b/test/unit/plugins/providers/virtualbox/model/storage_controller_array_test.rb @@ -0,0 +1,131 @@ +require File.expand_path("../../base", __FILE__) + +describe VagrantPlugins::ProviderVirtualBox::Model::StorageControllerArray do + include_context "unit" + + let(:controller1) { double("controller1", name: "IDE Controller", supported?: true, boot_priority: 1) } + let(:controller2) { double("controller2", name: "SATA Controller", supported?: true, boot_priority: 2) } + + let(:primary_disk) { {location: "/tmp/primary.vdi"} } + + before do + subject.replace([controller1, controller2]) + end + + describe "#get_controller" do + it "gets a controller by name" do + expect(subject.get_controller("IDE Controller")).to eq(controller1) + end + + it "raises an exception if a matching storage controller can't be found" do + expect { subject.get_controller(name: "Foo Controller") }. + to raise_error(Vagrant::Errors::VirtualBoxDisksControllerNotFound) + end + end + + describe "#get_primary_controller" do + context "with a single supported controller" do + before do + subject.replace([controller1]) + allow(controller1).to receive(:attachments).and_return([primary_disk]) + end + + it "returns the controller" do + expect(subject.get_primary_controller).to eq(controller1) + end + end + + context "with multiple controllers" do + before do + allow(controller1).to receive(:attachments).and_return([]) + allow(controller2).to receive(:attachments).and_return([primary_disk]) + end + + it "returns the first supported controller with a disk attached" do + expect(subject.get_primary_controller).to eq(controller2) + end + + it "raises an error if the primary disk is attached to an unsupported controller" do + allow(controller2).to receive(:supported?).and_return(false) + + expect { subject.get_primary_controller }. + to raise_error(Vagrant::Errors::VirtualBoxDisksNoSupportedControllers) + end + end + end + + describe "#hdd?" do + let(:attachment) { {} } + it "determines whether the given attachment represents a hard disk" do + expect(subject.send(:hdd?, attachment)).to be(false) + end + + it "returns true for disk files ending in compatible extensions" do + attachment[:location] = "/tmp/primary.vdi" + expect(subject.send(:hdd?, attachment)).to be(true) + end + + it "is case insensitive" do + attachment[:location] = "/tmp/PRIMARY.VDI" + expect(subject.send(:hdd?, attachment)).to be(true) + end + end + + describe "#get_primary_attachment" do + let(:attachment) { {location: "/tmp/primary.vdi"} } + + before do + allow(subject).to receive(:get_primary_controller).and_return(controller2) + end + + it "returns the first attachment on the primary controller" do + allow(controller2).to receive(:get_attachment).with(port: "0", device: "0").and_return(attachment) + expect(subject.get_primary_attachment).to be(attachment) + end + + it "raises an exception if no attachment exists at port 0, device 0" do + allow(controller2).to receive(:get_attachment).with(port: "0", device: "0").and_return(nil) + expect { subject.get_primary_attachment }.to raise_error(Vagrant::Errors::VirtualBoxDisksPrimaryNotFound) + end + end + + describe "#get_dvd_controller" do + context "with one controller" do + let(:controller) { double("controller", supported?: true) } + + before do + subject.replace([controller]) + end + + it "returns the controller" do + expect(subject.get_dvd_controller).to be(controller) + end + + it "raises an exception if the controller is unsupported" do + allow(controller).to receive(:supported?).and_return(false) + + expect { subject.get_dvd_controller }.to raise_error(Vagrant::Errors::VirtualBoxDisksNoSupportedControllers) + end + end + + context "with multiple controllers" do + let(:controller1) { double("controller", supported?: true, boot_priority: 2) } + let(:controller2) { double("controller", supported?: true, boot_priority: 1) } + + before do + subject.replace([controller1, controller2]) + end + + it "returns the first supported controller" do + expect(subject.get_dvd_controller).to be(controller2) + end + + it "raises an exception if no controllers are supported" do + allow(controller1).to receive(:supported?).and_return(false) + allow(controller2).to receive(:supported?).and_return(false) + + expect { subject.get_dvd_controller }.to raise_error(Vagrant::Errors::VirtualBoxDisksNoSupportedControllers) + end + end + end +end diff --git a/test/unit/plugins/providers/virtualbox/model/storage_controller_test.rb b/test/unit/plugins/providers/virtualbox/model/storage_controller_test.rb new file mode 100644 index 000000000..918f4815b --- /dev/null +++ b/test/unit/plugins/providers/virtualbox/model/storage_controller_test.rb @@ -0,0 +1,85 @@ +require File.expand_path("../../base", __FILE__) + +describe VagrantPlugins::ProviderVirtualBox::Model::StorageController do + include_context "unit" + + let(:name) {} + let(:type) { "IntelAhci" } + let(:maxportcount) { 30 } + let(:attachments) {} + + subject { described_class.new(name, type, maxportcount, attachments) } + + describe "#initialize" do + context "with SATA controller type" do + it "recognizes a SATA controller" do + expect(subject.sata?).to be(true) + end + + it "calculates the maximum number of attachments" do + expect(subject.limit).to eq(30) + end + + it "sets the boot priority" do + expect(subject.boot_priority).to eq(2) + end + end + + context "with IDE controller type" do + let(:type) { "PIIX4" } + let(:maxportcount) { 2 } + + it "recognizes an IDE controller" do + expect(subject.ide?).to be(true) + end + + it "calculates the maximum number of attachments" do + expect(subject.limit).to eq(4) + end + + it "sets the boot priority" do + expect(subject.boot_priority).to eq(1) + end + end + + context "with SCSI controller type" do + let(:type) { "LsiLogic" } + let(:maxportcount) { 16 } + + it "recognizes an SCSI controller" do + expect(subject.scsi?).to be(true) + end + + it "calculates the maximum number of attachments" do + expect(subject.limit).to eq(16) + end + + it "sets the boot priority" do + expect(subject.boot_priority).to eq(3) + end + end + + context "with some other type" do + let(:type) { "foo" } + + it "is unknown" do + expect(subject.ide?).to be(false) + expect(subject.sata?).to be(false) + end + end + end + + describe "#supported?" do + it "returns true if the controller type is supported" do + expect(subject.supported?).to be(true) + end + + context "with unsupported type" do + let(:type) { "foo" } + + it "returns false" do + expect(subject.supported?).to be(false) + end + end + end +end diff --git a/website/pages/docs/boxes/index.mdx b/website/pages/docs/boxes/index.mdx index eb9157985..f2da40784 100644 --- a/website/pages/docs/boxes/index.mdx +++ b/website/pages/docs/boxes/index.mdx @@ -60,7 +60,7 @@ with third-party published boxes. ## Official Boxes -HashiCorp (the makers of Vagrant) publish a basic Ubuntu 18.04 64-bit box that is available for minimal use cases. It is highly optimized, small in size, and includes support for Virtualbox, Hyper-V, and VMware. You can use it like this: +HashiCorp (the makers of Vagrant) publish a basic Ubuntu 18.04 64-bit box that is available for minimal use cases. It is highly optimized, small in size, and includes support for VirtualBox, Hyper-V, and VMware. You can use it like this: ```shell-session $ vagrant init hashicorp/bionic64 @@ -74,10 +74,10 @@ Vagrant.configure("2") do |config| end ``` -For other users, we recommend the [Bento boxes](https://vagrantcloud.com/bento). The Bento boxes are [open source](https://github.com/chef/bento) and built for a number of providers including VMware, Virtualbox, and Parallels. There are a variety of operating systems and versions available. +For other users, we recommend the [Bento boxes](https://vagrantcloud.com/bento). The Bento boxes are [open source](https://github.com/chef/bento) and built for a number of providers including VMware, VirtualBox, and Parallels. There are a variety of operating systems and versions available. These are the only two officially-recommended box sets. Special thanks to the Bento project for providing a solid base template for the `hashicorp/bionic64` box. -~> **It is often a point of confusion**, but Canonical (the company that makes the Ubuntu operating system) publishes boxes under the "ubuntu" namespace on Vagrant Cloud. These boxes only support Virtualbox and do not provide an ideal experience for most users. If you encounter issues with these boxes, please try the Bento boxes instead. +~> **It is often a point of confusion**, but Canonical (the company that makes the Ubuntu operating system) publishes boxes under the "ubuntu" namespace on Vagrant Cloud. These boxes only support VirtualBox and do not provide an ideal experience for most users. If you encounter issues with these boxes, please try the Bento boxes instead. diff --git a/website/pages/docs/disks/configuration.mdx b/website/pages/docs/disks/configuration.mdx index 59650e651..f0e657512 100644 --- a/website/pages/docs/disks/configuration.mdx +++ b/website/pages/docs/disks/configuration.mdx @@ -14,13 +14,21 @@ Vagrant Disks has several options that allow users to define and attach disks to - `disk_ext` (string) - Optional argument that defines what kind of file extension a disk should have. Defaults to `"vdi"` if unspecified. For a list of supported disk extensions, please check the specific provider being used. -- `file` (string) - Optional argument that defines a path on disk pointing to - the location of a disk file that already exists. -- `name` (string) - Required option to give the disk a name. This name will be - used as the filename when creating the disk. -- `primary` (boolean) - Optional argument that configures a given disk to be the - "primary" disk to manage on the guest. There can only be one `primary` disk per guest. - Defaults to `false` if not specified. + Not used for type `:dvd.` + +- `file` (string) - For type `:dvd`, this is a required argument that should + point to an `.iso` file on the host machine. For type `:disk`, this is an + optional argument that can point to the location of a disk file that already + exists. + +- `name` (string) - Required option to give the disk a name. This name will + also be used as the filename when creating a virtual hard disk. + +- `primary` (boolean) - Optional argument that configures a given disk to be + the "primary" disk to manage on the guest. There can only be one `primary` + disk per guest, and it must be of type `:disk`. Defaults to `false` if not + specified. + - `provider_config` (hash) - Additional provider specific options for managing a given disk. Please refer to the provider specific documentation to see any available provider_config options. @@ -31,7 +39,8 @@ Vagrant Disks has several options that allow users to define and attach disks to - `{providername: {diskoption: value}, otherprovidername: {diskoption: value}` - A hash where the top level key(s) are one or more providers, and each provider keys values are a hash of options and their values. -- `size` (String) - The size of the disk to create. For example, `"10GB"`. +- `size` (String) - The size of the disk to create. For example, `"10GB"`. Not + used for type `:dvd.` **Note:** More specific examples of these can be found under the provider specific disk page. The `provider_config` option will depend on the provider @@ -53,6 +62,7 @@ You can set a disk type with the first argument of a disk config in your Vagrant ```ruby config.vm.disk :disk, name: "backup", size: "10GB" +config.vm.disk :dvd, name: "installer", path: "./installer.iso" config.vm.disk :floppy, name: "cool_files" ``` diff --git a/website/pages/docs/disks/usage.mdx b/website/pages/docs/disks/usage.mdx index 1bedbf6e9..8e78204af 100644 --- a/website/pages/docs/disks/usage.mdx +++ b/website/pages/docs/disks/usage.mdx @@ -54,7 +54,7 @@ end It should be noted that due to how VirtualBox functions, it is not possible to shrink the size of a disk. -### Attaching new disks +### Attaching new hard disks Vagrant can attach multiple disks to a guest using the VirtualBox provider. An example of attaching a single disk to a guest with 10 GB of storage can be found below: @@ -86,12 +86,33 @@ Vagrant.configure("2") do |config| end ``` -Note: Virtualbox only allows for up to 30 disks to be attached to a given SATA Controller, -and this number includes the primary disk! Attempting to configure more than 30 will -result in a Vagrant error. +Note: VirtualBox has a hard limit on the number of disks that can be attached +to a given storage controller, which is defined by the controller type. +Attempting to configure more disks than are supported by the primary +controller will result in a Vagrant error. + +### Attaching optical drives + +Vagrant can attach `.iso` files as optical drives using the VirtualBox provider. +An example of attaching an optical drive to a guest can be found below: + +```ruby +Vagrant.configure("2") do |config| + config.vm.define "hashicorp" do |h| + h.vm.box = "hashicorp/bionic64" + h.vm.provider :virtualbox + + h.vm.disk :dvd, name: "installer", file: "./installer.iso" + end +end +``` + +As with hard disks, configuring more disks than are supported by your VM's +storage controller arrangement will result in a Vagrant error. ### Removing Disks -If you have removed a disk from your Vagrant config and wish for it to be detached from the guest, -you will need to `vagrant reload` your guest to apply these changes. **NOTE:** Doing so -will also delete the medium from your hard drive. +If you have removed a disk from your Vagrant config and wish for it to be +detached from the guest, you will need to `vagrant reload` your guest to apply +these changes. **NOTE:** Removing virtual hard disks created by Vagrant will +also delete the medium from your hard drive. diff --git a/website/pages/docs/disks/virtualbox/common-issues.mdx b/website/pages/docs/disks/virtualbox/common-issues.mdx index db8a6dd7e..fdcd8aab5 100644 --- a/website/pages/docs/disks/virtualbox/common-issues.mdx +++ b/website/pages/docs/disks/virtualbox/common-issues.mdx @@ -21,10 +21,21 @@ you should see a list of disks attached to your guest. ## How many disks can I attach? -Vagrant attaches all new disks defined to a guests SATA Controller. As of VirtualBox 6.1.x, -SATA Controllers can only support up to **30 disks** per guest. Therefore if you try -to define and attach more than 30, it will result in an error. This number _includes_ -the primary disk for the guest. +Vagrant attaches all new disks defined to guest's primary controller. As of +VirtualBox 6.1.x, storage controllers have the following limits to the number +of disks that are supported per guest: + +- IDE Controllers: 4 +- SATA Controllers: 30 +- SCSI Controllers: 16 + +Therefore if your primary disk is attached to a SATA Controller and you try to +define and attach more than 30, it will result in an error. This number +_includes_ the primary disk for the guest. + +DVD attachments are subject to the same limits. Optical disk attachments will +be attached to the storage controller with the highest boot priority (usually +the IDE controller). ## Resizing VMDK format disks diff --git a/website/pages/docs/disks/virtualbox/index.mdx b/website/pages/docs/disks/virtualbox/index.mdx index 8f0241767..d6e71af95 100644 --- a/website/pages/docs/disks/virtualbox/index.mdx +++ b/website/pages/docs/disks/virtualbox/index.mdx @@ -31,9 +31,25 @@ off for any changes to be applied to a guest. If you make a configuration change with a guests disk, you will need to `vagrant reload` the guest for any changes to be applied. -When new disks are defined to be attached to a guest, Vagrant will create and attach -these disks to a guests SATA Controller. It should be noted that up to 30 disks -can be attached to the SATA Controller. +When new disks are defined to be attached to a guest, Vagrant will attach +disks to a particular storage controller based on the type of disk configured: + +- For the `:disk` type, Vagrant will use the storage controller containing the + boot disk. It will also create the disk if necessary. +- For the `:dvd` type, Vagrant will attach the disk to the storage controller + that comes earliest in the machine's boot order. For example, if a VM has a + SATA controller and an IDE controller, the disk will be attached to the IDE + controller. + +Vagrant will not be able to configure disks of a given type if the associated +storage controller does not exist. In this case, you may use +[provider-specific customizations](/docs/providers/virtualbox/configuration#vboxmanage-customizations) +to add a required storage controller. + +It should also be noted that storage controllers have different limits for the +number of disks that can be attached. Attempting to configure more than the +maximum number of disks for a storage controller type will result in a Vagrant +error. For more information on how to use VirtualBox to configure disks for a guest, refer to the [general usage](/docs/disks/usage) and [configuration](/docs/disks/configuration)