From fc54996ba892c1230ca05cedde0d4e20a362db51 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 5 Dec 2019 16:08:49 -0800 Subject: [PATCH 001/127] Add beginning of ConfigureDisks capability to VirtualBox provider --- plugins/providers/virtualbox/cap/configure_disks.rb | 11 +++++++++++ plugins/providers/virtualbox/plugin.rb | 5 +++++ 2 files changed, 16 insertions(+) create mode 100644 plugins/providers/virtualbox/cap/configure_disks.rb diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb new file mode 100644 index 000000000..902ac9944 --- /dev/null +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -0,0 +1,11 @@ +module VagrantPlugins + module ProviderVirtualBox + module Cap + module ConfigureDisks + def self.configure_disks(machine, defined_disks) + return nil if defined_disks.empty? + end + end + end + end +end diff --git a/plugins/providers/virtualbox/plugin.rb b/plugins/providers/virtualbox/plugin.rb index f2fbf47e5..cb0b02725 100644 --- a/plugins/providers/virtualbox/plugin.rb +++ b/plugins/providers/virtualbox/plugin.rb @@ -39,6 +39,11 @@ module VagrantPlugins Cap::PublicAddress end + provider_capability(:virtualbox, :configure_disks) do + require_relative "cap/configure_disks" + Cap::ConfigureDisks + end + provider_capability(:virtualbox, :snapshot_list) do require_relative "cap" Cap From 00b47a285ee6e717d54bd909e36dd968569b1d94 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 5 Dec 2019 17:19:57 -0800 Subject: [PATCH 002/127] Ensure decimal points are respected for numeric conversions --- lib/vagrant/util/numeric.rb | 12 ++++++------ test/unit/vagrant/util/numeric_test.rb | 6 ++++++ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/vagrant/util/numeric.rb b/lib/vagrant/util/numeric.rb index 3a2989d97..5f1312702 100644 --- a/lib/vagrant/util/numeric.rb +++ b/lib/vagrant/util/numeric.rb @@ -6,12 +6,12 @@ module Vagrant # Authors Note: This conversion has been borrowed from the ActiveSupport Numeric class # Conversion helper constants - KILOBYTE = 1024 - MEGABYTE = KILOBYTE * 1024 - GIGABYTE = MEGABYTE * 1024 - TERABYTE = GIGABYTE * 1024 - PETABYTE = TERABYTE * 1024 - EXABYTE = PETABYTE * 1024 + KILOBYTE = 1024.0 + MEGABYTE = KILOBYTE * 1024.0 + GIGABYTE = MEGABYTE * 1024.0 + TERABYTE = GIGABYTE * 1024.0 + PETABYTE = TERABYTE * 1024.0 + EXABYTE = PETABYTE * 1024.0 BYTES_CONVERSION_MAP = {KB: KILOBYTE, MB: MEGABYTE, GB: GIGABYTE, TB: TERABYTE, PB: PETABYTE, EB: EXABYTE} diff --git a/test/unit/vagrant/util/numeric_test.rb b/test/unit/vagrant/util/numeric_test.rb index b13e4f9eb..2315f5f26 100644 --- a/test/unit/vagrant/util/numeric_test.rb +++ b/test/unit/vagrant/util/numeric_test.rb @@ -18,4 +18,10 @@ describe Vagrant::Util::Numeric do expect(bytes).to eq(nil) end end + + describe "bytes to megabytes" do + it "converts bytes to megabytes" do + expect(subject.bytes_to_megabytes(1000000)).to eq(0.95) + end + end end From 5273ba959059339cf291d9e60225d1f40dc2c360 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 5 Dec 2019 17:38:44 -0800 Subject: [PATCH 003/127] Add method for converting bytes to megabytes --- lib/vagrant/util/numeric.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/vagrant/util/numeric.rb b/lib/vagrant/util/numeric.rb index 5f1312702..31cdd75d5 100644 --- a/lib/vagrant/util/numeric.rb +++ b/lib/vagrant/util/numeric.rb @@ -49,6 +49,14 @@ module Vagrant bytes end + # Rounds actual value to two decimal places + # + # @param [Integer] bytes + # @return [Integer] megabytes - bytes representation in megabytes + def bytes_to_megabytes(bytes) + (bytes / MEGABYTE).round(2) + end + # @private # Reset the cached values for platform. This is not considered a public # API and should only be used for testing. From 55cb8d1f0a8e68397ea8cfb01ace5fa34677e2dc Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 5 Dec 2019 17:39:19 -0800 Subject: [PATCH 004/127] Add configure_disk blank unit test file and some method docs --- .../virtualbox/cap/configure_disks.rb | 4 +++ .../virtualbox/cap/configure_disks_test.rb | 27 +++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 902ac9944..8b98045d9 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -1,7 +1,11 @@ +require "vagrant/util/numeric" + module VagrantPlugins module ProviderVirtualBox module Cap module ConfigureDisks + # @param [Vagrant::Machine] machine + # @param [VagrantPlugins::Kernel_V2::VagrantConfigDisk] defined_disks def self.configure_disks(machine, defined_disks) return nil if defined_disks.empty? 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 new file mode 100644 index 000000000..2581c7f2b --- /dev/null +++ b/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb @@ -0,0 +1,27 @@ +require_relative "../base" + +require Vagrant.source_root.join("plugins/providers/virtualbox/cap/configure_disks") + +describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do + include_context "unit" + + let(:iso_env) do + # We have to create a Vagrantfile so there is a root path + env = isolated_environment + env.vagrantfile("") + env.create_vagrant_env + end + + let(:machine) do + iso_env.machine(iso_env.machine_names[0], :dummy).tap do |m| + allow(m).to receive(:state).and_return(state) + end + end + + let(:state) do + double(:state) + end + + describe "#configure_disks" do + end +end From 9bf41d34babbb135a6dd65ecc475d548330b7eb5 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 6 Dec 2019 16:47:49 -0800 Subject: [PATCH 005/127] Add some basic driver methods for configuring disks --- .../providers/virtualbox/cap/configure_disks.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 8b98045d9..f45f24b4e 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -9,6 +9,20 @@ module VagrantPlugins def self.configure_disks(machine, defined_disks) return nil if defined_disks.empty? end + + protected + + # Maybe move these into the virtualbox driver?? + # Versioning might be an issue :shrug: + + def current_vm_disks(driver) + end + + def vmdk_to_vdi(driver) + end + + def vdi_to_vmdk(driver) + end end end end From 16cccd504a05ad2effd9b1da9851cf7f81bcd2f5 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 10 Dec 2019 11:05:22 -0800 Subject: [PATCH 006/127] Add virtualbox driver method for obtaining vm information --- .../providers/virtualbox/cap/configure_disks.rb | 9 ++++++--- plugins/providers/virtualbox/driver/base.rb | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index f45f24b4e..2074d1a55 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -8,6 +8,7 @@ module VagrantPlugins # @param [VagrantPlugins::Kernel_V2::VagrantConfigDisk] defined_disks def self.configure_disks(machine, defined_disks) return nil if defined_disks.empty? + disks = current_vm_disks(machine) end protected @@ -15,13 +16,15 @@ module VagrantPlugins # Maybe move these into the virtualbox driver?? # Versioning might be an issue :shrug: - def current_vm_disks(driver) + def self.current_vm_disks(machine) + disks = {} + info = machine.provider.driver.show_vm_info(machine.id) end - def vmdk_to_vdi(driver) + def self.vmdk_to_vdi(driver) end - def vdi_to_vmdk(driver) + def self.vdi_to_vmdk(driver) end end end diff --git a/plugins/providers/virtualbox/driver/base.rb b/plugins/providers/virtualbox/driver/base.rb index 8f07c4bac..23f29cd6d 100644 --- a/plugins/providers/virtualbox/driver/base.rb +++ b/plugins/providers/virtualbox/driver/base.rb @@ -368,6 +368,21 @@ module VagrantPlugins def vm_exists?(uuid) end + # Returns a hash of information about a given virtual machine + # + # @param [String] uuid + # @return [Hash] info + def show_vm_info(uuid) + info = {} + execute('showvminfo', uuid, '--machinereadable', retryable: true).split("\n").each do |line| + parts = line.partition('=') + key = parts.first.gsub('"', '') + value = parts.last.gsub('"', '') + info[key] = value + end + info + end + # Execute the given subcommand for VBoxManage and return the output. def execute(*command, &block) # Get the options hash if it exists From 5be7989f2d6228977ebf030c6b181aafe0b0848c Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 10 Dec 2019 16:00:27 -0800 Subject: [PATCH 007/127] Add basic driver methods for disk operations --- .../virtualbox/cap/configure_disks.rb | 20 ++++---- plugins/providers/virtualbox/driver/base.rb | 49 +++++++++++++++++++ 2 files changed, 59 insertions(+), 10 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 2074d1a55..102a51159 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -7,20 +7,20 @@ module VagrantPlugins # @param [Vagrant::Machine] machine # @param [VagrantPlugins::Kernel_V2::VagrantConfigDisk] defined_disks def self.configure_disks(machine, defined_disks) - return nil if defined_disks.empty? - disks = current_vm_disks(machine) + return if defined_disks.empty? + + current_disks = machine.provider.driver.list_hdds + # Compare current disks to config, and if there's a delta, adjust accordingly + # + # Compare by name if possible + defined_disks.each do |disk| + if disk.type == :disk + end + end end protected - # Maybe move these into the virtualbox driver?? - # Versioning might be an issue :shrug: - - def self.current_vm_disks(machine) - disks = {} - info = machine.provider.driver.show_vm_info(machine.id) - end - def self.vmdk_to_vdi(driver) end diff --git a/plugins/providers/virtualbox/driver/base.rb b/plugins/providers/virtualbox/driver/base.rb index 23f29cd6d..cbd4baf93 100644 --- a/plugins/providers/virtualbox/driver/base.rb +++ b/plugins/providers/virtualbox/driver/base.rb @@ -383,6 +383,55 @@ module VagrantPlugins info end + # @return [Array] hdds An array of hashes of harddrive info for a guest + def list_hdds + hdds = [] + tmp_drive = {} + execute('list', 'hdds', retryable: true).split("\n").each do |line| + if line == "" # separator between disks + hdds << tmp_drive + tmp_drive = {} + next + end + parts = line.partition(":") + key = parts.first.strip + value = parts.last.strip + tmp_drive[key] = value + end + hdds << tmp_drive unless tmp_drive.empty? + + hdds + end + + # @param [String] disk_file + # @param [Integer] disk_size_in_mb + def resize_disk(disk_file, disk_size_in_mb) + # todo: better error handling for this execute + # todo: MEDIUM changes if virtualbox is older than 5. Need a proper check/switch + # Maybe move this into version_4, then version_5 + execute("modify#{MEDIUM}", disk_file, '--resize', disk_size_in_mb.to_s) + end + + # @param [String] uui - virtual machines uuid + # @param [Hash] disk - disk to attach + def attach_disk(uuid, disk) + controller = disk[:controller] + port = disk[:port] + device = disk[:device] + file = disk[:file] + + # todo: hard set to type hdd, need to look if all types are compatible with these flags + execute('storageattach', uuid, '--storagectl', controller, '--port', port, '--device', device, '--type', 'hdd', '--medium', file) + end + + # Removes a disk from the given virtual machine + # + # @param [String] disk_file + def remove_disk(disk_file) + # todo: better error handling for this execute + execute("closemedium", disk_file, '--delete') + end + # Execute the given subcommand for VBoxManage and return the output. def execute(*command, &block) # Get the options hash if it exists From cac56c983cbd9f9c610eb4d4c2510908ee7379a2 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 11 Dec 2019 10:41:39 -0800 Subject: [PATCH 008/127] Add extra key for vbox driver with listing hdds --- plugins/providers/virtualbox/driver/base.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/plugins/providers/virtualbox/driver/base.rb b/plugins/providers/virtualbox/driver/base.rb index cbd4baf93..9688563d8 100644 --- a/plugins/providers/virtualbox/driver/base.rb +++ b/plugins/providers/virtualbox/driver/base.rb @@ -383,6 +383,9 @@ module VagrantPlugins info end + # Lists all attached harddisks from a given virtual machine. Additionally, + # this method adds a new key "Disk Name" based on the disks file path from "Location" + # # @return [Array] hdds An array of hashes of harddrive info for a guest def list_hdds hdds = [] @@ -397,6 +400,10 @@ module VagrantPlugins key = parts.first.strip value = parts.last.strip tmp_drive[key] = value + + if key == "Location" + tmp_drive["Disk Name"] = File.basename(value, ".*") + end end hdds << tmp_drive unless tmp_drive.empty? From 35c8db56ece3fe3e638f957e6e8f4d2cfbdbc1b9 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 11 Dec 2019 16:32:21 -0800 Subject: [PATCH 009/127] Add handling for configuring and creating disks with vbox provider --- lib/vagrant/util/numeric.rb | 5 ++ .../virtualbox/cap/configure_disks.rb | 80 ++++++++++++++++++- plugins/providers/virtualbox/driver/base.rb | 12 +++ 3 files changed, 94 insertions(+), 3 deletions(-) diff --git a/lib/vagrant/util/numeric.rb b/lib/vagrant/util/numeric.rb index 31cdd75d5..f65d8512d 100644 --- a/lib/vagrant/util/numeric.rb +++ b/lib/vagrant/util/numeric.rb @@ -57,6 +57,11 @@ module Vagrant (bytes / MEGABYTE).round(2) end + # @param [Integer] bytes + # @return [Integer] bytes - rounded up to nearest 512 + def round_to_512(bytes) + end + # @private # Reset the cached values for platform. This is not considered a public # API and should only be used for testing. diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 102a51159..f0e899a94 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -1,30 +1,104 @@ +require "log4r" require "vagrant/util/numeric" module VagrantPlugins module ProviderVirtualBox module Cap module ConfigureDisks + LOGGER = Log4r::Logger.new("vagrant::plugins::virtualbox::configure_disks") + # @param [Vagrant::Machine] machine # @param [VagrantPlugins::Kernel_V2::VagrantConfigDisk] defined_disks def self.configure_disks(machine, defined_disks) return if defined_disks.empty? current_disks = machine.provider.driver.list_hdds - # Compare current disks to config, and if there's a delta, adjust accordingly - # - # Compare by name if possible + require 'pry' + binding.pry + defined_disks.each do |disk| if disk.type == :disk + handle_configure_disk(machine, disk, current_disks) + elsif disk.type == :floppy + # TODO: Write me + elsif disk.type == :dvd + # TODO: Write me end end end protected + # Handles all disk configs of type `:disk` + def self.handle_configure_disk(machine, disk, all_disks) + # Grab the existing configured disk, if it exists + current_disk = nil + if disk.primary + current_disk = all_disks.first + else + current_disk = all_disks.select { |d| d["Disk Name"] == disk.name}.first + end + + if !current_disk + machine.ui.warn("Disk '#{disk.name}' not found in guest. Creating and attaching disk to guest...") + # create new disk and attach + create_disk(machine, disk) + elsif !compare_disk_state(disk, current_disk) + machine.ui.warn("Disk '#{disk.name}' needs to be resized. Attempting to resize disk...", prefix: true) + resize_disk(machine, disk, current_disk) + else + # log no need to reconfigure disk, already in desired state + LOGGER.info("No further configuration required for disk '#{disk.name}'.") + end + end + + # Check to see if current disk is configured based on defined_disks + # + # @param [Kernel_V2::VagrantConfigDisk] disk_config + # @param [Hash] defined_disk + # @return [Boolean] + def self.compare_disk_state(disk_config, defined_disk) + requested_disk_size = Vagrant::Util::Numeric.bytes_to_megabytes(disk_config.size) + defined_disk_size = defined_disk["Capacity"].split(" ").first.to_f + + return defined_disk_size == requested_disk_size + end + + def self.create_disk(machine, disk_config) + guest_info = machine.provider.driver.show_vm_info(machine.id) + disk_provider_config = disk_config.provider_config[:virtualbox] + + guest_folder = File.dirname(guest_info["CfgFile"]) + if disk_provider_config + if disk_provider_config.include?(:disk_type) + disk_ext = disk_provider_config[:disk_type].downcase + end + else + disk_ext = "vdi" + end + disk_file = "#{guest_folder}/#{disk_config.name}.#{disk_ext}" + + # TODO: Round disk_config.size to the nearest 512 bytes to make it divisble by 512 + # Source: https://www.virtualbox.org/ticket/5582 + LOGGER.info("Attempting to create a new disk file '#{disk_file}' of size '#{disk_config.size}' bytes") + machine.provider.driver.create_disk(disk_file, disk_config.size, disk_ext.upcase) + end + + def self.resize_disk(machine, disk_config, defined_disk) + # check if vmdk (probably) + # if so, convert + # then resize + # if converted, convert back + # reattach?? + # done + end + def self.vmdk_to_vdi(driver) + LOGGER.warn("Converting disk from vmdk to vdi format") end def self.vdi_to_vmdk(driver) + LOGGER.warn("Converting disk from vdi to vmdk format") end end end diff --git a/plugins/providers/virtualbox/driver/base.rb b/plugins/providers/virtualbox/driver/base.rb index 9688563d8..168468bc8 100644 --- a/plugins/providers/virtualbox/driver/base.rb +++ b/plugins/providers/virtualbox/driver/base.rb @@ -419,6 +419,18 @@ module VagrantPlugins execute("modify#{MEDIUM}", disk_file, '--resize', disk_size_in_mb.to_s) end + # Creates a disk. Default format is VDI unless overridden + # + # Note: disk_size must be divisible by 512 bytes, otherwise operation will fail + # Source: https://www.virtualbox.org/ticket/5582 + # + # @param [String] disk_file + # @param [Integer] disk_size - size in bytes (MUST BE DIVISIBLE BY 512 bytes) + # @param [String] disk_format - format of disk, defaults to "VDI" + def create_disk(disk_file, disk_size, disk_format="VDI") + execute("createmedium", '--filename', disk_file, '--sizebyte', disk_size.to_s, '--format', disk_format) + end + # @param [String] uui - virtual machines uuid # @param [Hash] disk - disk to attach def attach_disk(uuid, disk) From 9b83e2ac30e36fe943ad2e6f0e879ca6641d10b9 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 12 Dec 2019 09:35:56 -0800 Subject: [PATCH 010/127] Move around pry --- plugins/providers/virtualbox/cap/configure_disks.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index f0e899a94..8dc150b50 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -13,8 +13,6 @@ module VagrantPlugins return if defined_disks.empty? current_disks = machine.provider.driver.list_hdds - require 'pry' - binding.pry defined_disks.each do |disk| if disk.type == :disk @@ -77,6 +75,8 @@ module VagrantPlugins disk_ext = "vdi" end disk_file = "#{guest_folder}/#{disk_config.name}.#{disk_ext}" + require 'pry' + binding.pry # TODO: Round disk_config.size to the nearest 512 bytes to make it divisble by 512 # Source: https://www.virtualbox.org/ticket/5582 From 259cb5fcca14c145ecc3521023fe8cde18f7e923 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 12 Dec 2019 09:36:16 -0800 Subject: [PATCH 011/127] Attempt to create and attach disk to guest --- .../providers/virtualbox/cap/configure_disks.rb | 13 +++++++++++-- plugins/providers/virtualbox/driver/base.rb | 16 ++++++++++------ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 8dc150b50..afd31ca34 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -62,18 +62,23 @@ module VagrantPlugins return defined_disk_size == requested_disk_size end + # Creates and attaches a disk to a machine + # + # @param [Vagrant::Machine] machine + # @param [Kernel_V2::VagrantConfigDisk] disk_config def self.create_disk(machine, disk_config) guest_info = machine.provider.driver.show_vm_info(machine.id) disk_provider_config = disk_config.provider_config[:virtualbox] guest_folder = File.dirname(guest_info["CfgFile"]) + disk_ext = "vdi" + if disk_provider_config if disk_provider_config.include?(:disk_type) disk_ext = disk_provider_config[:disk_type].downcase end - else - disk_ext = "vdi" end + # TODO: use File class for path separator instead disk_file = "#{guest_folder}/#{disk_config.name}.#{disk_ext}" require 'pry' binding.pry @@ -82,6 +87,10 @@ module VagrantPlugins # Source: https://www.virtualbox.org/ticket/5582 LOGGER.info("Attempting to create a new disk file '#{disk_file}' of size '#{disk_config.size}' bytes") machine.provider.driver.create_disk(disk_file, disk_config.size, disk_ext.upcase) + + # TODO: Determine what port and device to attach disk to??? + # look at guest_info and see what is in use + #machine.provider.driver.attach_disk(machine.id, nil, nil, disk_file) end def self.resize_disk(machine, disk_config, defined_disk) diff --git a/plugins/providers/virtualbox/driver/base.rb b/plugins/providers/virtualbox/driver/base.rb index 168468bc8..9a2bb00a0 100644 --- a/plugins/providers/virtualbox/driver/base.rb +++ b/plugins/providers/virtualbox/driver/base.rb @@ -428,16 +428,20 @@ module VagrantPlugins # @param [Integer] disk_size - size in bytes (MUST BE DIVISIBLE BY 512 bytes) # @param [String] disk_format - format of disk, defaults to "VDI" def create_disk(disk_file, disk_size, disk_format="VDI") - execute("createmedium", '--filename', disk_file, '--sizebyte', disk_size.to_s, '--format', disk_format) + execute("createmedium", '--filename', disk_file, '--sizebyte', disk_size.to_i.to_s, '--format', disk_format) end + # Controller-Port-Device looks like: + # SATA Controller-ImageUUID-0-0 (sub out ImageUUID) + # - Controller: SATA Controller + # - Port: 0 + # - Device: 0 + # # @param [String] uui - virtual machines uuid # @param [Hash] disk - disk to attach - def attach_disk(uuid, disk) - controller = disk[:controller] - port = disk[:port] - device = disk[:device] - file = disk[:file] + def attach_disk(uuid, port, device, file) + # Maybe only support SATA Controller for `:disk`??? + controller = "SATA Controller" # todo: hard set to type hdd, need to look if all types are compatible with these flags execute('storageattach', uuid, '--storagectl', controller, '--port', port, '--device', device, '--type', 'hdd', '--medium', file) From 2a3f9dad29e7da571d175ebe821df598373e4c71 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 12 Dec 2019 16:30:38 -0800 Subject: [PATCH 012/127] Remove uuid from params No need to pass in machine uuid as it is a class variable already for the driver --- plugins/providers/virtualbox/cap/configure_disks.rb | 2 +- plugins/providers/virtualbox/driver/base.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index afd31ca34..8a896a501 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -67,7 +67,7 @@ module VagrantPlugins # @param [Vagrant::Machine] machine # @param [Kernel_V2::VagrantConfigDisk] disk_config def self.create_disk(machine, disk_config) - guest_info = machine.provider.driver.show_vm_info(machine.id) + guest_info = machine.provider.driver.show_vm_info disk_provider_config = disk_config.provider_config[:virtualbox] guest_folder = File.dirname(guest_info["CfgFile"]) diff --git a/plugins/providers/virtualbox/driver/base.rb b/plugins/providers/virtualbox/driver/base.rb index 9a2bb00a0..5cecd4539 100644 --- a/plugins/providers/virtualbox/driver/base.rb +++ b/plugins/providers/virtualbox/driver/base.rb @@ -372,9 +372,9 @@ module VagrantPlugins # # @param [String] uuid # @return [Hash] info - def show_vm_info(uuid) + def show_vm_info info = {} - execute('showvminfo', uuid, '--machinereadable', retryable: true).split("\n").each do |line| + execute('showvminfo', @uuid, '--machinereadable', retryable: true).split("\n").each do |line| parts = line.partition('=') key = parts.first.gsub('"', '') value = parts.last.gsub('"', '') From 1c87351c94920891871ea0e2912a61f699ef233a Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 12 Dec 2019 16:31:10 -0800 Subject: [PATCH 013/127] Begin to add method for determining the proper port and device --- .../virtualbox/cap/configure_disks.rb | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 8a896a501..3ce3121a2 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -13,6 +13,8 @@ module VagrantPlugins return if defined_disks.empty? current_disks = machine.provider.driver.list_hdds + require 'pry' + binding.pry defined_disks.each do |disk| if disk.type == :disk @@ -90,9 +92,31 @@ module VagrantPlugins # TODO: Determine what port and device to attach disk to??? # look at guest_info and see what is in use - #machine.provider.driver.attach_disk(machine.id, nil, nil, disk_file) + + require 'pry' + binding.pry + # need to get the _correct_ port and device to attach disk to + # Port is easy (pick the "next one" available), but what about device??? can you have more than one device per controller? + #disk_data = get_sata_controller_list(machine) + machine.provider.driver.attach_disk(machine.id, disk_data[:port], disk_data[:device], disk_file) end + def self.get_sata_controller_list(machine) + vm_info = show_vm_info + + sata_controller = {} + vm_info.each do |key,value| + if key.include?("ImageUUID") + disk_info = key.split("-") + else + next + end + end + + sata_controller + end + + def self.resize_disk(machine, disk_config, defined_disk) # check if vmdk (probably) # if so, convert From 7e686e6f70289745df63aa396c4a24191490955a Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 13 Dec 2019 09:37:10 -0800 Subject: [PATCH 014/127] Attach harddisk after creation --- .../virtualbox/cap/configure_disks.rb | 18 ++++++++++-------- plugins/providers/virtualbox/driver/base.rb | 2 +- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 3ce3121a2..a5698b081 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -34,6 +34,7 @@ module VagrantPlugins # Grab the existing configured disk, if it exists current_disk = nil if disk.primary + # TODO: This instead might need to be determined through the show_vm_info data instead current_disk = all_disks.first else current_disk = all_disks.select { |d| d["Disk Name"] == disk.name}.first @@ -93,27 +94,28 @@ module VagrantPlugins # TODO: Determine what port and device to attach disk to??? # look at guest_info and see what is in use - require 'pry' - binding.pry # need to get the _correct_ port and device to attach disk to # Port is easy (pick the "next one" available), but what about device??? can you have more than one device per controller? - #disk_data = get_sata_controller_list(machine) - machine.provider.driver.attach_disk(machine.id, disk_data[:port], disk_data[:device], disk_file) + port = get_next_port(machine) + device = "0" + machine.provider.driver.attach_disk(machine.id, port, device, disk_file) end - def self.get_sata_controller_list(machine) - vm_info = show_vm_info + def self.get_next_port(machine) + vm_info = machine.provider.driver.show_vm_info - sata_controller = {} + port = 0 vm_info.each do |key,value| if key.include?("ImageUUID") disk_info = key.split("-") + port = disk_info[2] else next end end - sata_controller + port = (port.to_i + 1).to_s + port end diff --git a/plugins/providers/virtualbox/driver/base.rb b/plugins/providers/virtualbox/driver/base.rb index 5cecd4539..289cfea43 100644 --- a/plugins/providers/virtualbox/driver/base.rb +++ b/plugins/providers/virtualbox/driver/base.rb @@ -444,7 +444,7 @@ module VagrantPlugins controller = "SATA Controller" # todo: hard set to type hdd, need to look if all types are compatible with these flags - execute('storageattach', uuid, '--storagectl', controller, '--port', port, '--device', device, '--type', 'hdd', '--medium', file) + execute('storageattach', uuid, '--storagectl', controller, '--port', port.to_s, '--device', device.to_s, '--type', 'hdd', '--medium', file) end # Removes a disk from the given virtual machine From 4df346c5c63b79d1859885a28ea856991dd6019f Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 13 Dec 2019 10:22:33 -0800 Subject: [PATCH 015/127] Add resize for disks Also add warnings in case disk resize request is smaller than original disk size. --- plugins/providers/virtualbox/cap/configure_disks.rb | 13 ++++++++++--- plugins/providers/virtualbox/driver/base.rb | 4 +++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index a5698b081..06fa390a4 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -44,7 +44,7 @@ module VagrantPlugins machine.ui.warn("Disk '#{disk.name}' not found in guest. Creating and attaching disk to guest...") # create new disk and attach create_disk(machine, disk) - elsif !compare_disk_state(disk, current_disk) + elsif compare_disk_state(machine, disk, current_disk) machine.ui.warn("Disk '#{disk.name}' needs to be resized. Attempting to resize disk...", prefix: true) resize_disk(machine, disk, current_disk) else @@ -58,11 +58,18 @@ module VagrantPlugins # @param [Kernel_V2::VagrantConfigDisk] disk_config # @param [Hash] defined_disk # @return [Boolean] - def self.compare_disk_state(disk_config, defined_disk) + def self.compare_disk_state(machine, disk_config, defined_disk) requested_disk_size = Vagrant::Util::Numeric.bytes_to_megabytes(disk_config.size) defined_disk_size = defined_disk["Capacity"].split(" ").first.to_f - return defined_disk_size == requested_disk_size + if defined_disk_size > requested_disk_size + machine.ui.warn("VirtualBox does not support shrinking disk size. Cannot shrink '#{disk_config.name}' disks size") + return false + elsif defined_disk_size < requested_disk_size + return true + else + return false + end end # Creates and attaches a disk to a machine diff --git a/plugins/providers/virtualbox/driver/base.rb b/plugins/providers/virtualbox/driver/base.rb index 289cfea43..9ca21e224 100644 --- a/plugins/providers/virtualbox/driver/base.rb +++ b/plugins/providers/virtualbox/driver/base.rb @@ -416,7 +416,9 @@ module VagrantPlugins # todo: better error handling for this execute # todo: MEDIUM changes if virtualbox is older than 5. Need a proper check/switch # Maybe move this into version_4, then version_5 - execute("modify#{MEDIUM}", disk_file, '--resize', disk_size_in_mb.to_s) + # if version 4, medium = "hd" + medium = "medium" + execute("modify#{medium}", disk_file, '--resizebyte', disk_size_in_mb.to_i.to_s) end # Creates a disk. Default format is VDI unless overridden From fcc9c55fa010f71883d95eba5fc1e8236f64ad45 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 13 Dec 2019 10:23:03 -0800 Subject: [PATCH 016/127] Begin to add code for resizing VMDK type disks --- .../virtualbox/cap/configure_disks.rb | 31 +++++++++++++------ plugins/providers/virtualbox/driver/base.rb | 3 ++ 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 06fa390a4..fd6f12c50 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -125,21 +125,34 @@ module VagrantPlugins port end - def self.resize_disk(machine, disk_config, defined_disk) - # check if vmdk (probably) - # if so, convert - # then resize - # if converted, convert back - # reattach?? - # done + 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") + # How to: + # grab disks port and device number + # clone disk to vdi formatted disk + # detatch vmdk disk?? + # resize vdi + # clone disk to vmdk + # attach vmdk to original port/device + # delete vdi + # delete vmdk + # + # TODO: IF any of the above steps fail, display a useful error message + # telling the user how to recover + # + # Vagrant could also have a "rescue" here where in the case of failure, it simply + # reattaches the original disk + else + machine.provider.driver.resize_disk(defined_disk["Location"], disk_config.size.to_i) + end end - def self.vmdk_to_vdi(driver) + def self.vmdk_to_vdi(driver, defined_disk) LOGGER.warn("Converting disk from vmdk to vdi format") end - def self.vdi_to_vmdk(driver) + def self.vdi_to_vmdk(driver, defined_disk) LOGGER.warn("Converting disk from vdi to vmdk format") end end diff --git a/plugins/providers/virtualbox/driver/base.rb b/plugins/providers/virtualbox/driver/base.rb index 9ca21e224..eab647f71 100644 --- a/plugins/providers/virtualbox/driver/base.rb +++ b/plugins/providers/virtualbox/driver/base.rb @@ -433,6 +433,9 @@ module VagrantPlugins execute("createmedium", '--filename', disk_file, '--sizebyte', disk_size.to_i.to_s, '--format', disk_format) end + def clone_disk(disk_file, disk_format="VDI") + end + # Controller-Port-Device looks like: # SATA Controller-ImageUUID-0-0 (sub out ImageUUID) # - Controller: SATA Controller From 56cc23cdece1871eda85caf0de4bed4a813727fe Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 13 Dec 2019 10:24:20 -0800 Subject: [PATCH 017/127] Remove prys --- plugins/providers/virtualbox/cap/configure_disks.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index fd6f12c50..450aeb653 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -13,8 +13,6 @@ module VagrantPlugins return if defined_disks.empty? current_disks = machine.provider.driver.list_hdds - require 'pry' - binding.pry defined_disks.each do |disk| if disk.type == :disk @@ -90,8 +88,6 @@ module VagrantPlugins end # TODO: use File class for path separator instead disk_file = "#{guest_folder}/#{disk_config.name}.#{disk_ext}" - require 'pry' - binding.pry # TODO: Round disk_config.size to the nearest 512 bytes to make it divisble by 512 # Source: https://www.virtualbox.org/ticket/5582 From dec145b8f98a1f30392b4424945ef9b6c31c89e3 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 13 Dec 2019 11:19:08 -0800 Subject: [PATCH 018/127] Add additional note about resizing a disk that might be a primrary --- plugins/providers/virtualbox/cap/configure_disks.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 450aeb653..c7b3b290b 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -129,7 +129,7 @@ module VagrantPlugins # clone disk to vdi formatted disk # detatch vmdk disk?? # resize vdi - # clone disk to vmdk + # clone disk to vmdk ....(or don't clone back if requested file type is vdi??) # attach vmdk to original port/device # delete vdi # delete vmdk From 2e884207dffad598e13b9788d27feaf47bd04919 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 13 Dec 2019 13:39:23 -0800 Subject: [PATCH 019/127] Rename disk_size variable Use bytes instead of mb to align with flag used --- plugins/providers/virtualbox/driver/base.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/providers/virtualbox/driver/base.rb b/plugins/providers/virtualbox/driver/base.rb index eab647f71..f11a78124 100644 --- a/plugins/providers/virtualbox/driver/base.rb +++ b/plugins/providers/virtualbox/driver/base.rb @@ -411,14 +411,14 @@ module VagrantPlugins end # @param [String] disk_file - # @param [Integer] disk_size_in_mb - def resize_disk(disk_file, disk_size_in_mb) + # @param [Integer] disk_size in bytes + def resize_disk(disk_file, disk_size) # todo: better error handling for this execute # todo: MEDIUM changes if virtualbox is older than 5. Need a proper check/switch # Maybe move this into version_4, then version_5 # if version 4, medium = "hd" medium = "medium" - execute("modify#{medium}", disk_file, '--resizebyte', disk_size_in_mb.to_i.to_s) + execute("modify#{medium}", disk_file, '--resizebyte', disk_size.to_i.to_s) end # Creates a disk. Default format is VDI unless overridden From 7486fe5931e8fa2b90811d48f9a424de41b9b951 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 13 Dec 2019 13:51:42 -0800 Subject: [PATCH 020/127] Add ability to resize vmdk disks by converting to vdi --- .../virtualbox/cap/configure_disks.rb | 42 ++++++++++++++++++- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index c7b3b290b..da5bdbbdf 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -87,7 +87,8 @@ module VagrantPlugins end end # TODO: use File class for path separator instead - disk_file = "#{guest_folder}/#{disk_config.name}.#{disk_ext}" + #disk_file = "#{guest_folder}/#{disk_config.name}.#{disk_ext}" + disk_file = File.join(guest_folder, disk_config.name) + ".#{disk_ext}" # TODO: Round disk_config.size to the nearest 512 bytes to make it divisble by 512 # Source: https://www.virtualbox.org/ticket/5582 @@ -121,14 +122,36 @@ module VagrantPlugins port end + def self.get_port_and_device(vm_info, defined_disk) + disk = {} + vm_info.each do |key,value| + if key.include?("ImageUUID") && value == defined_disk["UUID"] + disk_info = key.split("-") + disk[:port] = disk_info[2] + disk[:device] = disk_info[3] + break + else + next + end + end + + disk + end + def self.resize_disk(machine, disk_config, defined_disk) 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") # How to: # grab disks port and device number + vm_info = machine.provider.driver.show_vm_info + disk_info = get_port_and_device(vm_info, defined_disk) # clone disk to vdi formatted disk + vdi_disk_file = vmdk_to_vdi(machine.provider.driver, defined_disk) # detatch vmdk disk?? + machine.provider.driver.attach_disk(machine.id, disk_info[:port], disk_info[:device], vdi_disk_file) + machine.provider.driver.remove_disk(defined_disk["Location"]) # resize vdi + machine.provider.driver.resize_disk(vdi_disk_file, disk_config.size.to_i) # clone disk to vmdk ....(or don't clone back if requested file type is vdi??) # attach vmdk to original port/device # delete vdi @@ -145,11 +168,26 @@ module VagrantPlugins end def self.vmdk_to_vdi(driver, defined_disk) - LOGGER.warn("Converting disk from vmdk to vdi format") + LOGGER.warn("Converting disk '#{defined_disk["Disk Name"]}' from 'vmdk' to 'vdi' format") + # todo: MEDIUM changes if virtualbox is older than 5. Need a proper check/switch + # Maybe move this into version_4, then version_5 + # if version 4, medium = "hd" + medium = "medium" + + source = defined_disk["Location"] + destination = File.join(File.dirname(source), File.basename(source, ".*")) + ".vdi" + driver.execute("clone#{medium}", source, destination, '--format', 'VDI') + + destination end def self.vdi_to_vmdk(driver, defined_disk) LOGGER.warn("Converting disk from vdi to vmdk format") + medium = "medium" + + source = defined_disk["Location"] + destination = File.join(File.dirname(source), File.basename(source, ".*")) + ".vmdk" + driver.execute("clone#{medium}", source, destination, '--format', 'VMDK') end end end From 91a274416ac12ea4838e99e9543909c890376ab1 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 13 Dec 2019 15:26:50 -0800 Subject: [PATCH 021/127] Improve disk messages to user --- plugins/providers/virtualbox/cap/configure_disks.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index da5bdbbdf..edcb28230 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -12,6 +12,8 @@ module VagrantPlugins def self.configure_disks(machine, defined_disks) return if defined_disks.empty? + machine.ui.info("Configuring storage mediums...") + current_disks = machine.provider.driver.list_hdds defined_disks.each do |disk| @@ -39,15 +41,15 @@ module VagrantPlugins end if !current_disk - machine.ui.warn("Disk '#{disk.name}' not found in guest. Creating and attaching disk to guest...") + machine.ui.detail("Disk '#{disk.name}' not found in guest. Creating and attaching disk to guest...") # create new disk and attach create_disk(machine, disk) elsif compare_disk_state(machine, disk, current_disk) - machine.ui.warn("Disk '#{disk.name}' needs to be resized. Attempting to resize disk...", prefix: true) + machine.ui.detail("Disk '#{disk.name}' needs to be resized. Resizing disk...", prefix: true) resize_disk(machine, disk, current_disk) else # log no need to reconfigure disk, already in desired state - LOGGER.info("No further configuration required for disk '#{disk.name}'.") + LOGGER.info("No further configuration required for disk '#{disk.name}'") end end From d1ccb6536f2d70daa6427e44a5396790c92b3824 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 13 Dec 2019 15:27:07 -0800 Subject: [PATCH 022/127] Add note about attaching device --- plugins/providers/virtualbox/cap/configure_disks.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index edcb28230..42e66f6eb 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -99,11 +99,10 @@ module VagrantPlugins # TODO: Determine what port and device to attach disk to??? # look at guest_info and see what is in use - # need to get the _correct_ port and device to attach disk to # Port is easy (pick the "next one" available), but what about device??? can you have more than one device per controller? port = get_next_port(machine) - device = "0" + device = "0" # TODO: Fix me machine.provider.driver.attach_disk(machine.id, port, device, disk_file) end From 0dfde12e9ac59abbba7a1a32b3639aee74cafc2f Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 13 Dec 2019 15:27:18 -0800 Subject: [PATCH 023/127] Add clone_disk method to virtualbox driver --- .../providers/virtualbox/cap/configure_disks.rb | 15 ++++++--------- plugins/providers/virtualbox/driver/base.rb | 11 ++++++++++- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 42e66f6eb..228153120 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -170,25 +170,22 @@ module VagrantPlugins def self.vmdk_to_vdi(driver, defined_disk) LOGGER.warn("Converting disk '#{defined_disk["Disk Name"]}' from 'vmdk' to 'vdi' format") - # todo: MEDIUM changes if virtualbox is older than 5. Need a proper check/switch - # Maybe move this into version_4, then version_5 - # if version 4, medium = "hd" - medium = "medium" - source = defined_disk["Location"] destination = File.join(File.dirname(source), File.basename(source, ".*")) + ".vdi" - driver.execute("clone#{medium}", source, destination, '--format', 'VDI') + + driver.clone_disk(source, destination, 'VDI') destination end def self.vdi_to_vmdk(driver, defined_disk) LOGGER.warn("Converting disk from vdi to vmdk format") - medium = "medium" - source = defined_disk["Location"] destination = File.join(File.dirname(source), File.basename(source, ".*")) + ".vmdk" - driver.execute("clone#{medium}", source, destination, '--format', 'VMDK') + + driver.clone_disk(source, destination, 'VMDK') + + destination end end end diff --git a/plugins/providers/virtualbox/driver/base.rb b/plugins/providers/virtualbox/driver/base.rb index f11a78124..1a230018a 100644 --- a/plugins/providers/virtualbox/driver/base.rb +++ b/plugins/providers/virtualbox/driver/base.rb @@ -433,7 +433,16 @@ module VagrantPlugins execute("createmedium", '--filename', disk_file, '--sizebyte', disk_size.to_i.to_s, '--format', disk_format) end - def clone_disk(disk_file, disk_format="VDI") + # @param [String] source + # @param [String] destination + # @param [String] disk_format + def clone_disk(source, destination, disk_format) + # todo: MEDIUM changes if virtualbox is older than 5. Need a proper check/switch + # Maybe move this into version_4, then version_5 + # if version 4, medium = "hd" + medium = "medium" + + execute("clone#{medium}", source, destination, '--format', disk_format) end # Controller-Port-Device looks like: From 616f8e4bd617574980f46c766d89be8144ca3115 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 13 Dec 2019 15:45:05 -0800 Subject: [PATCH 024/127] Add experimental flags --- plugins/providers/virtualbox/cap/configure_disks.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 228153120..05dd60f84 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -1,5 +1,6 @@ require "log4r" require "vagrant/util/numeric" +require "vagrant/util/experimental" module VagrantPlugins module ProviderVirtualBox @@ -12,6 +13,8 @@ module VagrantPlugins def self.configure_disks(machine, defined_disks) return if defined_disks.empty? + # return if Vagrant::Util::Experimental.feature_enabled?("virtualbox_disk_hdd") + machine.ui.info("Configuring storage mediums...") current_disks = machine.provider.driver.list_hdds From f076ee4708575bdec92a361022eda874285f6637 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 13 Dec 2019 15:45:14 -0800 Subject: [PATCH 025/127] Add method docs --- plugins/providers/virtualbox/cap/configure_disks.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 05dd60f84..2129a3860 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -109,6 +109,8 @@ module VagrantPlugins machine.provider.driver.attach_disk(machine.id, port, device, disk_file) end + # @param [Vagrant::Machine] machine + # @return [String] port - The next available port on a given device def self.get_next_port(machine) vm_info = machine.provider.driver.show_vm_info @@ -126,6 +128,9 @@ module VagrantPlugins port end + # @param [Hash] vm_info - Guest info from show_vm_info + # @param [Hash] defined_disk - A specific disk with info from list_hdd + # @return [Hash] disk - A hash with `port` and `device` keys found from a matching UUID in vm_info def self.get_port_and_device(vm_info, defined_disk) disk = {} vm_info.each do |key,value| From a38f0bb8c0834cd74835e4dad15a280d3187aedc Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 13 Dec 2019 16:35:34 -0800 Subject: [PATCH 026/127] Improve default vagrant disk name --- plugins/kernel_v2/config/disk.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/kernel_v2/config/disk.rb b/plugins/kernel_v2/config/disk.rb index 219a74aed..8f9b005a5 100644 --- a/plugins/kernel_v2/config/disk.rb +++ b/plugins/kernel_v2/config/disk.rb @@ -109,7 +109,7 @@ module VagrantPlugins if @primary @name = "vagrant_primary" else - @name = "name_#{@type.to_s}_#{@id.split("-").last}" + @name = "vagrant_#{@type.to_s}_#{@id.split("-").last}" end end From a75630e274b0930603647f6f677e3c00054340a0 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 13 Dec 2019 16:35:49 -0800 Subject: [PATCH 027/127] Enable experimental flag for virtualbox disk configs --- plugins/providers/virtualbox/cap/configure_disks.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 2129a3860..901b03c3c 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -13,7 +13,7 @@ module VagrantPlugins def self.configure_disks(machine, defined_disks) return if defined_disks.empty? - # return if Vagrant::Util::Experimental.feature_enabled?("virtualbox_disk_hdd") + return if !Vagrant::Util::Experimental.feature_enabled?("virtualbox_disk_hdd") machine.ui.info("Configuring storage mediums...") From 2600178d180e368edf900c3dc261fef14caefe00 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 13 Dec 2019 16:36:08 -0800 Subject: [PATCH 028/127] Move UI messages into methods --- plugins/providers/virtualbox/cap/configure_disks.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 901b03c3c..b61aa6fdb 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -44,11 +44,9 @@ module VagrantPlugins end if !current_disk - machine.ui.detail("Disk '#{disk.name}' not found in guest. Creating and attaching disk to guest...") # create new disk and attach create_disk(machine, disk) elsif compare_disk_state(machine, disk, current_disk) - machine.ui.detail("Disk '#{disk.name}' needs to be resized. Resizing disk...", prefix: true) resize_disk(machine, disk, current_disk) else # log no need to reconfigure disk, already in desired state @@ -80,6 +78,7 @@ module VagrantPlugins # @param [Vagrant::Machine] machine # @param [Kernel_V2::VagrantConfigDisk] disk_config def self.create_disk(machine, disk_config) + machine.ui.detail("Disk '#{disk_config.name}' not found in guest. Creating and attaching disk to guest...") guest_info = machine.provider.driver.show_vm_info disk_provider_config = disk_config.provider_config[:virtualbox] @@ -148,6 +147,8 @@ module VagrantPlugins end def self.resize_disk(machine, disk_config, defined_disk) + machine.ui.detail("Disk '#{disk.name}' needs to be resized. Resizing disk...", prefix: true) + 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") # How to: From b8bf424678bff2f85d3deca3594aa438212595f1 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 13 Dec 2019 16:36:30 -0800 Subject: [PATCH 029/127] Only grab provider disk config if it exists --- plugins/providers/virtualbox/cap/configure_disks.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index b61aa6fdb..736b5b9ae 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -80,7 +80,7 @@ module VagrantPlugins def self.create_disk(machine, disk_config) machine.ui.detail("Disk '#{disk_config.name}' not found in guest. Creating and attaching disk to guest...") guest_info = machine.provider.driver.show_vm_info - disk_provider_config = disk_config.provider_config[:virtualbox] + disk_provider_config = disk_config.provider_config[:virtualbox] if disk_config.provider_config guest_folder = File.dirname(guest_info["CfgFile"]) disk_ext = "vdi" From 26d922ad0eded1605e09b4ac2e349d2eb1c0ab08 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 13 Dec 2019 17:00:59 -0800 Subject: [PATCH 030/127] Add todo for fixing default disk names --- plugins/kernel_v2/config/disk.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/kernel_v2/config/disk.rb b/plugins/kernel_v2/config/disk.rb index 8f9b005a5..3a7c0dbd2 100644 --- a/plugins/kernel_v2/config/disk.rb +++ b/plugins/kernel_v2/config/disk.rb @@ -109,6 +109,7 @@ module VagrantPlugins if @primary @name = "vagrant_primary" else + # TODO: Can't rely on `id` because it changes in between vagrant runs @name = "vagrant_#{@type.to_s}_#{@id.split("-").last}" end end From 831b39f9e0c288a3e0a9c147420fb9e7fc422fbb Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 17 Dec 2019 09:18:16 -0800 Subject: [PATCH 031/127] Use correct variable for disk name --- plugins/providers/virtualbox/cap/configure_disks.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 736b5b9ae..5d9e432c0 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -147,7 +147,7 @@ module VagrantPlugins end def self.resize_disk(machine, disk_config, defined_disk) - machine.ui.detail("Disk '#{disk.name}' needs to be resized. Resizing disk...", prefix: true) + machine.ui.detail("Disk '#{disk_config.name}' needs to be resized. Resizing disk...", prefix: true) 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") From 81095090a0123ff8cc7aa27046ee416cbeddf7af Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 10 Jan 2020 10:36:53 -0800 Subject: [PATCH 032/127] Remove unused function --- lib/vagrant/util/numeric.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/vagrant/util/numeric.rb b/lib/vagrant/util/numeric.rb index f65d8512d..31cdd75d5 100644 --- a/lib/vagrant/util/numeric.rb +++ b/lib/vagrant/util/numeric.rb @@ -57,11 +57,6 @@ module Vagrant (bytes / MEGABYTE).round(2) end - # @param [Integer] bytes - # @return [Integer] bytes - rounded up to nearest 512 - def round_to_512(bytes) - end - # @private # Reset the cached values for platform. This is not considered a public # API and should only be used for testing. From ff77e6279b460a9282212a03ab634ebd98c300d0 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 10 Jan 2020 10:45:22 -0800 Subject: [PATCH 033/127] Remove todo comment --- plugins/providers/virtualbox/cap/configure_disks.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 5d9e432c0..f8ec8d0b0 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -90,8 +90,6 @@ module VagrantPlugins disk_ext = disk_provider_config[:disk_type].downcase end end - # TODO: use File class for path separator instead - #disk_file = "#{guest_folder}/#{disk_config.name}.#{disk_ext}" disk_file = File.join(guest_folder, disk_config.name) + ".#{disk_ext}" # TODO: Round disk_config.size to the nearest 512 bytes to make it divisble by 512 From ec350861cdbc0fcc5e95a5067a30ee64dd9894a9 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 13 Jan 2020 14:46:50 -0800 Subject: [PATCH 034/127] Write down metadata for disk configs after configuring disks --- lib/vagrant/action/builtin/disk.rb | 14 ++++++++- plugins/kernel_v2/config/disk.rb | 2 ++ .../virtualbox/cap/configure_disks.rb | 29 ++++++++++++++++--- 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/lib/vagrant/action/builtin/disk.rb b/lib/vagrant/action/builtin/disk.rb index cf0ac6a80..43ecb6856 100644 --- a/lib/vagrant/action/builtin/disk.rb +++ b/lib/vagrant/action/builtin/disk.rb @@ -1,3 +1,5 @@ +require "json" + module Vagrant module Action module Builtin @@ -14,17 +16,27 @@ module Vagrant # Call into providers machine implementation for disk management if !defined_disks.empty? if machine.provider.capability?(:configure_disks) - machine.provider.capability(:configure_disks, defined_disks) + configured_disks = machine.provider.capability(:configure_disks, defined_disks) else env[:ui].warn(I18n.t("vagrant.actions.disk.provider_unsupported", provider: machine.provider_name)) end end + write_disk_metadata(machine, configured_disks) + # Continue On @app.call(env) end + def write_disk_metadata(machine, current_disks) + meta_file = machine.data_dir.join("disk_meta") + @logger.debug("Writing box metadata file to #{meta_file}") + File.open(meta_file.to_s, "w+") do |file| + file.write(JSON.dump(current_disks)) + end + end + def get_disks(machine, env) return @_disks if @_disks diff --git a/plugins/kernel_v2/config/disk.rb b/plugins/kernel_v2/config/disk.rb index 3a7c0dbd2..423be34a8 100644 --- a/plugins/kernel_v2/config/disk.rb +++ b/plugins/kernel_v2/config/disk.rb @@ -110,6 +110,8 @@ module VagrantPlugins @name = "vagrant_primary" else # TODO: Can't rely on `id` because it changes in between vagrant runs + # + # INSTEAD: Make name a hard requirement, and see if that solves things @name = "vagrant_#{@type.to_s}_#{@id.split("-").last}" end end diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index f8ec8d0b0..ea2e83d65 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -10,6 +10,7 @@ module VagrantPlugins # @param [Vagrant::Machine] machine # @param [VagrantPlugins::Kernel_V2::VagrantConfigDisk] defined_disks + # @return [Hash] configured_disks - A hash of all the current configured disks def self.configure_disks(machine, defined_disks) return if defined_disks.empty? @@ -19,21 +20,28 @@ module VagrantPlugins current_disks = machine.provider.driver.list_hdds + configured_disks = {disk: [], floppy: [], dvd: []} + defined_disks.each do |disk| if disk.type == :disk - handle_configure_disk(machine, disk, current_disks) + disk_data = handle_configure_disk(machine, disk, current_disks) + configured_disks[:disk] << disk_data unless disk_data.empty? elsif disk.type == :floppy # TODO: Write me elsif disk.type == :dvd # TODO: Write me end end + + return configured_disks end protected # Handles all disk configs of type `:disk` + # @param [Hash] - disk_metadata def self.handle_configure_disk(machine, disk, all_disks) + disk_metadata = {} # Grab the existing configured disk, if it exists current_disk = nil if disk.primary @@ -45,13 +53,15 @@ module VagrantPlugins if !current_disk # create new disk and attach - create_disk(machine, disk) + disk_metadata = create_disk(machine, disk) elsif compare_disk_state(machine, disk, current_disk) - resize_disk(machine, disk, current_disk) + disk_metadata = resize_disk(machine, disk, current_disk) else # log no need to reconfigure disk, already in desired state LOGGER.info("No further configuration required for disk '#{disk.name}'") end + + return disk_metadata end # Check to see if current disk is configured based on defined_disks @@ -94,8 +104,13 @@ module VagrantPlugins # TODO: Round disk_config.size to the nearest 512 bytes to make it divisble by 512 # Source: https://www.virtualbox.org/ticket/5582 + # + # note: this might not be required... :thinking: needs more testing LOGGER.info("Attempting to create a new disk file '#{disk_file}' of size '#{disk_config.size}' bytes") - machine.provider.driver.create_disk(disk_file, disk_config.size, disk_ext.upcase) + + var = machine.provider.driver.create_disk(disk_file, disk_config.size, disk_ext.upcase) + # grab uuid from var, might also need disk name + disk_metadata = {uuid: var.split(':').last.strip, name: disk_config.name} # TODO: Determine what port and device to attach disk to??? # look at guest_info and see what is in use @@ -104,6 +119,8 @@ module VagrantPlugins port = get_next_port(machine) device = "0" # TODO: Fix me machine.provider.driver.attach_disk(machine.id, port, device, disk_file) + + disk_metadata end # @param [Vagrant::Machine] machine @@ -173,6 +190,10 @@ module VagrantPlugins else machine.provider.driver.resize_disk(defined_disk["Location"], disk_config.size.to_i) end + + + disk_metadata = {uuid: defined_disk["UUID"], name: defined_disk["Name"]} + return disk_metadata end def self.vmdk_to_vdi(driver, defined_disk) From ffe8fcef9b992a77588e6df47ea4645bc61ec408 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 13 Jan 2020 15:24:15 -0800 Subject: [PATCH 035/127] Make `name` required for defining non-primary disks --- plugins/kernel_v2/config/disk.rb | 9 ++++++--- templates/locales/en.yml | 2 ++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/plugins/kernel_v2/config/disk.rb b/plugins/kernel_v2/config/disk.rb index 423be34a8..d1691320d 100644 --- a/plugins/kernel_v2/config/disk.rb +++ b/plugins/kernel_v2/config/disk.rb @@ -110,9 +110,8 @@ module VagrantPlugins @name = "vagrant_primary" else # TODO: Can't rely on `id` because it changes in between vagrant runs - # - # INSTEAD: Make name a hard requirement, and see if that solves things - @name = "vagrant_#{@type.to_s}_#{@id.split("-").last}" + #@name = "vagrant_#{@type.to_s}_#{@id.split("-").last}" + @name = nil end end @@ -157,6 +156,10 @@ module VagrantPlugins end end + if !@name + errors << I18n.t("vagrant.config.disk.no_name_set", machine: machine.name) + end + errors end diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 1d91de3f5..abbd79f26 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -1812,6 +1812,8 @@ en: Disk file '%{file_path}' for disk '%{name}' on machine '%{machine}' does not exist. missing_provider: |- Guest '%{machine}' using provider '%{provider_name}' has provider specific config options for a provider other than '%{provider_name}'. These provider config options will be ignored for this guest + no_name_set: |- + A 'name' option is required when defining a disk for guest '%{machine}'. common: bad_field: "The following settings shouldn't exist: %{fields}" chef: From c2e6b5ae78a86dc84f38e321735c0b7a94150a63 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 13 Jan 2020 15:24:45 -0800 Subject: [PATCH 036/127] Ensure disks that go unchanged still get written into disk metadata file --- plugins/providers/virtualbox/cap/configure_disks.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index ea2e83d65..135c240e2 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -59,6 +59,7 @@ module VagrantPlugins else # log no need to reconfigure disk, already in desired state LOGGER.info("No further configuration required for disk '#{disk.name}'") + disk_metadata = {uuid: current_disk["UUID"], name: disk.name} end return disk_metadata From 5bf8d2363e111063f610d58ea8b81ed593b089bc Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 13 Jan 2020 15:55:19 -0800 Subject: [PATCH 037/127] Update logger message --- lib/vagrant/action/builtin/disk.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vagrant/action/builtin/disk.rb b/lib/vagrant/action/builtin/disk.rb index 43ecb6856..7cef91ec5 100644 --- a/lib/vagrant/action/builtin/disk.rb +++ b/lib/vagrant/action/builtin/disk.rb @@ -31,7 +31,7 @@ module Vagrant def write_disk_metadata(machine, current_disks) meta_file = machine.data_dir.join("disk_meta") - @logger.debug("Writing box metadata file to #{meta_file}") + @logger.debug("Writing disk metadata file to #{meta_file}") File.open(meta_file.to_s, "w+") do |file| file.write(JSON.dump(current_disks)) end From cc8ed8a6161adfa9408c7cd55698b811632d0fca Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 14 Jan 2020 10:00:55 -0800 Subject: [PATCH 038/127] Add cleanup_disks action --- lib/vagrant/action/builtin/cleanup_disks.rb | 49 +++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 lib/vagrant/action/builtin/cleanup_disks.rb diff --git a/lib/vagrant/action/builtin/cleanup_disks.rb b/lib/vagrant/action/builtin/cleanup_disks.rb new file mode 100644 index 000000000..414933f0b --- /dev/null +++ b/lib/vagrant/action/builtin/cleanup_disks.rb @@ -0,0 +1,49 @@ +require "json" + +module Vagrant + module Action + module Builtin + class CleanupDisks + # Removes any attached disks no longer defined in a Vagrantfile config + def initialize(app, env) + @app = app + @logger = Log4r::Logger.new("vagrant::action::builtin::disk") + end + + def call(env) + machine = env[:machine] + defined_disks = get_disks(machine, env) + + # Call into providers machine implementation for disk management + if !defined_disks.empty? + if machine.provider.capability?(:cleanup_disks) + machine.provider.capability(:cleanup_disks, defined_disks) + else + env[:ui].warn(I18n.t("vagrant.actions.disk.provider_unsupported", + provider: machine.provider_name)) + end + end + + # Continue On + @app.call(env) + end + + def read_disk_metadata(machine) + meta_file = machine.data_dir.join("disk_meta") + disk_meta = JSON.parse(meta_file.read) + + return disk_meta + end + + def get_disks(machine, env) + return @_disks if @_disks + + @_disks = [] + @_disks = machine.config.vm.disks + + @_disks + end + end + end + end +end From e4a57a8e1d0c3bab2e60ac37bb06c1a2b3105376 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 14 Jan 2020 10:15:57 -0800 Subject: [PATCH 039/127] Add virtualbox cleanup_disks action --- lib/vagrant/action.rb | 1 + plugins/providers/virtualbox/action.rb | 1 + .../providers/virtualbox/cap/cleanup_disks.rb | 23 +++++++++++++++++++ 3 files changed, 25 insertions(+) create mode 100644 plugins/providers/virtualbox/cap/cleanup_disks.rb diff --git a/lib/vagrant/action.rb b/lib/vagrant/action.rb index c0eaffcc8..432147691 100644 --- a/lib/vagrant/action.rb +++ b/lib/vagrant/action.rb @@ -12,6 +12,7 @@ module Vagrant autoload :BoxCheckOutdated, "vagrant/action/builtin/box_check_outdated" autoload :BoxRemove, "vagrant/action/builtin/box_remove" autoload :Call, "vagrant/action/builtin/call" + autoload :CleanupDisks, "vagrant/action/builtin/cleanup_disks" autoload :Confirm, "vagrant/action/builtin/confirm" autoload :ConfigValidate, "vagrant/action/builtin/config_validate" autoload :DestroyConfirm, "vagrant/action/builtin/destroy_confirm" diff --git a/plugins/providers/virtualbox/action.rb b/plugins/providers/virtualbox/action.rb index edfbbcbe0..00bfe09db 100644 --- a/plugins/providers/virtualbox/action.rb +++ b/plugins/providers/virtualbox/action.rb @@ -79,6 +79,7 @@ module VagrantPlugins b.use ForwardPorts b.use SetHostname b.use SaneDefaults + b.use CleanupDisks b.use Disk b.use Customize, "pre-boot" b.use Boot diff --git a/plugins/providers/virtualbox/cap/cleanup_disks.rb b/plugins/providers/virtualbox/cap/cleanup_disks.rb new file mode 100644 index 000000000..498e193f5 --- /dev/null +++ b/plugins/providers/virtualbox/cap/cleanup_disks.rb @@ -0,0 +1,23 @@ +require "log4r" +require "vagrant/util/numeric" +require "vagrant/util/experimental" + +module VagrantPlugins + module ProviderVirtualBox + module Cap + module CleanupDisks + LOGGER = Log4r::Logger.new("vagrant::plugins::virtualbox::cleanup_disks") + + # @param [Vagrant::Machine] machine + # @param [VagrantPlugins::Kernel_V2::VagrantConfigDisk] defined_disks + def self.cleanup_disks(machine, defined_disks) + return if defined_disks.empty? + + return if !Vagrant::Util::Experimental.feature_enabled?("virtualbox_disk_hdd") + end + + protected + end + end + end +end From 5d723b4e2364bfeb269b75a59fe0224074d3e4af Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 14 Jan 2020 10:26:41 -0800 Subject: [PATCH 040/127] Include cleanup_disk action for virtualbox plugin --- plugins/providers/virtualbox/plugin.rb | 5 +++++ templates/locales/en.yml | 2 ++ 2 files changed, 7 insertions(+) diff --git a/plugins/providers/virtualbox/plugin.rb b/plugins/providers/virtualbox/plugin.rb index cb0b02725..e5b3e03a5 100644 --- a/plugins/providers/virtualbox/plugin.rb +++ b/plugins/providers/virtualbox/plugin.rb @@ -44,6 +44,11 @@ module VagrantPlugins Cap::ConfigureDisks end + provider_capability(:virtualbox, :cleanup_disks) do + require_relative "cap/cleanup_disks" + Cap::CleanupDisks + end + provider_capability(:virtualbox, :snapshot_list) do require_relative "cap" Cap diff --git a/templates/locales/en.yml b/templates/locales/en.yml index abbd79f26..2725b2314 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -2171,6 +2171,8 @@ en: waiting_cleanup: "Waiting for cleanup before exiting..." exit_immediately: "Exiting immediately, without cleanup!" disk: + cleanup_provider_unsupported: |- + Guest provider '%{provider}' does not support the cleaning up disks, and will not attempt to clean up attached disks on the guest.. provider_unsupported: |- Guest provider '%{provider}' does not support the disk feature, and will not use the disk configuration defined. vm: From ad798c2c12a8de4d7a6882281a47d508e644b968 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 14 Jan 2020 10:27:04 -0800 Subject: [PATCH 041/127] Pass in disk meta file for cleanup_disks --- lib/vagrant/action/builtin/cleanup_disks.rb | 3 ++- plugins/providers/virtualbox/cap/cleanup_disks.rb | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/vagrant/action/builtin/cleanup_disks.rb b/lib/vagrant/action/builtin/cleanup_disks.rb index 414933f0b..c346c8ad2 100644 --- a/lib/vagrant/action/builtin/cleanup_disks.rb +++ b/lib/vagrant/action/builtin/cleanup_disks.rb @@ -17,7 +17,8 @@ module Vagrant # Call into providers machine implementation for disk management if !defined_disks.empty? if machine.provider.capability?(:cleanup_disks) - machine.provider.capability(:cleanup_disks, defined_disks) + disk_meta_file = read_disk_metadata(machine) + machine.provider.capability(:cleanup_disks, defined_disks, disk_meta_file) else env[:ui].warn(I18n.t("vagrant.actions.disk.provider_unsupported", provider: machine.provider_name)) diff --git a/plugins/providers/virtualbox/cap/cleanup_disks.rb b/plugins/providers/virtualbox/cap/cleanup_disks.rb index 498e193f5..830e288d4 100644 --- a/plugins/providers/virtualbox/cap/cleanup_disks.rb +++ b/plugins/providers/virtualbox/cap/cleanup_disks.rb @@ -10,7 +10,8 @@ module VagrantPlugins # @param [Vagrant::Machine] machine # @param [VagrantPlugins::Kernel_V2::VagrantConfigDisk] defined_disks - def self.cleanup_disks(machine, defined_disks) + # @param [Hash] disk_meta_file + def self.cleanup_disks(machine, defined_disks, disk_meta_file) return if defined_disks.empty? return if !Vagrant::Util::Experimental.feature_enabled?("virtualbox_disk_hdd") From 6ba621ac9265e002066b77d41d0053b950ffd75e Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 14 Jan 2020 14:30:44 -0800 Subject: [PATCH 042/127] Add ability to cleanup and close mediums for a guest --- lib/vagrant/action/builtin/cleanup_disks.rb | 11 ++++- .../providers/virtualbox/cap/cleanup_disks.rb | 44 ++++++++++++++++++- plugins/providers/virtualbox/driver/base.rb | 12 +++-- 3 files changed, 60 insertions(+), 7 deletions(-) diff --git a/lib/vagrant/action/builtin/cleanup_disks.rb b/lib/vagrant/action/builtin/cleanup_disks.rb index c346c8ad2..77f29e049 100644 --- a/lib/vagrant/action/builtin/cleanup_disks.rb +++ b/lib/vagrant/action/builtin/cleanup_disks.rb @@ -18,7 +18,9 @@ module Vagrant if !defined_disks.empty? if machine.provider.capability?(:cleanup_disks) disk_meta_file = read_disk_metadata(machine) - machine.provider.capability(:cleanup_disks, defined_disks, disk_meta_file) + if !disk_meta_file.empty? + machine.provider.capability(:cleanup_disks, defined_disks, disk_meta_file) + end else env[:ui].warn(I18n.t("vagrant.actions.disk.provider_unsupported", provider: machine.provider_name)) @@ -31,7 +33,12 @@ module Vagrant def read_disk_metadata(machine) meta_file = machine.data_dir.join("disk_meta") - disk_meta = JSON.parse(meta_file.read) + if File.file?(meta_file) + disk_meta = JSON.parse(meta_file.read) + else + @logger.info("No previous disk_meta file defined for guest #{machine.name}") + disk_meta = {} + end return disk_meta end diff --git a/plugins/providers/virtualbox/cap/cleanup_disks.rb b/plugins/providers/virtualbox/cap/cleanup_disks.rb index 830e288d4..11c8d6b42 100644 --- a/plugins/providers/virtualbox/cap/cleanup_disks.rb +++ b/plugins/providers/virtualbox/cap/cleanup_disks.rb @@ -1,5 +1,4 @@ require "log4r" -require "vagrant/util/numeric" require "vagrant/util/experimental" module VagrantPlugins @@ -15,9 +14,52 @@ module VagrantPlugins return if defined_disks.empty? return if !Vagrant::Util::Experimental.feature_enabled?("virtualbox_disk_hdd") + + handle_cleanup_disk(machine, defined_disks, disk_meta_file["disk"]) end protected + + # @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 self.get_device_port(vm_info, disk_uuid) + disk_info = {device: nil, port: nil} + + vm_info.each do |key,value| + if key.include?("ImageUUID") && + value == disk_uuid + info = key.split("-") + disk_info[:port] = info[2] + disk_info[:device] = info[3] + break + else + next + end + end + + disk_info + end + + def self.handle_cleanup_disk(machine, defined_disks, disk_meta) + vm_info = machine.provider.driver.show_vm_info + + disk_meta.each do |d| + dsk = defined_disks.select { |dk| dk.name == d["name"] } + if !dsk.empty? + next + else + LOGGER.warn("Found disk not in Vagrantfile config: '#{d["name"]}'. Removing disk from guest #{machine.name}") + disk_info = get_device_port(vm_info, d["uuid"]) + + machine.ui.detail("Disk '#{d["name"]}' no longer exists in Vagrant config. Removing and closing medium from guest...", prefix: true) + + # TODO: Maybe add a prompt for yes/no for closing the medium + machine.provider.driver.remove_disk(disk_info[:port], disk_info[:device]) + machine.provider.driver.close_medium(d["uuid"]) + end + end + end end end end diff --git a/plugins/providers/virtualbox/driver/base.rb b/plugins/providers/virtualbox/driver/base.rb index 1a230018a..4e8f3d152 100644 --- a/plugins/providers/virtualbox/driver/base.rb +++ b/plugins/providers/virtualbox/driver/base.rb @@ -463,10 +463,14 @@ module VagrantPlugins # Removes a disk from the given virtual machine # - # @param [String] disk_file - def remove_disk(disk_file) - # todo: better error handling for this execute - execute("closemedium", disk_file, '--delete') + # @param [String] disk_uuid + def close_medium(disk_uuid) + execute("closemedium", disk_uuid, '--delete') + end + + def remove_disk(port, device) + controller = "SATA Controller" + execute('storageattach', @uuid, '--storagectl', controller, '--port', port.to_s, '--device', device.to_s, '--medium', "none") end # Execute the given subcommand for VBoxManage and return the output. From 4d4844fdef4559b40c0310513e3ed83be2762759 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 14 Jan 2020 14:31:07 -0800 Subject: [PATCH 043/127] Simplify params for virtualbox driver attach_disk --- plugins/providers/virtualbox/cap/configure_disks.rb | 2 +- plugins/providers/virtualbox/driver/base.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 135c240e2..5389f9a95 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -119,7 +119,7 @@ module VagrantPlugins # Port is easy (pick the "next one" available), but what about device??? can you have more than one device per controller? port = get_next_port(machine) device = "0" # TODO: Fix me - machine.provider.driver.attach_disk(machine.id, port, device, disk_file) + machine.provider.driver.attach_disk(port, device, disk_file) disk_metadata end diff --git a/plugins/providers/virtualbox/driver/base.rb b/plugins/providers/virtualbox/driver/base.rb index 4e8f3d152..e3e264ed6 100644 --- a/plugins/providers/virtualbox/driver/base.rb +++ b/plugins/providers/virtualbox/driver/base.rb @@ -453,12 +453,12 @@ module VagrantPlugins # # @param [String] uui - virtual machines uuid # @param [Hash] disk - disk to attach - def attach_disk(uuid, port, device, file) + def attach_disk(port, device, file, type="hdd") # Maybe only support SATA Controller for `:disk`??? controller = "SATA Controller" # todo: hard set to type hdd, need to look if all types are compatible with these flags - execute('storageattach', uuid, '--storagectl', controller, '--port', port.to_s, '--device', device.to_s, '--type', 'hdd', '--medium', file) + execute('storageattach', @uuid, '--storagectl', controller, '--port', port.to_s, '--device', device.to_s, '--type', type, '--medium', file) end # Removes a disk from the given virtual machine From 71c3e3663512bf2771f0b0c696e1a32288396c41 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 14 Jan 2020 14:38:25 -0800 Subject: [PATCH 044/127] Raise exception if port and device cannot be determined --- plugins/providers/virtualbox/cap/cleanup_disks.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/providers/virtualbox/cap/cleanup_disks.rb b/plugins/providers/virtualbox/cap/cleanup_disks.rb index 11c8d6b42..e49358092 100644 --- a/plugins/providers/virtualbox/cap/cleanup_disks.rb +++ b/plugins/providers/virtualbox/cap/cleanup_disks.rb @@ -52,6 +52,11 @@ module VagrantPlugins LOGGER.warn("Found disk not in Vagrantfile config: '#{d["name"]}'. Removing disk from guest #{machine.name}") disk_info = get_device_port(vm_info, d["uuid"]) + # TODO: add proper vagrant error here with values + if disk_info.values.any?(nil) + raise Error, "could not determine device and port to remove disk" + end + machine.ui.detail("Disk '#{d["name"]}' no longer exists in Vagrant config. Removing and closing medium from guest...", prefix: true) # TODO: Maybe add a prompt for yes/no for closing the medium From 9712c599fe159d0b7d28b3ee6a52616ee254b7c9 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 14 Jan 2020 14:41:25 -0800 Subject: [PATCH 045/127] Update method docs for cleanup_disks cap --- plugins/providers/virtualbox/cap/cleanup_disks.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/plugins/providers/virtualbox/cap/cleanup_disks.rb b/plugins/providers/virtualbox/cap/cleanup_disks.rb index e49358092..0b154bb4c 100644 --- a/plugins/providers/virtualbox/cap/cleanup_disks.rb +++ b/plugins/providers/virtualbox/cap/cleanup_disks.rb @@ -9,13 +9,14 @@ module VagrantPlugins # @param [Vagrant::Machine] machine # @param [VagrantPlugins::Kernel_V2::VagrantConfigDisk] defined_disks - # @param [Hash] disk_meta_file + # @param [Hash] disk_meta_file - A hash of all the previously defined disks from the last configure_disk action def self.cleanup_disks(machine, defined_disks, disk_meta_file) return if defined_disks.empty? return if !Vagrant::Util::Experimental.feature_enabled?("virtualbox_disk_hdd") handle_cleanup_disk(machine, defined_disks, disk_meta_file["disk"]) + # TODO: Floppy and DVD disks end protected @@ -41,6 +42,9 @@ module VagrantPlugins disk_info end + # @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 def self.handle_cleanup_disk(machine, defined_disks, disk_meta) vm_info = machine.provider.driver.show_vm_info From d810f32ec5031fd0be392518263b380cc973124a Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 14 Jan 2020 14:41:34 -0800 Subject: [PATCH 046/127] Move message into info rather than detail for visibility --- plugins/providers/virtualbox/cap/cleanup_disks.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/providers/virtualbox/cap/cleanup_disks.rb b/plugins/providers/virtualbox/cap/cleanup_disks.rb index 0b154bb4c..b1a50a4a6 100644 --- a/plugins/providers/virtualbox/cap/cleanup_disks.rb +++ b/plugins/providers/virtualbox/cap/cleanup_disks.rb @@ -61,7 +61,7 @@ module VagrantPlugins raise Error, "could not determine device and port to remove disk" end - machine.ui.detail("Disk '#{d["name"]}' no longer exists in Vagrant config. Removing and closing medium from guest...", prefix: true) + machine.ui.info("Disk '#{d["name"]}' no longer exists in Vagrant config. Removing and closing medium from guest...", prefix: true) # TODO: Maybe add a prompt for yes/no for closing the medium machine.provider.driver.remove_disk(disk_info[:port], disk_info[:device]) From 98ab1798a2a891a1fd7021bf473a7cd13da9c166 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 15 Jan 2020 11:02:56 -0800 Subject: [PATCH 047/127] Only cleanup disks if defined from previous run --- plugins/providers/virtualbox/cap/cleanup_disks.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/providers/virtualbox/cap/cleanup_disks.rb b/plugins/providers/virtualbox/cap/cleanup_disks.rb index b1a50a4a6..c0d3f35b8 100644 --- a/plugins/providers/virtualbox/cap/cleanup_disks.rb +++ b/plugins/providers/virtualbox/cap/cleanup_disks.rb @@ -11,7 +11,7 @@ module VagrantPlugins # @param [VagrantPlugins::Kernel_V2::VagrantConfigDisk] defined_disks # @param [Hash] disk_meta_file - A hash of all the previously defined disks from the last configure_disk action def self.cleanup_disks(machine, defined_disks, disk_meta_file) - return if defined_disks.empty? + return if disk_meta_file.values.flatten.empty? return if !Vagrant::Util::Experimental.feature_enabled?("virtualbox_disk_hdd") From 424450cf16334689decf780b6ac766f5f78f653c Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 15 Jan 2020 11:04:42 -0800 Subject: [PATCH 048/127] Change info level from removing disks to WARN --- plugins/providers/virtualbox/cap/cleanup_disks.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/providers/virtualbox/cap/cleanup_disks.rb b/plugins/providers/virtualbox/cap/cleanup_disks.rb index c0d3f35b8..3813c66f0 100644 --- a/plugins/providers/virtualbox/cap/cleanup_disks.rb +++ b/plugins/providers/virtualbox/cap/cleanup_disks.rb @@ -61,7 +61,7 @@ module VagrantPlugins raise Error, "could not determine device and port to remove disk" end - machine.ui.info("Disk '#{d["name"]}' no longer exists in Vagrant config. Removing and closing medium from guest...", prefix: true) + machine.ui.warn("Disk '#{d["name"]}' no longer exists in Vagrant config. Removing and closing medium from guest...", prefix: true) # TODO: Maybe add a prompt for yes/no for closing the medium machine.provider.driver.remove_disk(disk_info[:port], disk_info[:device]) From c679b3f470e3b339a6930ed00813432ecae6ab3d Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 15 Jan 2020 11:16:44 -0800 Subject: [PATCH 049/127] Remove TODO --- plugins/providers/virtualbox/cap/cleanup_disks.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/providers/virtualbox/cap/cleanup_disks.rb b/plugins/providers/virtualbox/cap/cleanup_disks.rb index 3813c66f0..5891f40b1 100644 --- a/plugins/providers/virtualbox/cap/cleanup_disks.rb +++ b/plugins/providers/virtualbox/cap/cleanup_disks.rb @@ -63,7 +63,6 @@ module VagrantPlugins machine.ui.warn("Disk '#{d["name"]}' no longer exists in Vagrant config. Removing and closing medium from guest...", prefix: true) - # TODO: Maybe add a prompt for yes/no for closing the medium machine.provider.driver.remove_disk(disk_info[:port], disk_info[:device]) machine.provider.driver.close_medium(d["uuid"]) end From 16c38ed9f42626d31575becdcf86670c798b3c47 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 15 Jan 2020 13:29:34 -0800 Subject: [PATCH 050/127] Fix params for attach_disk driver call --- plugins/providers/virtualbox/cap/configure_disks.rb | 2 +- plugins/providers/virtualbox/driver/base.rb | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 5389f9a95..1dcd1c43d 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -174,7 +174,7 @@ module VagrantPlugins # clone disk to vdi formatted disk vdi_disk_file = vmdk_to_vdi(machine.provider.driver, defined_disk) # detatch vmdk disk?? - machine.provider.driver.attach_disk(machine.id, disk_info[:port], disk_info[:device], vdi_disk_file) + machine.provider.driver.attach_disk(disk_info[:port], disk_info[:device], vdi_disk_file, "hdd") machine.provider.driver.remove_disk(defined_disk["Location"]) # resize vdi machine.provider.driver.resize_disk(vdi_disk_file, disk_config.size.to_i) diff --git a/plugins/providers/virtualbox/driver/base.rb b/plugins/providers/virtualbox/driver/base.rb index e3e264ed6..cdf21b745 100644 --- a/plugins/providers/virtualbox/driver/base.rb +++ b/plugins/providers/virtualbox/driver/base.rb @@ -451,8 +451,10 @@ module VagrantPlugins # - Port: 0 # - Device: 0 # - # @param [String] uui - virtual machines uuid - # @param [Hash] disk - disk to attach + # @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 def attach_disk(port, device, file, type="hdd") # Maybe only support SATA Controller for `:disk`??? controller = "SATA Controller" From e31b013df8e3964bb128e684779fc8cc65d49d02 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 15 Jan 2020 14:46:29 -0800 Subject: [PATCH 051/127] Ensure vmdk disks that are resized stay in original format --- .../virtualbox/cap/configure_disks.rb | 42 +++++++++++-------- plugins/providers/virtualbox/driver/base.rb | 2 +- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 1dcd1c43d..bf9947e0e 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -167,23 +167,29 @@ module VagrantPlugins 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") - # How to: # grab disks port and device number vm_info = machine.provider.driver.show_vm_info disk_info = get_port_and_device(vm_info, defined_disk) # clone disk to vdi formatted disk - vdi_disk_file = vmdk_to_vdi(machine.provider.driver, defined_disk) - # detatch vmdk disk?? - machine.provider.driver.attach_disk(disk_info[:port], disk_info[:device], vdi_disk_file, "hdd") - machine.provider.driver.remove_disk(defined_disk["Location"]) + vdi_disk_file = vmdk_to_vdi(machine.provider.driver, defined_disk["Location"]) # resize vdi machine.provider.driver.resize_disk(vdi_disk_file, disk_config.size.to_i) - # clone disk to vmdk ....(or don't clone back if requested file type is vdi??) - # attach vmdk to original port/device - # delete vdi - # delete vmdk - # - # TODO: IF any of the above steps fail, display a useful error message + + # remove and close original volume + machine.provider.driver.remove_disk(disk_info[:port], disk_info[:device]) + machine.provider.driver.close_medium(defined_disk["UUID"]) + + vmdk_disk_file = vdi_to_vmdk(machine.provider.driver, vdi_disk_file) + machine.provider.driver.attach_disk(disk_info[:port], disk_info[:device], vmdk_disk_file, "hdd") + + # close cloned volume format + machine.provider.driver.close_medium(vdi_disk_file) + + # Get new disk UUID for vagrant disk_meta file + new_disk_info = machine.provider.driver.list_hdds.select { |h| h["Location"] == defined_disk["Location"] }.first + defined_disk = new_disk_info + + # TODO: If any of the above steps fail, display a useful error message # telling the user how to recover # # Vagrant could also have a "rescue" here where in the case of failure, it simply @@ -193,13 +199,13 @@ module VagrantPlugins end - disk_metadata = {uuid: defined_disk["UUID"], name: defined_disk["Name"]} + disk_metadata = {uuid: defined_disk["UUID"], name: disk_config.name} return disk_metadata end - def self.vmdk_to_vdi(driver, defined_disk) - LOGGER.warn("Converting disk '#{defined_disk["Disk Name"]}' from 'vmdk' to 'vdi' format") - source = defined_disk["Location"] + def self.vmdk_to_vdi(driver, defined_disk_path) + LOGGER.warn("Converting disk '#{defined_disk_path}' from 'vmdk' to 'vdi' format") + source = defined_disk_path destination = File.join(File.dirname(source), File.basename(source, ".*")) + ".vdi" driver.clone_disk(source, destination, 'VDI') @@ -207,9 +213,9 @@ module VagrantPlugins destination end - def self.vdi_to_vmdk(driver, defined_disk) - LOGGER.warn("Converting disk from vdi to vmdk format") - source = defined_disk["Location"] + def self.vdi_to_vmdk(driver, defined_disk_path) + LOGGER.warn("Converting disk '#{defined_disk_path}' from 'vdi' to 'vmdk' format") + source = defined_disk_path destination = File.join(File.dirname(source), File.basename(source, ".*")) + ".vmdk" driver.clone_disk(source, destination, 'VMDK') diff --git a/plugins/providers/virtualbox/driver/base.rb b/plugins/providers/virtualbox/driver/base.rb index cdf21b745..5f1d2e486 100644 --- a/plugins/providers/virtualbox/driver/base.rb +++ b/plugins/providers/virtualbox/driver/base.rb @@ -465,7 +465,7 @@ module VagrantPlugins # Removes a disk from the given virtual machine # - # @param [String] disk_uuid + # @param [String] disk_uuid or file path def close_medium(disk_uuid) execute("closemedium", disk_uuid, '--delete') end From 721576413071dfe65869b319c9c8ecbe6e0264f9 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 15 Jan 2020 15:03:44 -0800 Subject: [PATCH 052/127] Remove medium vars from base driver --- plugins/providers/virtualbox/driver/base.rb | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/plugins/providers/virtualbox/driver/base.rb b/plugins/providers/virtualbox/driver/base.rb index 5f1d2e486..e552362c2 100644 --- a/plugins/providers/virtualbox/driver/base.rb +++ b/plugins/providers/virtualbox/driver/base.rb @@ -413,12 +413,7 @@ module VagrantPlugins # @param [String] disk_file # @param [Integer] disk_size in bytes def resize_disk(disk_file, disk_size) - # todo: better error handling for this execute - # todo: MEDIUM changes if virtualbox is older than 5. Need a proper check/switch - # Maybe move this into version_4, then version_5 - # if version 4, medium = "hd" - medium = "medium" - execute("modify#{medium}", disk_file, '--resizebyte', disk_size.to_i.to_s) + execute("modifymedium", disk_file, '--resizebyte', disk_size.to_i.to_s) end # Creates a disk. Default format is VDI unless overridden @@ -437,12 +432,7 @@ module VagrantPlugins # @param [String] destination # @param [String] disk_format def clone_disk(source, destination, disk_format) - # todo: MEDIUM changes if virtualbox is older than 5. Need a proper check/switch - # Maybe move this into version_4, then version_5 - # if version 4, medium = "hd" - medium = "medium" - - execute("clone#{medium}", source, destination, '--format', disk_format) + execute("clonemedium", source, destination, '--format', disk_format) end # Controller-Port-Device looks like: From b388b34846872266b035a198146ca149e5ade37b Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 15 Jan 2020 16:00:38 -0800 Subject: [PATCH 053/127] Update todos and method docs --- plugins/kernel_v2/config/disk.rb | 2 -- plugins/providers/virtualbox/cap/configure_disks.rb | 11 +++++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/plugins/kernel_v2/config/disk.rb b/plugins/kernel_v2/config/disk.rb index d1691320d..33e37f16f 100644 --- a/plugins/kernel_v2/config/disk.rb +++ b/plugins/kernel_v2/config/disk.rb @@ -109,8 +109,6 @@ module VagrantPlugins if @primary @name = "vagrant_primary" else - # TODO: Can't rely on `id` because it changes in between vagrant runs - #@name = "vagrant_#{@type.to_s}_#{@id.split("-").last}" @name = nil end end diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index bf9947e0e..1bab9345c 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -57,7 +57,6 @@ module VagrantPlugins elsif compare_disk_state(machine, disk, current_disk) disk_metadata = resize_disk(machine, disk, current_disk) else - # log no need to reconfigure disk, already in desired state LOGGER.info("No further configuration required for disk '#{disk.name}'") disk_metadata = {uuid: current_disk["UUID"], name: disk.name} end @@ -124,6 +123,9 @@ module VagrantPlugins disk_metadata end + # TODO: Possibly consolidate this method and just use `get_port_and_device`, + # and determine which port and device to use from that info instead + # # @param [Vagrant::Machine] machine # @return [String] port - The next available port on a given device def self.get_next_port(machine) @@ -198,11 +200,13 @@ 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} return disk_metadata end + # @param [VagrantPlugins::VirtualboxProvider::Driver] driver + # @param [String] defined_disk_path + # @return [String] destination - The cloned disk def self.vmdk_to_vdi(driver, defined_disk_path) LOGGER.warn("Converting disk '#{defined_disk_path}' from 'vmdk' to 'vdi' format") source = defined_disk_path @@ -213,6 +217,9 @@ module VagrantPlugins destination end + # @param [VagrantPlugins::VirtualboxProvider::Driver] driver + # @param [String] defined_disk_path + # @return [String] destination - The cloned disk def self.vdi_to_vmdk(driver, defined_disk_path) LOGGER.warn("Converting disk '#{defined_disk_path}' from 'vdi' to 'vmdk' format") source = defined_disk_path From 91e9de79a93191625715cf080370a54e68f77676 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 15 Jan 2020 16:00:50 -0800 Subject: [PATCH 054/127] Add warning for currently unsupported disk types --- plugins/providers/virtualbox/cap/configure_disks.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 1bab9345c..78aa0ecd8 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -28,8 +28,10 @@ module VagrantPlugins configured_disks[:disk] << disk_data unless disk_data.empty? elsif disk.type == :floppy # TODO: Write me + machine.ui.warn("Floppy disk configuration not yet supported. Skipping disk #{disk.name}...") elsif disk.type == :dvd # TODO: Write me + machine.ui.warn("DVD disk configuration not yet supported. Skipping disk #{disk.name}...") end end From 4a4a48b69ca744a612506acd1f91c009774453b2 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 15 Jan 2020 16:01:03 -0800 Subject: [PATCH 055/127] More descriptive name for disk data --- plugins/providers/virtualbox/cap/configure_disks.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 78aa0ecd8..0641fbbe0 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -110,9 +110,8 @@ module VagrantPlugins # note: this might not be required... :thinking: needs more testing LOGGER.info("Attempting to create a new disk file '#{disk_file}' of size '#{disk_config.size}' bytes") - var = machine.provider.driver.create_disk(disk_file, disk_config.size, disk_ext.upcase) - # grab uuid from var, might also need disk name - disk_metadata = {uuid: var.split(':').last.strip, name: disk_config.name} + 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} # TODO: Determine what port and device to attach disk to??? # look at guest_info and see what is in use From a3b07a884ac37257b02e5d2ec6e70926981a434b Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 17 Jan 2020 10:05:36 -0800 Subject: [PATCH 056/127] Add way to configure disk extension --- plugins/kernel_v2/config/disk.rb | 18 ++++++++++++++++++ .../virtualbox/cap/configure_disks.rb | 3 ++- templates/locales/en.yml | 2 ++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/plugins/kernel_v2/config/disk.rb b/plugins/kernel_v2/config/disk.rb index 33e37f16f..ead05023c 100644 --- a/plugins/kernel_v2/config/disk.rb +++ b/plugins/kernel_v2/config/disk.rb @@ -11,6 +11,7 @@ module VagrantPlugins #------------------------------------------------------------------- DEFAULT_DISK_TYPES = [:disk, :dvd, :floppy].freeze + DEFAULT_DISK_EXT = ["vdi", "vmdk", "vhd", "vhdx"].freeze # Note: This value is for internal use only # @@ -29,6 +30,11 @@ module VagrantPlugins # @return [Symbol] attr_accessor :type + # Type of disk extension to create. Defaults to `vdi` + # + # @return [String] + attr_accessor :disk_ext + # Size of disk to create # # @return [Integer,String] @@ -61,6 +67,7 @@ module VagrantPlugins @size = UNSET_VALUE @primary = UNSET_VALUE @file = UNSET_VALUE + @disk_ext = UNSET_VALUE # Internal options @id = SecureRandom.uuid @@ -101,6 +108,8 @@ module VagrantPlugins @size = nil if @size == UNSET_VALUE @file = nil if @file == UNSET_VALUE + @disk_ext = "vmdk" if @disk_ext == UNSET_VALUE + if @primary == UNSET_VALUE @primary = false end @@ -127,6 +136,15 @@ module VagrantPlugins types: DEFAULT_DISK_TYPES.join(', ')) end + if !DEFAULT_DISK_EXT.include?(@disk_ext) + errors << I18n.t("vagrant.config.disk.invalid_ext", ext: @disk_ext, + name: @name, exts: DEFAULT_DISK_EXT.join(', ') ) + end + + if @disk_ext + @disk_ext = @disk_ext.downcase + end + if @size && !@size.is_a?(Integer) if @size.is_a?(String) @size = Vagrant::Util::Numeric.string_to_bytes(@size) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 0641fbbe0..73e5e63f0 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -95,7 +95,8 @@ module VagrantPlugins disk_provider_config = disk_config.provider_config[:virtualbox] if disk_config.provider_config guest_folder = File.dirname(guest_info["CfgFile"]) - disk_ext = "vdi" + + disk_ext = disk_config.disk_ext if disk_provider_config if disk_provider_config.include?(:disk_type) diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 2725b2314..5a7b098a7 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -1802,6 +1802,8 @@ en: #------------------------------------------------------------------------------- config: disk: + 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: |- Disk type '%{type}' is not a valid type. Please pick one of the following supported disk types: %{types} invalid_size: |- From 1e2570a599704994291521f228ecb026e2d5d49d Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 17 Jan 2020 10:47:35 -0800 Subject: [PATCH 057/127] Add methods for controller attaching --- .../virtualbox/cap/configure_disks.rb | 32 +++++++++++++------ plugins/providers/virtualbox/driver/base.rb | 6 +++- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 73e5e63f0..7cd17c412 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -8,6 +8,8 @@ module VagrantPlugins module ConfigureDisks LOGGER = Log4r::Logger.new("vagrant::plugins::virtualbox::configure_disks") + MAX_DISK_NUMER = 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 @@ -16,6 +18,13 @@ module VagrantPlugins return if !Vagrant::Util::Experimental.feature_enabled?("virtualbox_disk_hdd") + if defined_disks.size > MAX_DISK_NUMER + # TODO: THORW AN ERROR HERE + # + # you can only attach up to 30 disks per controller, INCLUDING the primary disk + raise Exception, "fix me" + end + machine.ui.info("Configuring storage mediums...") current_disks = machine.provider.driver.list_hdds @@ -118,33 +127,38 @@ module VagrantPlugins # look at guest_info and see what is in use # need to get the _correct_ port and device to attach disk to # Port is easy (pick the "next one" available), but what about device??? can you have more than one device per controller? - port = get_next_port(machine) - device = "0" # TODO: Fix me - machine.provider.driver.attach_disk(port, device, disk_file) + # + # Stderr: VBoxManage: error: The port and/or device parameter are out of range: port=30 (must be in range [0, 29]), device=0 (must be in range [0, 0]) + + dsk_controller_info = get_next_port_device(machine) + machine.provider.driver.attach_disk(dsk_controller_info[:port], dsk_controller_info[:device], disk_file) disk_metadata end - # TODO: Possibly consolidate this method and just use `get_port_and_device`, - # and determine which port and device to use from that info instead + # Finds the next available port and or device for a given controller # # @param [Vagrant::Machine] machine - # @return [String] port - The next available port on a given device - def self.get_next_port(machine) + # @return [Hash] dsk_info - The next available port and device on a given controller + def self.get_next_port_device(machine) vm_info = machine.provider.driver.show_vm_info + dsk_info = {device: "0", port: "0"} port = 0 + device = 0 vm_info.each do |key,value| if key.include?("ImageUUID") disk_info = key.split("-") port = disk_info[2] + device = disk_info[3] else next end end - port = (port.to_i + 1).to_s - port + dsk_info[:port] = (port.to_i + 1).to_s + + dsk_info end # @param [Hash] vm_info - Guest info from show_vm_info diff --git a/plugins/providers/virtualbox/driver/base.rb b/plugins/providers/virtualbox/driver/base.rb index e552362c2..a2d18b4f5 100644 --- a/plugins/providers/virtualbox/driver/base.rb +++ b/plugins/providers/virtualbox/driver/base.rb @@ -449,10 +449,14 @@ module VagrantPlugins # Maybe only support SATA Controller for `:disk`??? controller = "SATA Controller" - # todo: hard set to type hdd, need to look if all types are compatible with these flags execute('storageattach', @uuid, '--storagectl', controller, '--port', port.to_s, '--device', device.to_s, '--type', type, '--medium', file) end + def attach_sata_controller(name=nil) + name = 'SATA Controller' if !name + execute('storagectl', @uuid, '--name', name, '--add', 'sata', '--controller', 'IntelAhci', '--portcount', "30") + end + # Removes a disk from the given virtual machine # # @param [String] disk_uuid or file path From 2176216bf83ffefdd522878ff80ef8b85cb51e3d Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 17 Jan 2020 12:48:56 -0800 Subject: [PATCH 058/127] Add error for when disks defined are greater than the max limit --- lib/vagrant/errors.rb | 4 ++++ plugins/providers/virtualbox/cap/configure_disks.rb | 4 +--- templates/locales/en.yml | 5 +++++ 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index 90e5c42c1..ab7584eed 100644 --- a/lib/vagrant/errors.rb +++ b/lib/vagrant/errors.rb @@ -904,6 +904,10 @@ module Vagrant error_key(:virtualbox_broken_version_040214) end + class VirtualBoxDisksDefinedExceedLimit < VagrantError + error_key(:virtualbox_disks_defined_exceed_limit) + end + class VirtualBoxGuestPropertyNotFound < VagrantError error_key(:virtualbox_guest_property_not_found) end diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 7cd17c412..e2845a48c 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -19,10 +19,8 @@ module VagrantPlugins return if !Vagrant::Util::Experimental.feature_enabled?("virtualbox_disk_hdd") if defined_disks.size > MAX_DISK_NUMER - # TODO: THORW AN ERROR HERE - # # you can only attach up to 30 disks per controller, INCLUDING the primary disk - raise Exception, "fix me" + raise Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit end machine.ui.info("Configuring storage mediums...") diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 5a7b098a7..6b6e5a9e9 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -1641,6 +1641,11 @@ 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. + + Please ensure only up to 30 disks are configured for your guest. virtualbox_guest_property_not_found: |- Could not find a required VirtualBox guest property: %{guest_property} From fddcd71eaf8cc287ea9e674997dd6ee0f1ab2803 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 17 Jan 2020 13:14:52 -0800 Subject: [PATCH 059/127] Default to vdi since virtualbox default is vdi --- plugins/kernel_v2/config/disk.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/kernel_v2/config/disk.rb b/plugins/kernel_v2/config/disk.rb index ead05023c..e443f5f8a 100644 --- a/plugins/kernel_v2/config/disk.rb +++ b/plugins/kernel_v2/config/disk.rb @@ -108,7 +108,7 @@ module VagrantPlugins @size = nil if @size == UNSET_VALUE @file = nil if @file == UNSET_VALUE - @disk_ext = "vmdk" if @disk_ext == UNSET_VALUE + @disk_ext = "vdi" if @disk_ext == UNSET_VALUE if @primary == UNSET_VALUE @primary = false From 17b75db35b95d614346c1208f14fd1973c623032 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 17 Jan 2020 14:46:05 -0800 Subject: [PATCH 060/127] Move disk driver methods into virtualbox version 5 --- plugins/providers/virtualbox/driver/base.rb | 86 ------------------- plugins/providers/virtualbox/driver/meta.rb | 10 ++- .../virtualbox/driver/version_5_0.rb | 84 ++++++++++++++++++ 3 files changed, 93 insertions(+), 87 deletions(-) diff --git a/plugins/providers/virtualbox/driver/base.rb b/plugins/providers/virtualbox/driver/base.rb index a2d18b4f5..c2f79e40b 100644 --- a/plugins/providers/virtualbox/driver/base.rb +++ b/plugins/providers/virtualbox/driver/base.rb @@ -383,92 +383,6 @@ module VagrantPlugins info end - # Lists all attached harddisks from a given virtual machine. Additionally, - # this method adds a new key "Disk Name" based on the disks file path from "Location" - # - # @return [Array] hdds An array of hashes of harddrive info for a guest - def list_hdds - hdds = [] - tmp_drive = {} - execute('list', 'hdds', retryable: true).split("\n").each do |line| - if line == "" # separator between disks - hdds << tmp_drive - tmp_drive = {} - next - end - parts = line.partition(":") - key = parts.first.strip - value = parts.last.strip - tmp_drive[key] = value - - if key == "Location" - tmp_drive["Disk Name"] = File.basename(value, ".*") - end - end - hdds << tmp_drive unless tmp_drive.empty? - - hdds - end - - # @param [String] disk_file - # @param [Integer] disk_size in bytes - def resize_disk(disk_file, disk_size) - execute("modifymedium", disk_file, '--resizebyte', disk_size.to_i.to_s) - end - - # Creates a disk. Default format is VDI unless overridden - # - # Note: disk_size must be divisible by 512 bytes, otherwise operation will fail - # Source: https://www.virtualbox.org/ticket/5582 - # - # @param [String] disk_file - # @param [Integer] disk_size - size in bytes (MUST BE DIVISIBLE BY 512 bytes) - # @param [String] disk_format - format of disk, defaults to "VDI" - def create_disk(disk_file, disk_size, disk_format="VDI") - execute("createmedium", '--filename', disk_file, '--sizebyte', disk_size.to_i.to_s, '--format', disk_format) - end - - # @param [String] source - # @param [String] destination - # @param [String] disk_format - def clone_disk(source, destination, disk_format) - execute("clonemedium", source, destination, '--format', disk_format) - end - - # Controller-Port-Device looks like: - # SATA Controller-ImageUUID-0-0 (sub out ImageUUID) - # - Controller: SATA Controller - # - Port: 0 - # - Device: 0 - # - # @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 - def attach_disk(port, device, file, type="hdd") - # Maybe only support SATA Controller for `:disk`??? - controller = "SATA Controller" - - execute('storageattach', @uuid, '--storagectl', controller, '--port', port.to_s, '--device', device.to_s, '--type', type, '--medium', file) - end - - def attach_sata_controller(name=nil) - name = 'SATA Controller' if !name - execute('storagectl', @uuid, '--name', name, '--add', 'sata', '--controller', 'IntelAhci', '--portcount', "30") - end - - # Removes a disk from the given virtual machine - # - # @param [String] disk_uuid or file path - def close_medium(disk_uuid) - execute("closemedium", disk_uuid, '--delete') - end - - def remove_disk(port, device) - controller = "SATA Controller" - execute('storageattach', @uuid, '--storagectl', controller, '--port', port.to_s, '--device', device.to_s, '--medium', "none") - end - # Execute the given subcommand for VBoxManage and return the output. def execute(*command, &block) # Get the options hash if it exists diff --git a/plugins/providers/virtualbox/driver/meta.rb b/plugins/providers/virtualbox/driver/meta.rb index ed013737b..e634635d3 100644 --- a/plugins/providers/virtualbox/driver/meta.rb +++ b/plugins/providers/virtualbox/driver/meta.rb @@ -97,10 +97,15 @@ module VagrantPlugins end end - def_delegators :@driver, :clear_forwarded_ports, + def_delegators :@driver, + :attach_disk, + :clear_forwarded_ports, :clear_shared_folders, + :clone_disk, :clonevm, + :close_medium, :create_dhcp_server, + :create_disk, :create_host_only_network, :create_snapshot, :delete, @@ -114,6 +119,7 @@ module VagrantPlugins :halt, :import, :list_snapshots, + :list_hdds, :read_forwarded_ports, :read_bridged_interfaces, :read_dhcp_servers, @@ -130,6 +136,8 @@ module VagrantPlugins :read_vms, :reconfig_host_only, :remove_dhcp_server, + :remove_disk, + :resize_disk, :restore_snapshot, :resume, :set_mac_address, diff --git a/plugins/providers/virtualbox/driver/version_5_0.rb b/plugins/providers/virtualbox/driver/version_5_0.rb index aff0607ee..9390e9a2b 100644 --- a/plugins/providers/virtualbox/driver/version_5_0.rb +++ b/plugins/providers/virtualbox/driver/version_5_0.rb @@ -16,6 +16,23 @@ module VagrantPlugins @uuid = uuid end + # Controller-Port-Device looks like: + # SATA Controller-ImageUUID-0-0 (sub out ImageUUID) + # - Controller: SATA Controller + # - Port: 0 + # - Device: 0 + # + # @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 + def attach_disk(port, device, file, type="hdd") + # Maybe only support SATA Controller for `:disk`??? + controller = "SATA Controller" + + execute('storageattach', @uuid, '--storagectl', controller, '--port', port.to_s, '--device', device.to_s, '--type', type, '--medium', file) + end + def clear_forwarded_ports retryable(on: Vagrant::Errors::VBoxManageError, tries: 3, sleep: 1) do args = [] @@ -38,6 +55,13 @@ module VagrantPlugins end end + # @param [String] source + # @param [String] destination + # @param [String] disk_format + def clone_disk(source, destination, disk_format) + execute("clonemedium", source, destination, '--format', disk_format) + end + def clonevm(master_id, snapshot_name) machine_name = "temp_clone_#{(Time.now.to_f * 1000.0).to_i}_#{rand(100000)}" args = ["--register", "--name", machine_name] @@ -49,6 +73,13 @@ module VagrantPlugins return get_machine_id(machine_name) end + # Removes a disk from the given virtual machine + # + # @param [String] disk_uuid or file path + def close_medium(disk_uuid) + execute("closemedium", disk_uuid, '--delete') + end + def create_dhcp_server(network, options) retryable(on: Vagrant::Errors::VBoxManageError, tries: 3, sleep: 1) do begin @@ -65,6 +96,19 @@ module VagrantPlugins end end + # Creates a disk. Default format is VDI unless overridden + # + # Note: disk_size must be divisible by 512 bytes, otherwise operation will fail + # Source: https://www.virtualbox.org/ticket/5582 + # + # @param [String] disk_file + # @param [Integer] disk_size - size in bytes (MUST BE DIVISIBLE BY 512 bytes) + # @param [String] disk_format - format of disk, defaults to "VDI" + def create_disk(disk_file, disk_size, disk_format="VDI") + execute("createmedium", '--filename', disk_file, '--sizebyte', disk_size.to_i.to_s, '--format', disk_format) + end + + def create_host_only_network(options) # Create the interface execute("hostonlyif", "create", retryable: true) =~ /^Interface '(.+?)' was successfully created$/ @@ -130,6 +174,33 @@ module VagrantPlugins end end + # Lists all attached harddisks from a given virtual machine. Additionally, + # this method adds a new key "Disk Name" based on the disks file path from "Location" + # + # @return [Array] hdds An array of hashes of harddrive info for a guest + def list_hdds + hdds = [] + tmp_drive = {} + execute('list', 'hdds', retryable: true).split("\n").each do |line| + if line == "" # separator between disks + hdds << tmp_drive + tmp_drive = {} + next + end + parts = line.partition(":") + key = parts.first.strip + value = parts.last.strip + tmp_drive[key] = value + + if key == "Location" + tmp_drive["Disk Name"] = File.basename(value, ".*") + end + end + hdds << tmp_drive unless tmp_drive.empty? + + hdds + end + def list_snapshots(machine_id) output = execute( "snapshot", machine_id, "list", "--machinereadable", @@ -149,6 +220,19 @@ module VagrantPlugins raise end + # @param [String] port - port on device to attach disk to + # @param [String] device - device on controller for disk + def remove_disk(port, device) + controller = "SATA Controller" + execute('storageattach', @uuid, '--storagectl', controller, '--port', port.to_s, '--device', device.to_s, '--medium', "none") + end + + # @param [String] disk_file + # @param [Integer] disk_size in bytes + def resize_disk(disk_file, disk_size) + execute("modifymedium", disk_file, '--resizebyte', disk_size.to_i.to_s) + end + def restore_snapshot(machine_id, snapshot_name) # Start with 0% last = 0 From cb3cc42dce5af1621e241d8ecd6ffa180f93e097 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 17 Jan 2020 15:03:17 -0800 Subject: [PATCH 061/127] Determine primary disk through vm_info rather than list_hdds --- plugins/providers/virtualbox/cap/configure_disks.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index e2845a48c..ae5fcee57 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -51,11 +51,17 @@ module VagrantPlugins # @param [Hash] - disk_metadata def self.handle_configure_disk(machine, disk, all_disks) disk_metadata = {} + # Grab the existing configured disk, if it exists current_disk = nil if disk.primary - # TODO: This instead might need to be determined through the show_vm_info data instead - current_disk = all_disks.first + # 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"] + + current_disk = all_disks.select { |d| d["UUID"] == primary_uuid }.first else current_disk = all_disks.select { |d| d["Disk Name"] == disk.name}.first end From 524d98fe5c715ee2984e07632ea49a9b63f43a5b Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 17 Jan 2020 15:06:22 -0800 Subject: [PATCH 062/127] Add comment about MAX_DISK_SIZE const --- plugins/providers/virtualbox/cap/configure_disks.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index ae5fcee57..eaaaeea6e 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -8,6 +8,7 @@ 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_NUMER = 30.freeze # @param [Vagrant::Machine] machine From 6676fe6f7f44c4266e85c62b984af165ee9491d0 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 17 Jan 2020 15:06:36 -0800 Subject: [PATCH 063/127] Remove stale TODOs --- .../providers/virtualbox/cap/configure_disks.rb | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index eaaaeea6e..accc8c25a 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -111,30 +111,13 @@ module VagrantPlugins guest_folder = File.dirname(guest_info["CfgFile"]) disk_ext = disk_config.disk_ext - - if disk_provider_config - if disk_provider_config.include?(:disk_type) - disk_ext = disk_provider_config[:disk_type].downcase - end - end disk_file = File.join(guest_folder, disk_config.name) + ".#{disk_ext}" - # TODO: Round disk_config.size to the nearest 512 bytes to make it divisble by 512 - # Source: https://www.virtualbox.org/ticket/5582 - # - # note: this might not be required... :thinking: needs more testing 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} - # TODO: Determine what port and device to attach disk to??? - # look at guest_info and see what is in use - # need to get the _correct_ port and device to attach disk to - # Port is easy (pick the "next one" available), but what about device??? can you have more than one device per controller? - # - # Stderr: VBoxManage: error: The port and/or device parameter are out of range: port=30 (must be in range [0, 29]), device=0 (must be in range [0, 0]) - dsk_controller_info = get_next_port_device(machine) machine.provider.driver.attach_disk(dsk_controller_info[:port], dsk_controller_info[:device], disk_file) From c049dd9211a5dc6d547d92b6d3c8ae0d4628843a Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 17 Jan 2020 15:19:49 -0800 Subject: [PATCH 064/127] Rename method to match functionality This method now only returns the next available port, as virtualbox guests can only have 1 device controller. --- plugins/providers/virtualbox/cap/configure_disks.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index accc8c25a..ab2397fb0 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -118,17 +118,17 @@ module VagrantPlugins disk_var = machine.provider.driver.create_disk(disk_file, disk_config.size, disk_ext.upcase) disk_metadata = {uuid: disk_var.split(':').last.strip, name: disk_config.name} - dsk_controller_info = get_next_port_device(machine) + dsk_controller_info = get_next_port(machine) machine.provider.driver.attach_disk(dsk_controller_info[:port], dsk_controller_info[:device], disk_file) disk_metadata end - # Finds the next available port and or device for a given controller + # Finds the next available port # # @param [Vagrant::Machine] machine # @return [Hash] dsk_info - The next available port and device on a given controller - def self.get_next_port_device(machine) + def self.get_next_port(machine) vm_info = machine.provider.driver.show_vm_info dsk_info = {device: "0", port: "0"} From 637696220959c9dc0d4e6d36b551def553fba211 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 17 Jan 2020 15:20:31 -0800 Subject: [PATCH 065/127] Simplify obtaining port and device number info --- .../virtualbox/cap/configure_disks.rb | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index ab2397fb0..0646047a4 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -154,18 +154,16 @@ module VagrantPlugins # @return [Hash] disk - A hash with `port` and `device` keys found from a matching UUID in vm_info def self.get_port_and_device(vm_info, defined_disk) disk = {} - vm_info.each do |key,value| - if key.include?("ImageUUID") && value == defined_disk["UUID"] - disk_info = key.split("-") - disk[:port] = disk_info[2] - disk[:device] = disk_info[3] - break - else - next - end - end + disk_info_key = vm_info.key(defined_disk["UUID"]) + # TODO: Should we do something else here if the UUID cannot be found? + return disk if !disk_info_key - disk + disk_info = disk_info_key.split("-") + + disk[:port] = disk_info[2] + disk[:device] = disk_info[3] + + return disk end def self.resize_disk(machine, disk_config, defined_disk) From 8407fb28cbf731bfbbf13b8166f344ba6077e134 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 17 Jan 2020 15:22:46 -0800 Subject: [PATCH 066/127] Add more code comments for configuring disk class --- plugins/providers/virtualbox/cap/configure_disks.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 0646047a4..386e7bd5d 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -171,9 +171,10 @@ module VagrantPlugins 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 disks port and device number + # grab disk to be resized port and device number vm_info = machine.provider.driver.show_vm_info disk_info = get_port_and_device(vm_info, defined_disk) + # clone disk to vdi formatted disk vdi_disk_file = vmdk_to_vdi(machine.provider.driver, defined_disk["Location"]) # resize vdi @@ -183,13 +184,14 @@ module VagrantPlugins machine.provider.driver.remove_disk(disk_info[:port], disk_info[:device]) machine.provider.driver.close_medium(defined_disk["UUID"]) + # clone back to original vmdk format and attach resized disk vmdk_disk_file = vdi_to_vmdk(machine.provider.driver, vdi_disk_file) machine.provider.driver.attach_disk(disk_info[:port], disk_info[:device], vmdk_disk_file, "hdd") # close cloned volume format machine.provider.driver.close_medium(vdi_disk_file) - # Get new disk UUID for vagrant disk_meta file + # Get new updated disk UUID for vagrant disk_meta file new_disk_info = machine.provider.driver.list_hdds.select { |h| h["Location"] == defined_disk["Location"] }.first defined_disk = new_disk_info @@ -206,6 +208,8 @@ module VagrantPlugins return disk_metadata end + # TODO: Maybe these should be virtualbox driver methods? + # @param [VagrantPlugins::VirtualboxProvider::Driver] driver # @param [String] defined_disk_path # @return [String] destination - The cloned disk From 1b5f8760d04e0482afe6dceae98aa3b62af2393d Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 17 Jan 2020 15:40:44 -0800 Subject: [PATCH 067/127] Simplify get_port_device and add note about duplicated method in class --- .../providers/virtualbox/cap/cleanup_disks.rb | 30 +++++++++---------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/plugins/providers/virtualbox/cap/cleanup_disks.rb b/plugins/providers/virtualbox/cap/cleanup_disks.rb index 5891f40b1..f4441de53 100644 --- a/plugins/providers/virtualbox/cap/cleanup_disks.rb +++ b/plugins/providers/virtualbox/cap/cleanup_disks.rb @@ -21,25 +21,23 @@ module VagrantPlugins protected + # TODO: This method is duplicated in configure_disks + # # @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 self.get_device_port(vm_info, disk_uuid) - disk_info = {device: nil, port: nil} + def self.get_port_and_device(vm_info, disk_uuid) + disk = {} + disk_info_key = vm_info.key(disk_uuid) + # TODO: Should we do something else here if the UUID cannot be found? + return disk if !disk_info_key - vm_info.each do |key,value| - if key.include?("ImageUUID") && - value == disk_uuid - info = key.split("-") - disk_info[:port] = info[2] - disk_info[:device] = info[3] - break - else - next - end - end + disk_info = disk_info_key.split("-") - disk_info + disk[:port] = disk_info[2] + disk[:device] = disk_info[3] + + return disk end # @param [Vagrant::Machine] machine @@ -54,10 +52,10 @@ module VagrantPlugins next else LOGGER.warn("Found disk not in Vagrantfile config: '#{d["name"]}'. Removing disk from guest #{machine.name}") - disk_info = get_device_port(vm_info, d["uuid"]) + disk_info = get_port_and_device(vm_info, d["uuid"]) # TODO: add proper vagrant error here with values - if disk_info.values.any?(nil) + if disk_info.empty? raise Error, "could not determine device and port to remove disk" end From 98a81aac6bf3e0596704a140fcd90dd08ed26278 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 17 Jan 2020 15:41:00 -0800 Subject: [PATCH 068/127] Move show_vm_info call --- plugins/providers/virtualbox/cap/configure_disks.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 386e7bd5d..2d33ff54f 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -105,9 +105,9 @@ module VagrantPlugins # @param [Kernel_V2::VagrantConfigDisk] disk_config def self.create_disk(machine, disk_config) machine.ui.detail("Disk '#{disk_config.name}' not found in guest. Creating and attaching disk to guest...") - guest_info = machine.provider.driver.show_vm_info disk_provider_config = disk_config.provider_config[:virtualbox] if disk_config.provider_config + guest_info = machine.provider.driver.show_vm_info guest_folder = File.dirname(guest_info["CfgFile"]) disk_ext = disk_config.disk_ext From 9a5ce8381b69ba1a9836e9133922f7cb86d02825 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 17 Jan 2020 15:58:49 -0800 Subject: [PATCH 069/127] Add comment field for new disks --- plugins/providers/virtualbox/driver/version_5_0.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/plugins/providers/virtualbox/driver/version_5_0.rb b/plugins/providers/virtualbox/driver/version_5_0.rb index 9390e9a2b..bb8cc3c06 100644 --- a/plugins/providers/virtualbox/driver/version_5_0.rb +++ b/plugins/providers/virtualbox/driver/version_5_0.rb @@ -30,7 +30,11 @@ module VagrantPlugins # Maybe only support SATA Controller for `:disk`??? controller = "SATA Controller" - execute('storageattach', @uuid, '--storagectl', controller, '--port', port.to_s, '--device', device.to_s, '--type', type, '--medium', file) + 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) end def clear_forwarded_ports From f6a5a3ae41cd98018c0e77ff185f9133891f4746 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 21 Jan 2020 13:54:43 -0800 Subject: [PATCH 070/127] Simplify determining next port to use --- plugins/providers/virtualbox/cap/configure_disks.rb | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 2d33ff54f..da715e68b 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -134,14 +134,11 @@ module VagrantPlugins port = 0 device = 0 - vm_info.each do |key,value| - if key.include?("ImageUUID") - disk_info = key.split("-") - port = disk_info[2] - device = disk_info[3] - else - next - end + disk_images = vm_info.select { |v| v.include?("ImageUUID") } + disk_images.each do |key,value| + disk_info = key.split("-") + port = disk_info[2] + device = disk_info[3] end dsk_info[:port] = (port.to_i + 1).to_s From 2db81b049b8dced96d63eb12413886122bd63a30 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 21 Jan 2020 14:22:56 -0800 Subject: [PATCH 071/127] Only look at SATA Controller disk images --- plugins/providers/virtualbox/cap/configure_disks.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index da715e68b..ca26a4a0f 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -134,7 +134,8 @@ module VagrantPlugins port = 0 device = 0 - disk_images = vm_info.select { |v| v.include?("ImageUUID") } + + disk_images = vm_info.select { |v| v.include?("ImageUUID") && v.include?("SATA Controller") } disk_images.each do |key,value| disk_info = key.split("-") port = disk_info[2] From c0fb8af1a3265a484fae6c15073daf3d55fff03c Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 21 Jan 2020 15:25:47 -0800 Subject: [PATCH 072/127] Fix NUMBER typo --- plugins/providers/virtualbox/cap/configure_disks.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index ca26a4a0f..64efe4d7c 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -9,7 +9,7 @@ module VagrantPlugins 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_NUMER = 30.freeze + MAX_DISK_NUMBER = 30.freeze # @param [Vagrant::Machine] machine # @param [VagrantPlugins::Kernel_V2::VagrantConfigDisk] defined_disks From 3d4d0be58f812a40ffaa14ee576281afddb1e0e7 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 21 Jan 2020 15:38:47 -0800 Subject: [PATCH 073/127] Attach disk to next available port to avoid port fragmentation This commit ensures that vagrant attaches a new disk to the _next available_ port, rather than the last one "used". This can occur if a disk has been removed in a previous run, leaving a port open --- .../virtualbox/cap/configure_disks.rb | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 64efe4d7c..b9e51d4fe 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -19,7 +19,7 @@ module VagrantPlugins return if !Vagrant::Util::Experimental.feature_enabled?("virtualbox_disk_hdd") - if defined_disks.size > MAX_DISK_NUMER + 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 @@ -126,23 +126,30 @@ module VagrantPlugins # Finds the next available port # + # SATA Controller-ImageUUID-0-0 (sub out ImageUUID) + # - Controller: SATA Controller + # - Port: 0 + # - Device: 0 + # + # Note: Virtualbox returns the string above with the port and device info + # disk_info = key.split("-") + # port = disk_info[2] + # device = disk_info[3] + # # @param [Vagrant::Machine] machine # @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"} - port = 0 - device = 0 - disk_images = vm_info.select { |v| v.include?("ImageUUID") && v.include?("SATA Controller") } - disk_images.each do |key,value| - disk_info = key.split("-") - port = disk_info[2] - device = disk_info[3] - end + 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 - dsk_info[:port] = (port.to_i + 1).to_s + dsk_info[:port] = next_available_port.to_s + if dsk_info[:port] == "" + LOGGER.warn("There are no more available ports to attach disks to for the SATA Controller. Clear up some space to attach new disks.") + end dsk_info end From df742603cbef7a9fa250f5d4b815c546cdc717a8 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 21 Jan 2020 15:40:54 -0800 Subject: [PATCH 074/127] Add code comment about when SATA controller has no more open ports --- plugins/providers/virtualbox/cap/configure_disks.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index b9e51d4fe..076672b4a 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -148,7 +148,8 @@ module VagrantPlugins dsk_info[:port] = next_available_port.to_s if dsk_info[:port] == "" - LOGGER.warn("There are no more available ports to attach disks to for the SATA Controller. Clear up some space to attach new disks.") + # 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.") end dsk_info From ca26afa299e479601cf9e797ec520ad0c359b053 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 22 Jan 2020 13:56:59 -0800 Subject: [PATCH 075/127] Don't write disk_meta file if no disks to configure --- lib/vagrant/action/builtin/disk.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/vagrant/action/builtin/disk.rb b/lib/vagrant/action/builtin/disk.rb index 7cef91ec5..8fc776f95 100644 --- a/lib/vagrant/action/builtin/disk.rb +++ b/lib/vagrant/action/builtin/disk.rb @@ -14,6 +14,7 @@ module Vagrant defined_disks = get_disks(machine, env) # Call into providers machine implementation for disk management + configured_disks = {} if !defined_disks.empty? if machine.provider.capability?(:configure_disks) configured_disks = machine.provider.capability(:configure_disks, defined_disks) @@ -23,7 +24,7 @@ module Vagrant end end - write_disk_metadata(machine, configured_disks) + write_disk_metadata(machine, configured_disks) unless configured_disks.empty? # Continue On @app.call(env) From 2e53c21fea5b4c2bd1db877aa347399ec255a5a1 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 22 Jan 2020 13:58:00 -0800 Subject: [PATCH 076/127] Return empty hash instead of null from configure_disks --- plugins/providers/virtualbox/cap/configure_disks.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 076672b4a..432e2d827 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -15,9 +15,9 @@ module VagrantPlugins # @param [VagrantPlugins::Kernel_V2::VagrantConfigDisk] defined_disks # @return [Hash] configured_disks - A hash of all the current configured disks def self.configure_disks(machine, defined_disks) - return if defined_disks.empty? + return {} if defined_disks.empty? - return if !Vagrant::Util::Experimental.feature_enabled?("virtualbox_disk_hdd") + return {} if !Vagrant::Util::Experimental.feature_enabled?("virtualbox_disk_hdd") if defined_disks.size > MAX_DISK_NUMBER # you can only attach up to 30 disks per controller, INCLUDING the primary disk From d215d9d785f8abc45aadfcf5f03285fa858eb89b Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 22 Jan 2020 14:58:03 -0800 Subject: [PATCH 077/127] Only log warning when cleaning up disks If a disk exists but isn't attached to a guest, don't attempt to remove disk from guest. --- plugins/providers/virtualbox/cap/cleanup_disks.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/plugins/providers/virtualbox/cap/cleanup_disks.rb b/plugins/providers/virtualbox/cap/cleanup_disks.rb index f4441de53..d2ba18631 100644 --- a/plugins/providers/virtualbox/cap/cleanup_disks.rb +++ b/plugins/providers/virtualbox/cap/cleanup_disks.rb @@ -54,14 +54,14 @@ module VagrantPlugins LOGGER.warn("Found disk not in Vagrantfile config: '#{d["name"]}'. Removing disk from guest #{machine.name}") disk_info = get_port_and_device(vm_info, d["uuid"]) - # TODO: add proper vagrant error here with values - if disk_info.empty? - raise Error, "could not determine device and port to remove disk" - end - machine.ui.warn("Disk '#{d["name"]}' no longer exists in Vagrant config. Removing and closing medium from guest...", prefix: true) - machine.provider.driver.remove_disk(disk_info[:port], disk_info[:device]) + if disk_info.empty? + LOGGER.warn("Disk '#{d["name"]}' not attached to guest, but still exists.") + else + machine.provider.driver.remove_disk(disk_info[:port], disk_info[:device]) + end + machine.provider.driver.close_medium(d["uuid"]) end end From 236815f2fffd956381d0bcad66221e2472f65921 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 22 Jan 2020 16:21:44 -0800 Subject: [PATCH 078/127] Attempt to connect vagrant managed disks to guest if not attached --- .../providers/virtualbox/cap/configure_disks.rb | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 432e2d827..23d6b5c88 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -73,7 +73,20 @@ module VagrantPlugins elsif compare_disk_state(machine, disk, current_disk) disk_metadata = resize_disk(machine, disk, current_disk) else - LOGGER.info("No further configuration required for disk '#{disk.name}'") + # TODO: What if it needs to be resized? + + vm_info = machine.provider.driver.show_vm_info + disk_info = get_port_and_device(vm_info, current_disk) + 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[:device], + current_disk["Location"]) + else + LOGGER.info("No further configuration required for disk '#{disk.name}'") + end + disk_metadata = {uuid: current_disk["UUID"], name: disk.name} end From e4782e9d6a080de95fe426fc293233fc8394e6fe Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 22 Jan 2020 16:22:55 -0800 Subject: [PATCH 079/127] Add method doc for method --- plugins/providers/virtualbox/cap/cleanup_disks.rb | 1 - plugins/providers/virtualbox/cap/configure_disks.rb | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/plugins/providers/virtualbox/cap/cleanup_disks.rb b/plugins/providers/virtualbox/cap/cleanup_disks.rb index d2ba18631..871fa85c4 100644 --- a/plugins/providers/virtualbox/cap/cleanup_disks.rb +++ b/plugins/providers/virtualbox/cap/cleanup_disks.rb @@ -29,7 +29,6 @@ module VagrantPlugins def self.get_port_and_device(vm_info, disk_uuid) disk = {} disk_info_key = vm_info.key(disk_uuid) - # TODO: Should we do something else here if the UUID cannot be found? return disk if !disk_info_key disk_info = disk_info_key.split("-") diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 23d6b5c88..ea93b42cd 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -168,13 +168,15 @@ module VagrantPlugins dsk_info end + # Looks up a defined_disk for a guests information from virtualbox. If + # no matching UUID is found, it returns an empty hash. + # # @param [Hash] vm_info - Guest info from show_vm_info # @param [Hash] defined_disk - A specific disk with info from list_hdd # @return [Hash] disk - A hash with `port` and `device` keys found from a matching UUID in vm_info def self.get_port_and_device(vm_info, defined_disk) disk = {} disk_info_key = vm_info.key(defined_disk["UUID"]) - # TODO: Should we do something else here if the UUID cannot be found? return disk if !disk_info_key disk_info = disk_info_key.split("-") From 6ea3c3f40e07c65a34fc035f58f239aaf9963dc5 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 22 Jan 2020 16:43:27 -0800 Subject: [PATCH 080/127] Move current_disk work into own method --- .../virtualbox/cap/configure_disks.rb | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index ea93b42cd..beb49386d 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -48,12 +48,11 @@ module VagrantPlugins protected - # Handles all disk configs of type `:disk` - # @param [Hash] - disk_metadata - def self.handle_configure_disk(machine, disk, all_disks) - disk_metadata = {} - - # Grab the existing configured disk, if it exists + # @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 + # @return [Hash] current_disk - Returns the current disk. Returns nil if it doesn't exist + def self.get_current_disk(machine, disk, all_disks) current_disk = nil if disk.primary # Ensure we grab the proper primary disk @@ -67,6 +66,22 @@ module VagrantPlugins current_disk = all_disks.select { |d| d["Disk Name"] == disk.name}.first end + return current_disk + end + + # Handles all disk configs of type `:disk` + # + # @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 + # @return [Hash] - disk_metadata + def self.handle_configure_disk(machine, disk, all_disks) + disk_metadata = {} + + # Grab the existing configured disk, if it exists + current_disk = get_current_disk(machine, disk, all_disks) + + # Configure current disk if !current_disk # create new disk and attach disk_metadata = create_disk(machine, disk) From 72576d30464f7c8003fe7ff438b5a07f4199f1dc Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 22 Jan 2020 16:45:13 -0800 Subject: [PATCH 081/127] Rename method for clarity for what it handles --- plugins/providers/virtualbox/cap/configure_disks.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index beb49386d..cddf1feea 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -85,7 +85,7 @@ module VagrantPlugins if !current_disk # create new disk and attach disk_metadata = create_disk(machine, disk) - elsif compare_disk_state(machine, disk, current_disk) + elsif compare_disk_size(machine, disk, current_disk) disk_metadata = resize_disk(machine, disk, current_disk) else # TODO: What if it needs to be resized? @@ -113,7 +113,7 @@ module VagrantPlugins # @param [Kernel_V2::VagrantConfigDisk] disk_config # @param [Hash] defined_disk # @return [Boolean] - def self.compare_disk_state(machine, disk_config, defined_disk) + def self.compare_disk_size(machine, disk_config, defined_disk) requested_disk_size = Vagrant::Util::Numeric.bytes_to_megabytes(disk_config.size) defined_disk_size = defined_disk["Capacity"].split(" ").first.to_f From 8654b2bb6796e4a0f3317933a2616d6cdbdaf5b5 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 22 Jan 2020 16:47:55 -0800 Subject: [PATCH 082/127] Add note about provider specific disk configs --- plugins/providers/virtualbox/cap/configure_disks.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index cddf1feea..724915de1 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -133,6 +133,8 @@ module VagrantPlugins # @param [Kernel_V2::VagrantConfigDisk] disk_config def self.create_disk(machine, disk_config) machine.ui.detail("Disk '#{disk_config.name}' not found in guest. Creating and attaching disk to guest...") + # NOTE: At the moment, there are no provider specific configs for VirtualBox + # but we grab it anyway for future use. disk_provider_config = disk_config.provider_config[:virtualbox] if disk_config.provider_config guest_info = machine.provider.driver.show_vm_info From 8cd04db602a3304ce0cad80484991dfa6775e99b Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 23 Jan 2020 08:19:28 -0800 Subject: [PATCH 083/127] Update configuration with disk_ext option --- website/source/docs/disks/configuration.html.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/website/source/docs/disks/configuration.html.md b/website/source/docs/disks/configuration.html.md index f9c05cecc..4bbee1386 100644 --- a/website/source/docs/disks/configuration.html.md +++ b/website/source/docs/disks/configuration.html.md @@ -29,9 +29,11 @@ Vagrant Disks has several options that allow users to define and attach disks to ## Disk Options -* `name` (string) - Optional argument to give the disk a name -* `type` (symbol) - The type of disk to manage. This option defaults to `:disk`. Please read the provider specific documentation for supported types. +* `disk_ext` (string) - Optional argument that defines what kind of file +extension a disk should have. 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. +* `name` (string) - Optional argument to give the disk a name * `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. * `provider_config` (hash) - Additional provider specific options for managing a given disk. @@ -41,6 +43,7 @@ Vagrant Disks has several options that allow users to define and attach disks to - The provider name followed by a double underscore, and then the provider specific option for that disk + `{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. +* `type` (symbol) - The type of disk to manage. This option defaults to `:disk`. Please read the provider specific documentation for supported types. **Note:** More specific examples of these can be found under the provider specific disk page. The `provider_config` option will depend on the provider you are using. Please read the provider specific documentation for disk management to learn about what options are available to use. From 718332b35e75ff8ecd111abcb7b94cce1d9f8191 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 23 Jan 2020 08:36:15 -0800 Subject: [PATCH 084/127] Fixup disk tests --- test/unit/plugins/kernel_v2/config/vm_test.rb | 4 ++-- test/unit/vagrant/action/builtin/disk_test.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/unit/plugins/kernel_v2/config/vm_test.rb b/test/unit/plugins/kernel_v2/config/vm_test.rb index 20775ce75..e86db848f 100644 --- a/test/unit/plugins/kernel_v2/config/vm_test.rb +++ b/test/unit/plugins/kernel_v2/config/vm_test.rb @@ -556,8 +556,8 @@ describe VagrantPlugins::Kernel_V2::VMConfig do end it "stores the disks" do - subject.disk(:disk, size: 100) - subject.disk(:disk, size: 1000, primary: false, name: "storage") + subject.disk(:disk, size: 100, primary: true) + subject.disk(:disk, size: 1000, name: "storage") subject.finalize! assert_valid diff --git a/test/unit/vagrant/action/builtin/disk_test.rb b/test/unit/vagrant/action/builtin/disk_test.rb index 05f485602..641de121a 100644 --- a/test/unit/vagrant/action/builtin/disk_test.rb +++ b/test/unit/vagrant/action/builtin/disk_test.rb @@ -20,7 +20,7 @@ describe Vagrant::Action::Builtin::Disk do subject = described_class.new(app, env) expect(app).to receive(:call).with(env).ordered - expect(machine.provider).to receive(:capability).with(:configure_disks, disks) + expect(machine.provider).to receive(:capability).with(:configure_disks, disks).and_return({}) subject.call(env) end From 9ca535dbfad6e1b85dc1b4211fa0986cd6c07cb0 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 23 Jan 2020 09:01:26 -0800 Subject: [PATCH 085/127] Add test for writing disk_meta file --- test/unit/vagrant/action/builtin/disk_test.rb | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/test/unit/vagrant/action/builtin/disk_test.rb b/test/unit/vagrant/action/builtin/disk_test.rb index 641de121a..63f37a812 100644 --- a/test/unit/vagrant/action/builtin/disk_test.rb +++ b/test/unit/vagrant/action/builtin/disk_test.rb @@ -5,13 +5,16 @@ describe Vagrant::Action::Builtin::Disk do let(:vm) { double("vm") } let(:config) { double("config", vm: vm) } let(:provider) { double("provider") } - let(:machine) { double("machine", config: config, provider: provider, provider_name: "provider") } + let(:machine) { double("machine", config: config, provider: provider, + provider_name: "provider", data_dir: Pathname.new("/fake/dir")) } let(:env) { { ui: ui, machine: machine} } let(:disks) { [double("disk")] } let(:ui) { double("ui") } + let(:disk_data) { {disk: [{uuid: "123456789", name: "storage"}], floppy: [], dvd: []} } + describe "#call" do it "calls configure_disks if disk config present" do allow(vm).to receive(:disks).and_return(disks) @@ -20,7 +23,10 @@ describe Vagrant::Action::Builtin::Disk do subject = described_class.new(app, env) expect(app).to receive(:call).with(env).ordered - expect(machine.provider).to receive(:capability).with(:configure_disks, disks).and_return({}) + expect(machine.provider).to receive(:capability). + with(:configure_disks, disks).and_return(disk_data) + + expect(subject).to receive(:write_disk_metadata).and_return(true) subject.call(env) end @@ -32,6 +38,8 @@ describe Vagrant::Action::Builtin::Disk do expect(app).to receive(:call).with(env).ordered expect(machine.provider).not_to receive(:capability).with(:configure_disks, disks) + expect(subject).not_to receive(:write_disk_metadata) + subject.call(env) end @@ -46,5 +54,13 @@ describe Vagrant::Action::Builtin::Disk do subject.call(env) end + + it "writes down a disk_meta file if disks are configured" do + subject = described_class.new(app, env) + + expect(File).to receive(:open).with("/fake/dir/disk_meta", "w+").and_return(true) + + subject.write_disk_metadata(machine, disk_data) + end end end From eff3d47f82501482496114cf545f4fdc67f96b66 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 23 Jan 2020 09:10:31 -0800 Subject: [PATCH 086/127] Add cleanup_disks test --- lib/vagrant/action/builtin/cleanup_disks.rb | 3 + .../action/builtin/cleanup_disks_test.rb | 55 +++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 test/unit/vagrant/action/builtin/cleanup_disks_test.rb diff --git a/lib/vagrant/action/builtin/cleanup_disks.rb b/lib/vagrant/action/builtin/cleanup_disks.rb index 77f29e049..08fbf4c8e 100644 --- a/lib/vagrant/action/builtin/cleanup_disks.rb +++ b/lib/vagrant/action/builtin/cleanup_disks.rb @@ -14,6 +14,9 @@ module Vagrant machine = env[:machine] defined_disks = get_disks(machine, env) + # TODO: Maybe always cleanup disks, even if no disks are defined. + # Check meta file first + # # Call into providers machine implementation for disk management if !defined_disks.empty? if machine.provider.capability?(:cleanup_disks) diff --git a/test/unit/vagrant/action/builtin/cleanup_disks_test.rb b/test/unit/vagrant/action/builtin/cleanup_disks_test.rb new file mode 100644 index 000000000..c79f50a0e --- /dev/null +++ b/test/unit/vagrant/action/builtin/cleanup_disks_test.rb @@ -0,0 +1,55 @@ +require File.expand_path("../../../../base", __FILE__) + +describe Vagrant::Action::Builtin::CleanupDisks do + let(:app) { lambda { |env| } } + let(:vm) { double("vm") } + let(:config) { double("config", vm: vm) } + let(:provider) { double("provider") } + let(:machine) { double("machine", config: config, provider: provider, + provider_name: "provider", data_dir: Pathname.new("/fake/dir")) } + let(:env) { { ui: ui, machine: machine} } + + let(:disks) { [double("disk")] } + + let(:ui) { double("ui") } + + let(:disk_meta_file) { {disk: [{uuid: "123456789", name: "storage"}], floppy: [], dvd: []} } + + describe "#call" do + it "calls configure_disks if disk config present" do + allow(vm).to receive(:disks).and_return(disks) + allow(machine).to receive(:disks).and_return(disks) + allow(machine.provider).to receive(:capability?).with(:cleanup_disks).and_return(true) + subject = described_class.new(app, env) + + expect(app).to receive(:call).with(env).ordered + expect(subject).to receive(:read_disk_metadata).with(machine).and_return(disk_meta_file) + expect(machine.provider).to receive(:capability). + with(:cleanup_disks, disks, disk_meta_file) + + subject.call(env) + end + + it "continues on if no disk config present" do + allow(vm).to receive(:disks).and_return([]) + subject = described_class.new(app, env) + + expect(app).to receive(:call).with(env).ordered + expect(machine.provider).not_to receive(:capability).with(:cleanup_disks, disks) + + subject.call(env) + end + + it "prints a warning if disk config capability is unsupported" do + allow(vm).to receive(:disks).and_return(disks) + allow(machine.provider).to receive(:capability?).with(:cleanup_disks).and_return(false) + subject = described_class.new(app, env) + + expect(app).to receive(:call).with(env).ordered + expect(machine.provider).not_to receive(:capability).with(:cleanup_disks, disks) + expect(ui).to receive(:warn) + + subject.call(env) + end + end +end From 9150d3db7b856f19e4edea93fc77d79e8e75f88d Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 23 Jan 2020 09:20:12 -0800 Subject: [PATCH 087/127] Always cleanup disks even if no disks in Vagrantfile --- lib/vagrant/action/builtin/cleanup_disks.rb | 14 +++++--------- .../vagrant/action/builtin/cleanup_disks_test.rb | 3 ++- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/lib/vagrant/action/builtin/cleanup_disks.rb b/lib/vagrant/action/builtin/cleanup_disks.rb index 08fbf4c8e..27b2f887a 100644 --- a/lib/vagrant/action/builtin/cleanup_disks.rb +++ b/lib/vagrant/action/builtin/cleanup_disks.rb @@ -14,19 +14,15 @@ module Vagrant machine = env[:machine] defined_disks = get_disks(machine, env) - # TODO: Maybe always cleanup disks, even if no disks are defined. - # Check meta file first - # # Call into providers machine implementation for disk management - if !defined_disks.empty? + disk_meta_file = read_disk_metadata(machine) + + if !disk_meta_file.empty? if machine.provider.capability?(:cleanup_disks) - disk_meta_file = read_disk_metadata(machine) - if !disk_meta_file.empty? - machine.provider.capability(:cleanup_disks, defined_disks, disk_meta_file) - end + machine.provider.capability(:cleanup_disks, defined_disks, disk_meta_file) else env[:ui].warn(I18n.t("vagrant.actions.disk.provider_unsupported", - provider: machine.provider_name)) + provider: machine.provider_name)) end end diff --git a/test/unit/vagrant/action/builtin/cleanup_disks_test.rb b/test/unit/vagrant/action/builtin/cleanup_disks_test.rb index c79f50a0e..ed2fbcb88 100644 --- a/test/unit/vagrant/action/builtin/cleanup_disks_test.rb +++ b/test/unit/vagrant/action/builtin/cleanup_disks_test.rb @@ -5,7 +5,7 @@ describe Vagrant::Action::Builtin::CleanupDisks do let(:vm) { double("vm") } let(:config) { double("config", vm: vm) } let(:provider) { double("provider") } - let(:machine) { double("machine", config: config, provider: provider, + let(:machine) { double("machine", config: config, provider: provider, name: "machine", provider_name: "provider", data_dir: Pathname.new("/fake/dir")) } let(:env) { { ui: ui, machine: machine} } @@ -44,6 +44,7 @@ describe Vagrant::Action::Builtin::CleanupDisks do allow(vm).to receive(:disks).and_return(disks) allow(machine.provider).to receive(:capability?).with(:cleanup_disks).and_return(false) subject = described_class.new(app, env) + expect(subject).to receive(:read_disk_metadata).with(machine).and_return(disk_meta_file) expect(app).to receive(:call).with(env).ordered expect(machine.provider).not_to receive(:capability).with(:cleanup_disks, disks) From 252a7f7a4ef5b69743d779972bded0304c0d625a Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 23 Jan 2020 09:43:16 -0800 Subject: [PATCH 088/127] Do not clean up and detach primary disk --- plugins/providers/virtualbox/cap/cleanup_disks.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/providers/virtualbox/cap/cleanup_disks.rb b/plugins/providers/virtualbox/cap/cleanup_disks.rb index 871fa85c4..072d35016 100644 --- a/plugins/providers/virtualbox/cap/cleanup_disks.rb +++ b/plugins/providers/virtualbox/cap/cleanup_disks.rb @@ -44,10 +44,11 @@ module VagrantPlugins # @param [Hash] disk_meta - A hash of all the previously defined disks from the last configure_disk action def self.handle_cleanup_disk(machine, defined_disks, disk_meta) vm_info = machine.provider.driver.show_vm_info + primary_disk = vm_info["SATA Controller-ImageUUID-0-0"] disk_meta.each do |d| dsk = defined_disks.select { |dk| dk.name == d["name"] } - if !dsk.empty? + if !dsk.empty? || d["uuid"] == primary_disk next else LOGGER.warn("Found disk not in Vagrantfile config: '#{d["name"]}'. Removing disk from guest #{machine.name}") From b9a72ce8ffd82effec57bfaa653cdeca4fb8fab1 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 23 Jan 2020 09:43:32 -0800 Subject: [PATCH 089/127] Make get_port_and_device disk method a driver method instead --- .../providers/virtualbox/cap/cleanup_disks.rb | 20 +-------------- .../virtualbox/cap/configure_disks.rb | 25 ++----------------- plugins/providers/virtualbox/driver/meta.rb | 1 + .../virtualbox/driver/version_5_0.rb | 21 ++++++++++++++++ 4 files changed, 25 insertions(+), 42 deletions(-) diff --git a/plugins/providers/virtualbox/cap/cleanup_disks.rb b/plugins/providers/virtualbox/cap/cleanup_disks.rb index 072d35016..1416f042b 100644 --- a/plugins/providers/virtualbox/cap/cleanup_disks.rb +++ b/plugins/providers/virtualbox/cap/cleanup_disks.rb @@ -21,24 +21,6 @@ module VagrantPlugins protected - # TODO: This method is duplicated in configure_disks - # - # @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 self.get_port_and_device(vm_info, disk_uuid) - 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] - - return disk - end - # @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 @@ -52,7 +34,7 @@ module VagrantPlugins next else LOGGER.warn("Found disk not in Vagrantfile config: '#{d["name"]}'. Removing disk from guest #{machine.name}") - disk_info = get_port_and_device(vm_info, d["uuid"]) + disk_info = machine.provider.driver.get_port_and_device(d["uuid"]) machine.ui.warn("Disk '#{d["name"]}' no longer exists in Vagrant config. Removing and closing medium from guest...", prefix: true) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 724915de1..913b2270a 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -90,8 +90,7 @@ module VagrantPlugins else # TODO: What if it needs to be resized? - vm_info = machine.provider.driver.show_vm_info - disk_info = get_port_and_device(vm_info, current_disk) + 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) @@ -185,33 +184,13 @@ module VagrantPlugins dsk_info end - # Looks up a defined_disk for a guests information from virtualbox. If - # no matching UUID is found, it returns an empty hash. - # - # @param [Hash] vm_info - Guest info from show_vm_info - # @param [Hash] defined_disk - A specific disk with info from list_hdd - # @return [Hash] disk - A hash with `port` and `device` keys found from a matching UUID in vm_info - def self.get_port_and_device(vm_info, defined_disk) - disk = {} - disk_info_key = vm_info.key(defined_disk["UUID"]) - return disk if !disk_info_key - - disk_info = disk_info_key.split("-") - - disk[:port] = disk_info[2] - disk[:device] = disk_info[3] - - return disk - end - def self.resize_disk(machine, disk_config, defined_disk) machine.ui.detail("Disk '#{disk_config.name}' needs to be resized. Resizing disk...", prefix: true) 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 - vm_info = machine.provider.driver.show_vm_info - disk_info = get_port_and_device(vm_info, defined_disk) + disk_info = machine.provider.driver.get_port_and_device(defined_disk["UUID"]) # clone disk to vdi formatted disk vdi_disk_file = vmdk_to_vdi(machine.provider.driver, defined_disk["Location"]) diff --git a/plugins/providers/virtualbox/driver/meta.rb b/plugins/providers/virtualbox/driver/meta.rb index e634635d3..470149515 100644 --- a/plugins/providers/virtualbox/driver/meta.rb +++ b/plugins/providers/virtualbox/driver/meta.rb @@ -116,6 +116,7 @@ module VagrantPlugins :execute_command, :export, :forward_ports, + :get_port_and_device, :halt, :import, :list_snapshots, diff --git a/plugins/providers/virtualbox/driver/version_5_0.rb b/plugins/providers/virtualbox/driver/version_5_0.rb index bb8cc3c06..ecb3de985 100644 --- a/plugins/providers/virtualbox/driver/version_5_0.rb +++ b/plugins/providers/virtualbox/driver/version_5_0.rb @@ -383,6 +383,27 @@ module VagrantPlugins nil end + # 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] + + return disk + end + def halt execute("controlvm", @uuid, "poweroff", retryable: true) end From b41147237e32706cec94e2f51004abfbdfcfdde8 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 23 Jan 2020 11:31:51 -0800 Subject: [PATCH 090/127] Begin to add docs for disk configuration with virtualbox --- .../docs/disks/virtualbox/common-issues.html.md | 13 +++++++++++++ website/source/docs/disks/virtualbox/index.html.md | 12 ++++++++++++ website/source/docs/disks/virtualbox/usage.html.md | 10 ++++++++++ website/source/layouts/docs.erb | 7 +++++++ 4 files changed, 42 insertions(+) create mode 100644 website/source/docs/disks/virtualbox/common-issues.html.md create mode 100644 website/source/docs/disks/virtualbox/index.html.md create mode 100644 website/source/docs/disks/virtualbox/usage.html.md diff --git a/website/source/docs/disks/virtualbox/common-issues.html.md b/website/source/docs/disks/virtualbox/common-issues.html.md new file mode 100644 index 000000000..13c987180 --- /dev/null +++ b/website/source/docs/disks/virtualbox/common-issues.html.md @@ -0,0 +1,13 @@ +--- +layout: "docs" +page_title: "Common Issues - Disks VirtualBox Provider" +sidebar_current: "disks-providers-virtualbox-issues" +description: |- + This page lists some common issues people run into with Vagrant and VirtualBox + as well as solutions for those issues. +--- + +# Common Issues + +This page lists some common issues people run into with Vagrant and VirtualBox +as well as solutions for those issues. diff --git a/website/source/docs/disks/virtualbox/index.html.md b/website/source/docs/disks/virtualbox/index.html.md new file mode 100644 index 000000000..23057f235 --- /dev/null +++ b/website/source/docs/disks/virtualbox/index.html.md @@ -0,0 +1,12 @@ +--- +layout: "docs" +page_title: "Disks for VirtualBox Provider" +sidebar_current: "disks-providers-virtualbox" +description: |- + Vagrant comes with support out of the box for VirtualBox, a free, + cross-platform consumer virtualization product. +--- + +# VirtualBox + + diff --git a/website/source/docs/disks/virtualbox/usage.html.md b/website/source/docs/disks/virtualbox/usage.html.md new file mode 100644 index 000000000..f984dc740 --- /dev/null +++ b/website/source/docs/disks/virtualbox/usage.html.md @@ -0,0 +1,10 @@ +--- +layout: "docs" +page_title: "Usage - Disks VirtualBox Provider" +sidebar_current: "disks-providers-virtualbox-usage" +description: |- + The Vagrant VirtualBox provider is used just like any other provider. Please + read the general basic usage page for providers. +--- + +# Usage diff --git a/website/source/layouts/docs.erb b/website/source/layouts/docs.erb index dfd94aa59..5d48c3823 100644 --- a/website/source/layouts/docs.erb +++ b/website/source/layouts/docs.erb @@ -139,6 +139,13 @@ From 337f7dd616520d3b7adae071bc14bb0717142dad Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 23 Jan 2020 13:36:42 -0800 Subject: [PATCH 091/127] Add cleanup_disks tests --- .../virtualbox/cap/cleanup_disks_test.rb | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 test/unit/plugins/providers/virtualbox/cap/cleanup_disks_test.rb diff --git a/test/unit/plugins/providers/virtualbox/cap/cleanup_disks_test.rb b/test/unit/plugins/providers/virtualbox/cap/cleanup_disks_test.rb new file mode 100644 index 000000000..7a64c14d8 --- /dev/null +++ b/test/unit/plugins/providers/virtualbox/cap/cleanup_disks_test.rb @@ -0,0 +1,88 @@ +require_relative "../base" + +require Vagrant.source_root.join("plugins/providers/virtualbox/cap/cleanup_disks") + +describe VagrantPlugins::ProviderVirtualBox::Cap::CleanupDisks do + include_context "unit" + + let(:iso_env) do + # We have to create a Vagrantfile so there is a root path + env = isolated_environment + env.vagrantfile("") + env.create_vagrant_env + end + + let(:driver) { double("driver") } + + let(:machine) do + iso_env.machine(iso_env.machine_names[0], :dummy).tap do |m| + allow(m.provider).to receive(:driver).and_return(driver) + allow(m).to receive(:state).and_return(state) + end + end + + let(:state) do + double(:state) + end + + let(:subject) { described_class } + + 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"} } + + before do + allow(Vagrant::Util::Experimental).to receive(:feature_enabled?).and_return(true) + allow(driver).to receive(:show_vm_info).and_return(vm_info) + end + + context "#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: []} } + + it "calls the cleanup method if a disk_meta file is defined" do + expect(subject).to receive(:handle_cleanup_disk). + with(machine, defined_disks, disk_meta_file["disk"]). + 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: []} } + 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(: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 + it "only closes the medium" do + allow(driver).to receive(:get_port_and_device). + with("67890"). + and_return({}) + + 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 +end From a9e678327bfe402a6521e7f6c1b9846d77e7ce30 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 23 Jan 2020 14:51:57 -0800 Subject: [PATCH 092/127] Update docs for disk name --- website/source/docs/disks/configuration.html.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/source/docs/disks/configuration.html.md b/website/source/docs/disks/configuration.html.md index 4bbee1386..afbea13b3 100644 --- a/website/source/docs/disks/configuration.html.md +++ b/website/source/docs/disks/configuration.html.md @@ -33,7 +33,7 @@ Vagrant Disks has several options that allow users to define and attach disks to extension a disk should have. 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. -* `name` (string) - Optional argument to give the disk a name +* `name` (string) - Required option to give the disk a name. This name will be used as a 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. * `provider_config` (hash) - Additional provider specific options for managing a given disk. From 6872625609d6ebda35d6c517cb1fe07e78be25dc Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 23 Jan 2020 14:52:15 -0800 Subject: [PATCH 093/127] Add more tests for configure_disks virtualbox cap --- .../virtualbox/cap/configure_disks_test.rb | 98 ++++++++++++++++++- 1 file changed, 97 insertions(+), 1 deletion(-) 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 2581c7f2b..9e8694ccb 100644 --- a/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb +++ b/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb @@ -12,8 +12,11 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do env.create_vagrant_env end + let(:driver) { double("driver") } + let(:machine) do iso_env.machine(iso_env.machine_names[0], :dummy).tap do |m| + allow(m.provider).to receive(:driver).and_return(driver) allow(m).to receive(:state).and_return(state) end end @@ -22,6 +25,99 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do double(:state) end - describe "#configure_disks" do + let(:vm_info) { {"SATA Controller-ImageUUID-0-0" => "12345", + "SATA Controller-ImageUUID-1-0" => "67890"} } + + let(:defined_disks) { [double("disk-0", name: "disk-0", size: "5GB", type: :disk), + double("disk-1", name: "disk-1", size: "5GB", type: :disk), + double("disk-2", name: "disk-2", size: "5GB", type: :disk), + double("disk-3", name: "disk-3", size: "5GB", type: :disk)] } + + let(:subject) { described_class } + + before do + allow(Vagrant::Util::Experimental).to receive(:feature_enabled?).and_return(true) + allow(driver).to receive(:show_vm_info).and_return(vm_info) + end + + context "#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([]) + + expect(subject).to receive(:handle_configure_disk).exactly(4).and_return(dsk_data) + subject.configure_disks(machine, defined_disks) + end + + describe "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 } + + it "raises an exception if the disks defined exceed the limit for a SATA Controller" do + expect{subject.configure_disks(machine, defined_disks)}. + to raise_error(Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit) + end + end + end + + context "#get_current_disk" do + it "gets primary disk uuid if disk to configure is primary" do + end + + it "finds the disk to configure" do + end + + it "returns nil if disk is not found" do + end + end + + context "#handle_configure_disk" do + end + + context "#compare_disk_size" do + it "shows a warning if user attempts to shrink size" do + end + + it "returns true if requested size is bigger than current size" do + end + end + + context "#create_disk" do + it "creates a disk and attaches it to a guest" do + end + end + + context "#get_next_port" do + it "determines the next available port to use" do + end + + it "returns empty string if no usable port is available" do + end + end + + context "#resize_disk" do + describe "when a disk is vmdk format" do + it "converts the disk to vdi, resizes it, and converts back to vmdk" do + end + end + + it "resizes the disk" do + end + end + + context "#vmdk_to_vdi" do + it "converts a disk from vmdk to vdi" do + end + end + + context "#vdi_to_vmdk" do + it "converts a disk from vdi to vmdk" do + end end end From 54f17929b51853e0d58fd7cd4d988bfff6499c4c Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 23 Jan 2020 15:00:17 -0800 Subject: [PATCH 094/127] Add note about vbox gui for troubleshooting --- .../source/docs/disks/virtualbox/common-issues.html.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/website/source/docs/disks/virtualbox/common-issues.html.md b/website/source/docs/disks/virtualbox/common-issues.html.md index 13c987180..6331a1dde 100644 --- a/website/source/docs/disks/virtualbox/common-issues.html.md +++ b/website/source/docs/disks/virtualbox/common-issues.html.md @@ -7,7 +7,14 @@ description: |- as well as solutions for those issues. --- -# Common Issues +# Common Issues and Troubleshooting This page lists some common issues people run into with Vagrant and VirtualBox as well as solutions for those issues. + +## VirtualBox GUI + +A handy way to figure out what disks are attached (or not attached) to your guest +is to open up the VirtualBox GUI and select the guest. When selecting a guest on the GUI, +it should open more information about the guest, including storage information. Here +you should see a list of disks attached to your guest. From d1d27a3b5642bb0089a69617bdeea87f1cc7bb54 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 24 Jan 2020 10:34:41 -0800 Subject: [PATCH 095/127] Add tests for handle_configure_disk --- .../virtualbox/cap/configure_disks_test.rb | 149 +++++++++++++++++- 1 file changed, 145 insertions(+), 4 deletions(-) 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 9e8694ccb..3d28857c6 100644 --- a/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb +++ b/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb @@ -28,10 +28,38 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do let(:vm_info) { {"SATA Controller-ImageUUID-0-0" => "12345", "SATA Controller-ImageUUID-1-0" => "67890"} } - let(:defined_disks) { [double("disk-0", name: "disk-0", size: "5GB", type: :disk), - double("disk-1", name: "disk-1", size: "5GB", type: :disk), - double("disk-2", name: "disk-2", size: "5GB", type: :disk), - double("disk-3", name: "disk-3", size: "5GB", type: :disk)] } + 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), + double("disk", name: "disk-2", size: "5GB", primary: false, type: :disk)] } + + let(:all_disks) { [{"UUID"=>"12345", + "Parent UUID"=>"base", + "State"=>"created", + "Type"=>"normal (base)", + "Location"=>"/home/vagrant/VirtualBox VMs/vagrant-sandbox_1579888721946_75923/ubuntu-18.04-amd64-disk001.vmdk", + "Disk Name"=>"ubuntu-18.04-amd64-disk001", + "Storage format"=>"VMDK", + "Capacity"=>"65536 MBytes", + "Encryption"=>"disabled"}, + {"UUID"=>"67890", + "Parent UUID"=>"base", + "State"=>"created", + "Type"=>"normal (base)", + "Location"=>"/home/vagrant/VirtualBox VMs/vagrant-sandbox_1579888721946_75923/disk-0.vdi", + "Disk Name"=>"disk-0", + "Storage format"=>"VDI", + "Capacity"=>"10240 MBytes", + "Encryption"=>"disabled"}, + {"UUID"=>"324bbb53-d5ad-45f8-9bfa-1f2468b199a8", + "Parent UUID"=>"base", + "State"=>"created", + "Type"=>"normal (base)", + "Location"=>"/home/vagrant/VirtualBox VMs/vagrant-sandbox_1579888721946_75923/disk-1.vdi", + "Disk Name"=>"disk-1", + "Storage format"=>"VDI", + "Capacity"=>"5120 MBytes", + "Encryption"=>"disabled"}] } let(:subject) { described_class } @@ -68,16 +96,129 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do context "#get_current_disk" do it "gets primary disk uuid if disk to configure is primary" do + primary_disk = subject.get_current_disk(machine, defined_disks.first, all_disks) + expect(primary_disk).to eq(all_disks.first) 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]) end it "returns nil if disk is not found" do + disk = subject.get_current_disk(machine, defined_disks[3], all_disks) + expect(disk).to be_nil end end context "#handle_configure_disk" do + describe "when creating a new disk" do + let(:all_disks) { [{"UUID"=>"12345", + "Parent UUID"=>"base", + "State"=>"created", + "Type"=>"normal (base)", + "Location"=>"/home/vagrant/VirtualBox VMs/vagrant-sandbox_1579888721946_75923/ubuntu-18.04-amd64-disk001.vmdk", + "Disk Name"=>"ubuntu-18.04-amd64-disk001", + "Storage format"=>"VMDK", + "Capacity"=>"65536 MBytes", + "Encryption"=>"disabled"}] } + + let(:disk_meta) { {uuid: "67890", name: "disk-0"} } + + it "creates a new disk if it doesn't yet exist" do + expect(subject).to receive(:create_disk).with(machine, defined_disks[1]) + .and_return(disk_meta) + + subject.handle_configure_disk(machine, defined_disks[1], all_disks) + end + end + + describe "when a disk needs to be resized" do + let(:all_disks) { [{"UUID"=>"12345", + "Parent UUID"=>"base", + "State"=>"created", + "Type"=>"normal (base)", + "Location"=>"/home/vagrant/VirtualBox VMs/vagrant-sandbox_1579888721946_75923/ubuntu-18.04-amd64-disk001.vmdk", + "Disk Name"=>"ubuntu-18.04-amd64-disk001", + "Storage format"=>"VMDK", + "Capacity"=>"65536 MBytes", + "Encryption"=>"disabled"}, + {"UUID"=>"67890", + "Parent UUID"=>"base", + "State"=>"created", + "Type"=>"normal (base)", + "Location"=>"/home/vagrant/VirtualBox VMs/vagrant-sandbox_1579888721946_75923/disk-0.vdi", + "Disk Name"=>"disk-0", + "Storage format"=>"VDI", + "Capacity"=>"10240 MBytes", + "Encryption"=>"disabled"}] } + + it "resizes a disk" do + expect(subject).to receive(:get_current_disk). + with(machine, defined_disks[1], all_disks).and_return(all_disks[1]) + + expect(subject).to receive(:compare_disk_size). + 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) + + subject.handle_configure_disk(machine, defined_disks[1], all_disks) + end + end + + describe "if no additional disk configuration is required" do + let(:all_disks) { [{"UUID"=>"12345", + "Parent UUID"=>"base", + "State"=>"created", + "Type"=>"normal (base)", + "Location"=>"/home/vagrant/VirtualBox VMs/vagrant-sandbox_1579888721946_75923/ubuntu-18.04-amd64-disk001.vmdk", + "Disk Name"=>"ubuntu-18.04-amd64-disk001", + "Storage format"=>"VMDK", + "Capacity"=>"65536 MBytes", + "Encryption"=>"disabled"}, + {"UUID"=>"67890", + "Parent UUID"=>"base", + "State"=>"created", + "Type"=>"normal (base)", + "Location"=>"/home/vagrant/VirtualBox VMs/vagrant-sandbox_1579888721946_75923/disk-0.vdi", + "Disk Name"=>"disk-0", + "Storage format"=>"VDI", + "Capacity"=>"10240 MBytes", + "Encryption"=>"disabled"}] } + + let(:disk_info) { {port: "1", device: "0"} } + + it "reattaches disk if vagrant defined disk exists but is not attached to guest" do + expect(subject).to receive(:get_current_disk). + with(machine, defined_disks[1], all_disks).and_return(all_disks[1]) + + expect(subject).to receive(:compare_disk_size). + with(machine, defined_disks[1], all_disks[1]).and_return(false) + + 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, + disk_info[:device], + all_disks[1]["Location"]) + + subject.handle_configure_disk(machine, defined_disks[1], all_disks) + end + + it "does nothing if all disks are properly configured" do + expect(subject).to receive(:get_current_disk). + with(machine, defined_disks[1], all_disks).and_return(all_disks[1]) + + expect(subject).to receive(:compare_disk_size). + with(machine, defined_disks[1], all_disks[1]).and_return(false) + + expect(driver).to receive(:get_port_and_device).with("67890"). + and_return(disk_info) + + subject.handle_configure_disk(machine, defined_disks[1], all_disks) + end + end end context "#compare_disk_size" do From 0ea26b9506b8db1556868c8fa2b15eac16deadb8 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 24 Jan 2020 10:44:28 -0800 Subject: [PATCH 096/127] Add test for comparing disk size --- .../providers/virtualbox/cap/configure_disks_test.rb | 6 ++++++ 1 file changed, 6 insertions(+) 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 3d28857c6..6c1cefdd2 100644 --- a/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb +++ b/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb @@ -222,10 +222,16 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do end context "#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) } + it "shows a warning if user attempts to shrink size" do + expect(machine.ui).to receive(:warn) + expect(subject.compare_disk_size(machine, disk_config_small, all_disks[1])).to be_falsey end it "returns true if requested size is bigger than current size" do + expect(subject.compare_disk_size(machine, disk_config_large, all_disks[1])).to be_truthy end end From b594715deddda3a543b253daa737ca3be1c2d7cf Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 24 Jan 2020 11:25:42 -0800 Subject: [PATCH 097/127] Add tests for resizing disks --- .../virtualbox/cap/configure_disks_test.rb | 68 +++++++++++++++++-- 1 file changed, 64 insertions(+), 4 deletions(-) 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 6c1cefdd2..7cd722b73 100644 --- a/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb +++ b/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb @@ -236,25 +236,85 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do end context "#create_disk" do + let(:disk_config) { double("disk", name: "disk-0", size: 1073741824.0, + primary: false, type: :disk, disk_ext: "vdi", + provider_config: nil) } + let(:vm_info) { {"CfgFile"=>"/home/vagrant/VirtualBox VMs/disks/"} } + let(:disk_file) { "/home/vagrant/VirtualBox VMs/disk-0.vdi" } + let(:disk_data) { "Medium created. UUID: 67890\n" } + + let(:port_and_device) { {port: "1", device: "0"} } + it "creates a disk and attaches it to a guest" do + expect(driver).to receive(:show_vm_info).and_return(vm_info) + + 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). + and_return(port_and_device) + + expect(driver).to receive(:attach_disk).with(port_and_device[:port], + port_and_device[:device], + disk_file) + + subject.create_disk(machine, disk_config) end end context "#get_next_port" do it "determines the next available port to use" do - end - - it "returns empty string if no usable port is available" do + dsk_info = subject.get_next_port(machine) + expect(dsk_info[:device]).to eq("0") + expect(dsk_info[:port]).to eq("2") end end context "#resize_disk" do describe "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) } + 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" } + it "converts the disk to vdi, resizes it, and converts back to vmdk" do + expect(driver).to receive(:get_port_and_device).with("12345"). + and_return(attach_info) + + expect(subject).to receive(:vmdk_to_vdi).with(driver, 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(:remove_disk).with(attach_info[:port], attach_info[:device]). + and_return(true) + expect(driver).to receive(:close_medium).with("12345") + + expect(subject).to receive(:vdi_to_vmdk).with(driver, vdi_disk_file). + 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) + expect(driver).to receive(:close_medium).with(vdi_disk_file).and_return(true) + + expect(driver).to receive(:list_hdds).and_return(all_disks) + + subject.resize_disk(machine, disk_config, all_disks[0]) end end - it "resizes the disk" do + describe "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]) + end end end From f59a5c2c709937e2573f940cc2bc347fb3780256 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 24 Jan 2020 13:58:47 -0800 Subject: [PATCH 098/127] Enable virtualbox and disk config in single flag --- plugins/kernel_v2/config/vm.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index 7abd9ee4b..b2fe0550d 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -433,7 +433,8 @@ module VagrantPlugins # Add provider config disk_config.add_provider_config(provider_options, &block) - if !Vagrant::Util::Experimental.feature_enabled?("disk_base_config") + if !Vagrant::Util::Experimental.feature_enabled?("disk_base_config") && + !Vagrant::Util::Experimental.feature_enabled?("virtualbox_disk_hdd") @logger.warn("Disk config defined, but experimental feature is not enabled. To use this feature, enable it with the experimental flag `disk_base_config`. Disk will not be added to internal config, and will be ignored.") return end From edd9ec89cb9c7b43cd5dcf07db63b97052cf90ca Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 24 Jan 2020 14:31:52 -0800 Subject: [PATCH 099/127] Begin to add docs for the virtualbox disk feature --- .../source/docs/disks/configuration.html.md | 25 +++++-- website/source/docs/disks/index.html.md | 3 +- website/source/docs/disks/usage.html.md | 74 +++++++++++++++++-- .../docs/disks/virtualbox/usage.html.md | 16 ++++ 4 files changed, 103 insertions(+), 15 deletions(-) diff --git a/website/source/docs/disks/configuration.html.md b/website/source/docs/disks/configuration.html.md index afbea13b3..015cd5950 100644 --- a/website/source/docs/disks/configuration.html.md +++ b/website/source/docs/disks/configuration.html.md @@ -21,8 +21,7 @@ description: |- Please note that `VAGRANT_EXPERIMENTAL` is an environment variable. For more information about this flag visit the [Experimental docs page](/docs/experimental/) - for more info. Without this flag enabled, triggers with the `:type` option - will be ignored. + for more info. Without this flag enabled, any disks defined will not be configured. Vagrant Disks has several options that allow users to define and attach disks to guests. @@ -43,7 +42,7 @@ the specific provider being used. - The provider name followed by a double underscore, and then the provider specific option for that disk + `{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. -* `type` (symbol) - The type of disk to manage. This option defaults to `:disk`. Please read the provider specific documentation for supported types. +* `size` (String) - The size of the disk to create. For example, `"10GB"`. **Note:** More specific examples of these can be found under the provider specific disk page. The `provider_config` option will depend on the provider you are using. Please read the provider specific documentation for disk management to learn about what options are available to use. @@ -72,8 +71,22 @@ If you are a vagrant plugin author who maintains a provider for Vagrant, this sh feature is more fully developed in Vagrant. -- Entry level builtin action `disk` and how to use it as a provider author -- `id` is unique to each disk config object +TODO: Write a bit here about what the internal disk config object looks like, and +how to use it. Add code links if possible to the virtualbox disk feature. + - `provider_config` and how to its structured and how to use/validate it -More information should be coming once the disk feature is more functional. +All providers must implement the capability `configure_disks`, and `cleanup_disks`. +These methods are responsible for the following: + +- `configure_disks` - Reads in a Vagrant config for defined disks from a Vagrantfile, +and creates and attaches the disks based on the given config +- `cleanup_disks` - Compares the current Vagrant config for defined disks and detaches +any disks that are no longer valid for a guest. + +These methods are called in the builtin Vagrant actions _Disk_ and _CleanupDisks_. +If the provider does not support these capabilities, they will be skipped over and no +disks will be configured. + +For a more detailed example of how to use this disk configuration with Vagrant, please +check out how it was implemented using the VirtualBox provider. diff --git a/website/source/docs/disks/index.html.md b/website/source/docs/disks/index.html.md index 6c4f6b3f5..ebc4c8faf 100644 --- a/website/source/docs/disks/index.html.md +++ b/website/source/docs/disks/index.html.md @@ -21,8 +21,7 @@ description: |- Please note that `VAGRANT_EXPERIMENTAL` is an environment variable. For more information about this flag visit the [Experimental docs page](/docs/experimental/) - for more info. Without this flag enabled, triggers with the `:type` option - will be ignored. + for more info. Without this flag enabled, any disks defined will not be configured. NOTE: Vagrant disks is currently a future feature for Vagrant that is not yet supported. Some documentation exists here for future reference, however the Disk feature is diff --git a/website/source/docs/disks/usage.html.md b/website/source/docs/disks/usage.html.md index a5e001cdb..123825d2c 100644 --- a/website/source/docs/disks/usage.html.md +++ b/website/source/docs/disks/usage.html.md @@ -21,14 +21,74 @@ description: |- Please note that `VAGRANT_EXPERIMENTAL` is an environment variable. For more information about this flag visit the [Experimental docs page](/docs/experimental/) - for more info. Without this flag enabled, triggers with the `:type` option - will be ignored. + for more info. Without this flag enabled, any disks defined will not be configured. -Below are some very simple examples of how to use Vagrant Disks. +Below are some very simple examples of how to use Vagrant Disks with the VirtualBox provider. -## Examples +## Basic disk examples -- Resizing a disk (primary) -- Attaching a new disk -- Using provider specific options +### Resizing your primary disk + +Sometimes, the primary disk for a guest is not large enough and you will need to +add more space. To resize a disk, you can simply add a config like this below +to expand the size of your guests drive: + +```ruby +config.vm.disk :disk, size: "100GB", primary: true +``` + +Note: the `primary: true` is what tells Vagrant to expand the guests main drive. +Without this option, Vagrant will instead attach a _new_ disk to the guest. + +For example, this Ubuntu guest will now come with 100GB of space, rather than the default: + +```ruby +Vagrant.configure("2") do |config| + config.vm.define "hashicorp" do |h| + h.vm.box = "hashicorp/bionic64" + h.vm.provider :virtualbox + + h.vm.disk :disk, size: "100GB", primary: true + end +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 + +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: + +```ruby +Vagrant.configure("2") do |config| + config.vm.define "hashicorp" do |h| + h.vm.box = "hashicorp/bionic64" + h.vm.provider :virtualbox + + h.vm.disk :disk, size: "10GB", name: "extra_storage" + end +end +``` + +Optionally, if you need to attach many disks, you can use Ruby to generate multiple +disks for Vagrant to create and attach to your guest: + +```ruby +Vagrant.configure("2") do |config| + config.vm.define "hashicorp" do |h| + h.vm.box = "hashicorp/bionic64" + h.vm.provider :virtualbox + + (0..3).each do |i| + h.vm.disk :disk, size: "5GB", name: "disk-#{i}" + end + end +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. diff --git a/website/source/docs/disks/virtualbox/usage.html.md b/website/source/docs/disks/virtualbox/usage.html.md index f984dc740..9c30df11b 100644 --- a/website/source/docs/disks/virtualbox/usage.html.md +++ b/website/source/docs/disks/virtualbox/usage.html.md @@ -8,3 +8,19 @@ description: |- --- # Usage + +
+ Warning! This feature is experimental and may break or + change in between releases. Use at your own risk. It currently is not officially + supported or functional. + + This feature currently reqiures the experimental flag to be used. To explicitly enable this feature, you can set the experimental flag to: + + ``` + VAGRANT_EXPERIMENTAL="virtualbox_disk_hdd" + ``` + + Please note that `VAGRANT_EXPERIMENTAL` is an environment variable. For more + information about this flag visit the [Experimental docs page](/docs/experimental/) + for more info. Without this flag enabled, any disks defined will not be configured. +
From 90461014dbff1f2bef6612643366936de252000a Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 24 Jan 2020 15:13:42 -0800 Subject: [PATCH 100/127] Website updates for disk management feature --- website/source/docs/disks/index.html.md | 25 ++++--------------- .../docs/disks/virtualbox/index.html.md | 23 +++++++++++++++++ 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/website/source/docs/disks/index.html.md b/website/source/docs/disks/index.html.md index ebc4c8faf..72616e695 100644 --- a/website/source/docs/disks/index.html.md +++ b/website/source/docs/disks/index.html.md @@ -8,26 +8,11 @@ description: |- # Vagrant Disks -
- Warning! This feature is experimental and may break or - change in between releases. Use at your own risk. It currently is not officially - supported or functional. - - This feature currently reqiures the experimental flag to be used. To explicitly enable this feature, you can set the experimental flag to: - - ``` - VAGRANT_EXPERIMENTAL="disk_base_config" - ``` - - Please note that `VAGRANT_EXPERIMENTAL` is an environment variable. For more - information about this flag visit the [Experimental docs page](/docs/experimental/) - for more info. Without this flag enabled, any disks defined will not be configured. - - NOTE: Vagrant disks is currently a future feature for Vagrant that is not yet supported. - Some documentation exists here for future reference, however the Disk feature is - not yet functional. Please be patient for us to develop this new feature, and stay - tuned for a future release of Vagrant with this new functionality! -
+Add a simple paragraph about the feature and what it can do here. For more information about what options are available for configuring disks, see the [configuration section](/docs/disks/configuration.html). + +## Supported Providers + +Currently, only VirtualBox is supported. Add link here diff --git a/website/source/docs/disks/virtualbox/index.html.md b/website/source/docs/disks/virtualbox/index.html.md index 23057f235..e3eb550c1 100644 --- a/website/source/docs/disks/virtualbox/index.html.md +++ b/website/source/docs/disks/virtualbox/index.html.md @@ -9,4 +9,27 @@ description: |- # VirtualBox +
+ Warning! This feature is experimental and may break or + change in between releases. Use at your own risk. It currently is not officially + supported or functional. + This feature currently reqiures the experimental flag to be used. To explicitly enable this feature, you can set the experimental flag to: + + ``` + VAGRANT_EXPERIMENTAL="virtualbox_disk_hdd" + ``` + + Please note that `VAGRANT_EXPERIMENTAL` is an environment variable. For more + information about this flag visit the [Experimental docs page](/docs/experimental/) + for more info. Without this flag enabled, any disks defined will not be configured. +
+ + +Vagrant using the `vboxmanage` command line interface to manage disks for a +VirtualBox guest. + +Because of how VirtualBox handles disk management, a Vagrant guest _must_ be powered +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. From 773a3aeb7e6b1fcee88147619bd29efb365f6fde Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 28 Jan 2020 09:12:35 -0800 Subject: [PATCH 101/127] Add optional hash arguments to driver methods --- plugins/providers/virtualbox/driver/version_5_0.rb | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/plugins/providers/virtualbox/driver/version_5_0.rb b/plugins/providers/virtualbox/driver/version_5_0.rb index ecb3de985..61a26fee7 100644 --- a/plugins/providers/virtualbox/driver/version_5_0.rb +++ b/plugins/providers/virtualbox/driver/version_5_0.rb @@ -26,7 +26,8 @@ module VagrantPlugins # @param [String] device - device on controller for disk # @param [String] file - disk file path # @param [String] type - type of disk to attach - def attach_disk(port, device, file, type="hdd") + # @param [Hash] opts - additional options + def attach_disk(port, device, file, type="hdd", **opts) # Maybe only support SATA Controller for `:disk`??? controller = "SATA Controller" @@ -62,7 +63,7 @@ module VagrantPlugins # @param [String] source # @param [String] destination # @param [String] disk_format - def clone_disk(source, destination, disk_format) + def clone_disk(source, destination, disk_format, **opts) execute("clonemedium", source, destination, '--format', disk_format) end @@ -80,6 +81,7 @@ module VagrantPlugins # Removes a disk from the given virtual machine # # @param [String] disk_uuid or file path + # @param [Hash] opts - additional options def close_medium(disk_uuid) execute("closemedium", disk_uuid, '--delete') end @@ -108,7 +110,8 @@ module VagrantPlugins # @param [String] disk_file # @param [Integer] disk_size - size in bytes (MUST BE DIVISIBLE BY 512 bytes) # @param [String] disk_format - format of disk, defaults to "VDI" - def create_disk(disk_file, disk_size, disk_format="VDI") + # @param [Hash] opts - additional options + def create_disk(disk_file, disk_size, disk_format="VDI", **opts) execute("createmedium", '--filename', disk_file, '--sizebyte', disk_size.to_i.to_s, '--format', disk_format) end @@ -226,6 +229,7 @@ module VagrantPlugins # @param [String] port - port on device to attach disk to # @param [String] device - device on controller for disk + # @param [Hash] opts - additional options def remove_disk(port, device) controller = "SATA Controller" execute('storageattach', @uuid, '--storagectl', controller, '--port', port.to_s, '--device', device.to_s, '--medium', "none") @@ -233,7 +237,8 @@ module VagrantPlugins # @param [String] disk_file # @param [Integer] disk_size in bytes - def resize_disk(disk_file, disk_size) + # @param [Hash] opts - additional options + def resize_disk(disk_file, disk_size, **opts) execute("modifymedium", disk_file, '--resizebyte', disk_size.to_i.to_s) end From a15016931a181b13ba385659e402f6ec915140c3 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 28 Jan 2020 11:04:21 -0800 Subject: [PATCH 102/127] Add more docs for disk feature --- .../source/docs/disks/configuration.html.md | 24 +++++-------------- website/source/docs/disks/index.html.md | 13 ++++++++-- website/source/docs/disks/usage.html.md | 5 +++- .../disks/virtualbox/common-issues.html.md | 15 +++++++++++- .../docs/disks/virtualbox/index.html.md | 4 ++++ .../docs/disks/virtualbox/usage.html.md | 8 +++++++ 6 files changed, 47 insertions(+), 22 deletions(-) diff --git a/website/source/docs/disks/configuration.html.md b/website/source/docs/disks/configuration.html.md index 015cd5950..aef69ed1b 100644 --- a/website/source/docs/disks/configuration.html.md +++ b/website/source/docs/disks/configuration.html.md @@ -8,22 +8,6 @@ description: |- # Configuration -
- Warning! This feature is experimental and may break or - change in between releases. Use at your own risk. It currently is not officially - supported or functional. - - This feature currently reqiures the experimental flag to be used. To explicitly enable this feature, you can set the experimental flag to: - - ``` - VAGRANT_EXPERIMENTAL="disk_base_config" - ``` - - Please note that `VAGRANT_EXPERIMENTAL` is an environment variable. For more - information about this flag visit the [Experimental docs page](/docs/experimental/) - for more info. Without this flag enabled, any disks defined will not be configured. -
- Vagrant Disks has several options that allow users to define and attach disks to guests. ## Disk Options @@ -31,10 +15,11 @@ 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. 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. +* `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 a 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. -* `provider_config` (hash) - Additional provider specific options for managing a given disk. +* `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. Generally, the disk option accepts two kinds of ways to define a provider config: @@ -54,6 +39,9 @@ The disk config currently accepts three kinds of disk types: * `dvd` (symbol) * `floppy` (symbol) +**NOTE:** These types depend on the provider used, and may not yet be functional. Please +refer to the provider specific implementation for more details for what is supported. + You can set a disk type with the first argument of a disk config in your Vagrantfile: ```ruby diff --git a/website/source/docs/disks/index.html.md b/website/source/docs/disks/index.html.md index 72616e695..2830aa8d9 100644 --- a/website/source/docs/disks/index.html.md +++ b/website/source/docs/disks/index.html.md @@ -8,11 +8,20 @@ description: |- # Vagrant Disks -Add a simple paragraph about the feature and what it can do here. +
+ Warning! This feature is experimental and may break or + change in between releases. Use at your own risk. It currently is not officially + supported or functional. +
+ +Vagrant Disks is a feature that allows users to define what mediums should be attached +to their guests, as well as allowing users to resize their primary disk. For examples +on how to achieve this, among other use cases, please refer to the [usage](/docs/disks/usage.html) +guide for more information! For more information about what options are available for configuring disks, see the [configuration section](/docs/disks/configuration.html). ## Supported Providers -Currently, only VirtualBox is supported. Add link here +Currently, only VirtualBox is supported. Please refer to the [VirtualBox documentation](/docs/disks/virtualbox/index.html) for more information on using disks with the VirtualBox provider! diff --git a/website/source/docs/disks/usage.html.md b/website/source/docs/disks/usage.html.md index 123825d2c..c978ffa1a 100644 --- a/website/source/docs/disks/usage.html.md +++ b/website/source/docs/disks/usage.html.md @@ -16,12 +16,15 @@ description: |- This feature currently reqiures the experimental flag to be used. To explicitly enable this feature, you can set the experimental flag to: ``` - VAGRANT_EXPERIMENTAL="disk_base_config" + VAGRANT_EXPERIMENTAL="virtualbox_disk_hdd" ``` Please note that `VAGRANT_EXPERIMENTAL` is an environment variable. For more information about this flag visit the [Experimental docs page](/docs/experimental/) for more info. Without this flag enabled, any disks defined will not be configured. + + Also note that the examples below use the VirtualBox provider, which is the current + supported providier for this feature. Below are some very simple examples of how to use Vagrant Disks with the VirtualBox provider. diff --git a/website/source/docs/disks/virtualbox/common-issues.html.md b/website/source/docs/disks/virtualbox/common-issues.html.md index 6331a1dde..09de55e16 100644 --- a/website/source/docs/disks/virtualbox/common-issues.html.md +++ b/website/source/docs/disks/virtualbox/common-issues.html.md @@ -12,9 +12,22 @@ description: |- This page lists some common issues people run into with Vagrant and VirtualBox as well as solutions for those issues. -## VirtualBox GUI +## Are my disks attached? A handy way to figure out what disks are attached (or not attached) to your guest is to open up the VirtualBox GUI and select the guest. When selecting a guest on the GUI, it should open more information about the guest, including storage information. Here 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. + +## Applying Changes to Guests + +Due to how VirtualBox works, you must reload your guest for any disk config changes +to be applied. So if you update your Vagrantfile to update or even remove disks, make +sure to `vagrant reload` your guests for these changes to be applied. diff --git a/website/source/docs/disks/virtualbox/index.html.md b/website/source/docs/disks/virtualbox/index.html.md index e3eb550c1..da1306061 100644 --- a/website/source/docs/disks/virtualbox/index.html.md +++ b/website/source/docs/disks/virtualbox/index.html.md @@ -33,3 +33,7 @@ Because of how VirtualBox handles disk management, a Vagrant guest _must_ be pow 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. diff --git a/website/source/docs/disks/virtualbox/usage.html.md b/website/source/docs/disks/virtualbox/usage.html.md index 9c30df11b..f1b9444e0 100644 --- a/website/source/docs/disks/virtualbox/usage.html.md +++ b/website/source/docs/disks/virtualbox/usage.html.md @@ -24,3 +24,11 @@ description: |- information about this flag visit the [Experimental docs page](/docs/experimental/) for more info. Without this flag enabled, any disks defined will not be configured. + +For examples of how to use the disk feature with VirtualBox, please refer to the +[general disk usage guide](/docs/disks/usage.html) for more examples. + +## provider_config options + +Currently, there are no additional options supported for the `provider_config` option. +This page will be updated with any valid options as they become supported. From fe6be8c886cf2d0731e57745a084b2c792935126 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 28 Jan 2020 11:19:09 -0800 Subject: [PATCH 103/127] Disk doc updates --- .../source/docs/disks/configuration.html.md | 35 ++++++++++++------- website/source/docs/disks/usage.html.md | 8 ++++- .../docs/disks/virtualbox/index.html.md | 9 +++-- 3 files changed, 36 insertions(+), 16 deletions(-) diff --git a/website/source/docs/disks/configuration.html.md b/website/source/docs/disks/configuration.html.md index aef69ed1b..3e195fd88 100644 --- a/website/source/docs/disks/configuration.html.md +++ b/website/source/docs/disks/configuration.html.md @@ -15,9 +15,13 @@ 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. 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 a 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. +* `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 a 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. * `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. @@ -29,7 +33,10 @@ the provider specific documentation to see any available provider_config options - 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"`. - **Note:** More specific examples of these can be found under the provider specific disk page. The `provider_config` option will depend on the provider you are using. Please read the provider specific documentation for disk management to learn about what options are available to use. + **Note:** More specific examples of these can be found under the provider + specific disk page. The `provider_config` option will depend on the provider + you are using. Please read the provider specific documentation for disk + management to learn about what options are available to use. ## Disk Types @@ -55,15 +62,10 @@ If you are a vagrant plugin author who maintains a provider for Vagrant, this sh
Warning! This guide is still being written as we develop this - new feature for Vagrant. Some points below are what we plan on covering once this - feature is more fully developed in Vagrant. + new feature for Vagrant. Is something missing, or could this be improved? Please + let us know on GitHub by opening an issue or open a pull request directly.
-TODO: Write a bit here about what the internal disk config object looks like, and -how to use it. Add code links if possible to the virtualbox disk feature. - -- `provider_config` and how to its structured and how to use/validate it - All providers must implement the capability `configure_disks`, and `cleanup_disks`. These methods are responsible for the following: @@ -74,7 +76,16 @@ any disks that are no longer valid for a guest. These methods are called in the builtin Vagrant actions _Disk_ and _CleanupDisks_. If the provider does not support these capabilities, they will be skipped over and no -disks will be configured. +disks will be configured. It is the providers job to implement these provider capabilities +and handle the methods required to support disk creation and deletion. Vagrant will +handle parsing and supplying the config object based on what has been defined inside +a users Vagrantfile. For a more detailed example of how to use this disk configuration with Vagrant, please check out how it was implemented using the VirtualBox provider. + +### The provider_config hash + +The disk config class supports an optional hash of options called `provider_config`. +This allows the user to define some additional options for a provider to use that +may be non-standard across different providers. diff --git a/website/source/docs/disks/usage.html.md b/website/source/docs/disks/usage.html.md index c978ffa1a..0cf074924 100644 --- a/website/source/docs/disks/usage.html.md +++ b/website/source/docs/disks/usage.html.md @@ -29,7 +29,7 @@ description: |- Below are some very simple examples of how to use Vagrant Disks with the VirtualBox provider. -## Basic disk examples +## Basic Examples ### Resizing your primary disk @@ -95,3 +95,9 @@ 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. + +### 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. diff --git a/website/source/docs/disks/virtualbox/index.html.md b/website/source/docs/disks/virtualbox/index.html.md index da1306061..826343541 100644 --- a/website/source/docs/disks/virtualbox/index.html.md +++ b/website/source/docs/disks/virtualbox/index.html.md @@ -25,9 +25,8 @@ description: |- for more info. Without this flag enabled, any disks defined will not be configured. - -Vagrant using the `vboxmanage` command line interface to manage disks for a -VirtualBox guest. +**Vagrant currently only supports VirtualBox version 5.x and newer for configuring and +attaching disks.** Because of how VirtualBox handles disk management, a Vagrant guest _must_ be powered off for any changes to be applied to a guest. If you make a configuration change @@ -37,3 +36,7 @@ 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. + +For more information on how to use VirtualBox to configure disks for a guest, refer +to the [general usage](/docs/disks/usage.html) and [configuration](/docs/disks/configuration.html) +guide for more information. From bb8f04f18a38f05323cdace3dfeddc2c43335b11 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 28 Jan 2020 13:04:52 -0800 Subject: [PATCH 104/127] Add docs note about disk_meta file --- .../source/docs/disks/configuration.html.md | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/website/source/docs/disks/configuration.html.md b/website/source/docs/disks/configuration.html.md index 3e195fd88..0eeb066e8 100644 --- a/website/source/docs/disks/configuration.html.md +++ b/website/source/docs/disks/configuration.html.md @@ -35,7 +35,7 @@ the provider specific documentation to see any available provider_config options **Note:** More specific examples of these can be found under the provider specific disk page. The `provider_config` option will depend on the provider - you are using. Please read the provider specific documentation for disk + you are using. Please read the provider specific documentation for disk management to learn about what options are available to use. ## Disk Types @@ -84,6 +84,26 @@ a users Vagrantfile. For a more detailed example of how to use this disk configuration with Vagrant, please check out how it was implemented using the VirtualBox provider. +### The disk_meta file + +Both builtin disk actions `configure_disks` and `cleanup_disks` expect to read and +write down a `disk_meta` file inside a machines data dir. This file is specifically +for keeping track of the _last configured state_ for disks in a given provider. +Generally, this file is used as a way for Vagrant to keep track of what disks +are being managed by Vagrant with the provider uses, so that it does not accidentally +delete or manage disks that were configured outside of Vagrants configuration. + +For the VirtualBox provider, Vagrant uses this file to see what disks were configured +on the _last run_ of Vagrant, and compares that to the current configured state for +the Vagrantfile on the _current run_ of Vagrant. It specifically stores each disks +UUID and disk name for use. If it notices a disk that is no longer in the +Vagrantfile, it can be assumed that the disk is no longer valid for that guest, +and cleans up the disk. + +This may not be required for your provider, however with the VirtualBox provider, Vagrant +needs a way to keep track of the defined disks managed by Vagrant and their disk UUIDs +that VirtualBox uses to keep track of these disks. + ### The provider_config hash The disk config class supports an optional hash of options called `provider_config`. From 0aa8a517c4c56e99d5aa6126e4d11d098c655560 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 28 Jan 2020 13:11:15 -0800 Subject: [PATCH 105/127] Add note about resizing VMDK disks --- .../docs/disks/virtualbox/common-issues.html.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/website/source/docs/disks/virtualbox/common-issues.html.md b/website/source/docs/disks/virtualbox/common-issues.html.md index 09de55e16..c6b5ba9c2 100644 --- a/website/source/docs/disks/virtualbox/common-issues.html.md +++ b/website/source/docs/disks/virtualbox/common-issues.html.md @@ -26,7 +26,16 @@ SATA Controllers can only support up to **30 disks** per guest. Therefore if you to define and attach more than 30, it will result in an error. This number _includes_ the primary disk for the guest. -## Applying Changes to Guests +## Resizing VMDK format disks + +VMDK disks cannot be resized in their current state, so Vagrant will automatically +convert these disks to VDI, resize the disk, and convert it back to its original format. +Many Vagrant boxes default to using the VMDK disk format, so resizing disks for +many users will require Vagrant to convert these disks. Generally, this will be transparent +to the user. However if Vagrant crashes or if a user interrupts Vagrant during the +cloning process, there is a chance that you might lose your data. + +## Applying Vagrant disk configuration changes to guests Due to how VirtualBox works, you must reload your guest for any disk config changes to be applied. So if you update your Vagrantfile to update or even remove disks, make From 4394962f56ff6347ec1846f05da6d9f5b3ad469c Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 28 Jan 2020 13:17:16 -0800 Subject: [PATCH 106/127] Include default disk_ext option in docs --- website/source/docs/disks/configuration.html.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/source/docs/disks/configuration.html.md b/website/source/docs/disks/configuration.html.md index 0eeb066e8..99a97316b 100644 --- a/website/source/docs/disks/configuration.html.md +++ b/website/source/docs/disks/configuration.html.md @@ -13,8 +13,8 @@ Vagrant Disks has several options that allow users to define and attach disks to ## Disk Options * `disk_ext` (string) - Optional argument that defines what kind of file -extension a disk should have. For a list of supported disk extensions, please check -the specific provider being used. +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 From b2a89f87116981870a865cb6e4faa8b4f6efa3a2 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 28 Jan 2020 13:19:22 -0800 Subject: [PATCH 107/127] Remove old comment on vbox driver disk method --- plugins/providers/virtualbox/driver/version_5_0.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/plugins/providers/virtualbox/driver/version_5_0.rb b/plugins/providers/virtualbox/driver/version_5_0.rb index 61a26fee7..a5f872ae2 100644 --- a/plugins/providers/virtualbox/driver/version_5_0.rb +++ b/plugins/providers/virtualbox/driver/version_5_0.rb @@ -104,11 +104,8 @@ module VagrantPlugins # Creates a disk. Default format is VDI unless overridden # - # Note: disk_size must be divisible by 512 bytes, otherwise operation will fail - # Source: https://www.virtualbox.org/ticket/5582 - # # @param [String] disk_file - # @param [Integer] disk_size - size in bytes (MUST BE DIVISIBLE BY 512 bytes) + # @param [Integer] disk_size - size in bytes # @param [String] disk_format - format of disk, defaults to "VDI" # @param [Hash] opts - additional options def create_disk(disk_file, disk_size, disk_format="VDI", **opts) From 567351a2af962c07a60cd8ea9e2b9b4804bd19fb Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 30 Jan 2020 15:31:49 -0800 Subject: [PATCH 108/127] Disk doc updates --- website/source/docs/disks/configuration.html.md | 2 +- website/source/docs/disks/index.html.md | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/website/source/docs/disks/configuration.html.md b/website/source/docs/disks/configuration.html.md index 99a97316b..fecef6b21 100644 --- a/website/source/docs/disks/configuration.html.md +++ b/website/source/docs/disks/configuration.html.md @@ -18,7 +18,7 @@ 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 a filename when creating the disk. +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. diff --git a/website/source/docs/disks/index.html.md b/website/source/docs/disks/index.html.md index 2830aa8d9..c9494819d 100644 --- a/website/source/docs/disks/index.html.md +++ b/website/source/docs/disks/index.html.md @@ -11,12 +11,14 @@ description: |-
Warning! This feature is experimental and may break or change in between releases. Use at your own risk. It currently is not officially - supported or functional. + supported or functional. Please refer to the providier specific disk documentation + for more information on how to use and enable this feature.
Vagrant Disks is a feature that allows users to define what mediums should be attached -to their guests, as well as allowing users to resize their primary disk. For examples -on how to achieve this, among other use cases, please refer to the [usage](/docs/disks/usage.html) +to their guests, as well as allowing users to resize their primary disk. + +For examples on how to achieve this, among other use cases, please refer to the [usage](/docs/disks/usage.html) guide for more information! For more information about what options are available for configuring disks, see the From 8af2e5c8be963c19fe129d89ae829a3bc1ab48d0 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 10 Feb 2020 13:01:41 -0800 Subject: [PATCH 109/127] Cast to float on conversation, and return helper size vars to integer --- lib/vagrant/util/numeric.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/vagrant/util/numeric.rb b/lib/vagrant/util/numeric.rb index 31cdd75d5..93346b81d 100644 --- a/lib/vagrant/util/numeric.rb +++ b/lib/vagrant/util/numeric.rb @@ -6,12 +6,12 @@ module Vagrant # Authors Note: This conversion has been borrowed from the ActiveSupport Numeric class # Conversion helper constants - KILOBYTE = 1024.0 - MEGABYTE = KILOBYTE * 1024.0 - GIGABYTE = MEGABYTE * 1024.0 - TERABYTE = GIGABYTE * 1024.0 - PETABYTE = TERABYTE * 1024.0 - EXABYTE = PETABYTE * 1024.0 + KILOBYTE = 1024 + MEGABYTE = KILOBYTE * 1024 + GIGABYTE = MEGABYTE * 1024 + TERABYTE = GIGABYTE * 1024 + PETABYTE = TERABYTE * 1024 + EXABYTE = PETABYTE * 1024 BYTES_CONVERSION_MAP = {KB: KILOBYTE, MB: MEGABYTE, GB: GIGABYTE, TB: TERABYTE, PB: PETABYTE, EB: EXABYTE} @@ -54,7 +54,7 @@ module Vagrant # @param [Integer] bytes # @return [Integer] megabytes - bytes representation in megabytes def bytes_to_megabytes(bytes) - (bytes / MEGABYTE).round(2) + (bytes / MEGABYTE.to_f).round(2) end # @private From b11aa532946abe4616ae60bc71d9e4ec126d861a Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 10 Feb 2020 13:03:06 -0800 Subject: [PATCH 110/127] Remove unused experimental flag for disk enablement --- plugins/kernel_v2/config/vm.rb | 5 ++--- test/unit/plugins/kernel_v2/config/vm_test.rb | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index b2fe0550d..a2b9f35f8 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -433,9 +433,8 @@ module VagrantPlugins # Add provider config disk_config.add_provider_config(provider_options, &block) - if !Vagrant::Util::Experimental.feature_enabled?("disk_base_config") && - !Vagrant::Util::Experimental.feature_enabled?("virtualbox_disk_hdd") - @logger.warn("Disk config defined, but experimental feature is not enabled. To use this feature, enable it with the experimental flag `disk_base_config`. Disk will not be added to internal config, and will be ignored.") + if !Vagrant::Util::Experimental.feature_enabled?("virtualbox_disk_hdd") + @logger.warn("Disk config defined, but experimental feature is not enabled. To use this feature, enable it with the experimental flag `virtualbox_disk_hdd`. Disk will not be added to internal config, and will be ignored.") return end diff --git a/test/unit/plugins/kernel_v2/config/vm_test.rb b/test/unit/plugins/kernel_v2/config/vm_test.rb index e86db848f..58b058295 100644 --- a/test/unit/plugins/kernel_v2/config/vm_test.rb +++ b/test/unit/plugins/kernel_v2/config/vm_test.rb @@ -552,7 +552,7 @@ describe VagrantPlugins::Kernel_V2::VMConfig do describe "#disk" do before(:each) do allow(Vagrant::Util::Experimental).to receive(:feature_enabled?). - with("disk_base_config").and_return("true") + with("virtualbox_disk_hdd").and_return("true") end it "stores the disks" do From 00d7ecb57fca9d071f1e741dc17a84d86072f4b0 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 10 Feb 2020 13:04:47 -0800 Subject: [PATCH 111/127] Remove explicit return for disk configure methods --- plugins/providers/virtualbox/cap/configure_disks.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 913b2270a..fba23b02c 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -43,7 +43,7 @@ module VagrantPlugins end end - return configured_disks + configured_disks end protected @@ -66,7 +66,7 @@ module VagrantPlugins current_disk = all_disks.select { |d| d["Disk Name"] == disk.name}.first end - return current_disk + current_disk end # Handles all disk configs of type `:disk` @@ -104,7 +104,7 @@ module VagrantPlugins disk_metadata = {uuid: current_disk["UUID"], name: disk.name} end - return disk_metadata + disk_metadata end # Check to see if current disk is configured based on defined_disks @@ -222,7 +222,8 @@ module VagrantPlugins end disk_metadata = {uuid: defined_disk["UUID"], name: disk_config.name} - return disk_metadata + + disk_metadata end # TODO: Maybe these should be virtualbox driver methods? From 5bc0b28116cf6b3626f8aeff7802293f811a2edd Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 10 Feb 2020 13:21:27 -0800 Subject: [PATCH 112/127] Update how to check for empty string --- plugins/providers/virtualbox/cap/configure_disks.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index fba23b02c..d790d2a18 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -176,7 +176,7 @@ module VagrantPlugins next_available_port = ((0..(MAX_DISK_NUMBER-1)).to_a - used_ports).first dsk_info[:port] = next_available_port.to_s - if dsk_info[:port] == "" + if dsk_info[:port].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.") end From 4866709c6747cd3839e012dd9c9c513ac6cec2a5 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 10 Feb 2020 15:01:35 -0800 Subject: [PATCH 113/127] Move disk cloning to virtualbox driver 5 --- .../virtualbox/cap/configure_disks.rb | 32 ++----------------- plugins/providers/virtualbox/driver/meta.rb | 4 ++- .../virtualbox/driver/version_5_0.rb | 24 ++++++++++++++ 3 files changed, 29 insertions(+), 31 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index d790d2a18..8e9d65345 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -193,7 +193,7 @@ module VagrantPlugins disk_info = machine.provider.driver.get_port_and_device(defined_disk["UUID"]) # clone disk to vdi formatted disk - vdi_disk_file = vmdk_to_vdi(machine.provider.driver, defined_disk["Location"]) + vdi_disk_file = machine.provider.driver.vmdk_to_vdi(defined_disk["Location"]) # resize vdi machine.provider.driver.resize_disk(vdi_disk_file, disk_config.size.to_i) @@ -202,7 +202,7 @@ module VagrantPlugins machine.provider.driver.close_medium(defined_disk["UUID"]) # clone back to original vmdk format and attach resized disk - vmdk_disk_file = vdi_to_vmdk(machine.provider.driver, vdi_disk_file) + 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") # close cloned volume format @@ -225,34 +225,6 @@ module VagrantPlugins disk_metadata end - - # TODO: Maybe these should be virtualbox driver methods? - - # @param [VagrantPlugins::VirtualboxProvider::Driver] driver - # @param [String] defined_disk_path - # @return [String] destination - The cloned disk - def self.vmdk_to_vdi(driver, defined_disk_path) - LOGGER.warn("Converting disk '#{defined_disk_path}' from 'vmdk' to 'vdi' format") - source = defined_disk_path - destination = File.join(File.dirname(source), File.basename(source, ".*")) + ".vdi" - - driver.clone_disk(source, destination, 'VDI') - - destination - end - - # @param [VagrantPlugins::VirtualboxProvider::Driver] driver - # @param [String] defined_disk_path - # @return [String] destination - The cloned disk - def self.vdi_to_vmdk(driver, defined_disk_path) - LOGGER.warn("Converting disk '#{defined_disk_path}' from 'vdi' to 'vmdk' format") - source = defined_disk_path - destination = File.join(File.dirname(source), File.basename(source, ".*")) + ".vmdk" - - driver.clone_disk(source, destination, 'VMDK') - - destination - end end end end diff --git a/plugins/providers/virtualbox/driver/meta.rb b/plugins/providers/virtualbox/driver/meta.rb index 470149515..7175112ba 100644 --- a/plugins/providers/virtualbox/driver/meta.rb +++ b/plugins/providers/virtualbox/driver/meta.rb @@ -147,9 +147,11 @@ module VagrantPlugins :ssh_port, :start, :suspend, + :vdi_to_vmdk, :verify!, :verify_image, - :vm_exists? + :vm_exists?, + :vmdk_to_vdi protected diff --git a/plugins/providers/virtualbox/driver/version_5_0.rb b/plugins/providers/virtualbox/driver/version_5_0.rb index a5f872ae2..7220bb5d9 100644 --- a/plugins/providers/virtualbox/driver/version_5_0.rb +++ b/plugins/providers/virtualbox/driver/version_5_0.rb @@ -894,6 +894,30 @@ module VagrantPlugins return true end + # @param [VagrantPlugins::VirtualboxProvider::Driver] driver + # @param [String] defined_disk_path + # @return [String] destination - The cloned disk + def vmdk_to_vdi(defined_disk_path) + source = defined_disk_path + destination = File.join(File.dirname(source), File.basename(source, ".*")) + ".vdi" + + clone_disk(source, destination, 'VDI') + + destination + end + + # @param [VagrantPlugins::VirtualboxProvider::Driver] driver + # @param [String] defined_disk_path + # @return [String] destination - The cloned disk + def vdi_to_vmdk(defined_disk_path) + source = defined_disk_path + destination = File.join(File.dirname(source), File.basename(source, ".*")) + ".vmdk" + + clone_disk(source, destination, 'VMDK') + + destination + end + protected def valid_ip_address?(ip) From 3662344d2e5e27542fbce2383b05c88f3648d937 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 10 Feb 2020 15:31:10 -0800 Subject: [PATCH 114/127] Update rspec tests for driver methods --- .../plugins/providers/virtualbox/cap/configure_disks_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 7cd722b73..943d446ef 100644 --- a/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb +++ b/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb @@ -283,7 +283,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do expect(driver).to receive(:get_port_and_device).with("12345"). and_return(attach_info) - expect(subject).to receive(:vmdk_to_vdi).with(driver, all_disks[0]["Location"]). + 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). @@ -293,7 +293,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do and_return(true) expect(driver).to receive(:close_medium).with("12345") - expect(subject).to receive(:vdi_to_vmdk).with(driver, vdi_disk_file). + expect(driver).to receive(:vdi_to_vmdk).with(vdi_disk_file). and_return(vmdk_disk_file) expect(driver).to receive(:attach_disk). From 6fcc5fa31a526cf77aad8ecb540bb7cfa8961122 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 11 Feb 2020 13:06:57 -0800 Subject: [PATCH 115/127] Raise error if guest has no available ports for attaching disks --- plugins/providers/virtualbox/cap/configure_disks.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 8e9d65345..41604dc05 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -179,6 +179,7 @@ module VagrantPlugins if dsk_info[:port].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 end dsk_info From 1826d210a061b06fafeeebbb02317779c3fc86c6 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 11 Feb 2020 16:00:54 -0800 Subject: [PATCH 116/127] Recover from cloning and resizin disk failures This commit adds some recovery if Vagrant fails to reattach or clone a vmdk disk that needs to be resized. It first moves the original disk (after cloning) to a backup file. If something fails, it will reattach the original disk to the guest and continue to raise the exception. --- .../virtualbox/cap/configure_disks.rb | 55 ++++++++++++++----- .../virtualbox/cap/configure_disks_test.rb | 31 +++++++++++ 2 files changed, 73 insertions(+), 13 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 41604dc05..092645006 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -1,4 +1,5 @@ require "log4r" +require "fileutils" require "vagrant/util/numeric" require "vagrant/util/experimental" @@ -190,34 +191,62 @@ module VagrantPlugins 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 # clone disk to vdi formatted disk vdi_disk_file = machine.provider.driver.vmdk_to_vdi(defined_disk["Location"]) # resize vdi machine.provider.driver.resize_disk(vdi_disk_file, disk_config.size.to_i) - # remove and close original volume - machine.provider.driver.remove_disk(disk_info[:port], disk_info[:device]) - machine.provider.driver.close_medium(defined_disk["UUID"]) + begin + # Danger Zone - # 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") + # remove and close original volume + machine.provider.driver.remove_disk(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"], "#{defined_disk["Location"]}.backup") - # close cloned volume format + # we have to close here, otherwise we can't re-clone after + # resizing the vdi disk + machine.provider.driver.close_medium(defined_disk["UUID"]) + + # 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") + rescue Exception => e + LOGGER.warn("Vagrant encountered an error while trying to resize a disk. Vagrant will now attempt to reattach and preserve the original disk...") + # TODO: Actually write some good recovery steps for this scenario + machine.ui.error("Vagrant has encountered an exception while trying to resize a disk. It will now attempt to reattach the original disk, as to prevent any data loss.") + machine.ui.error("The original disk is located at : #{original_disk["Location"]}") + # move backup to original name + FileUtils.mv("#{original_disk["Location"]}.backup", + original_disk["Location"], force: true) + # Attach disk + machine.provider.driver. + attach_disk(disk_info[:port], disk_info[:device], original_disk["Location"], "hdd") + + # Remove cloned disk if still hanging around + if vdi_disk_file + machine.provider.driver.close_medium(vdi_disk_file) + end + + raise e + ensure + # Remove backup disk file if all goes well + FileUtils.remove("#{original_disk["Location"]}.backup", force: true) + end + + # Remove cloned resized volume format machine.provider.driver.close_medium(vdi_disk_file) # Get new updated disk UUID for vagrant disk_meta file new_disk_info = machine.provider.driver.list_hdds.select { |h| h["Location"] == defined_disk["Location"] }.first defined_disk = new_disk_info - - # TODO: If any of the above steps fail, display a useful error message - # telling the user how to recover - # - # Vagrant could also have a "rescue" here where in the case of failure, it simply - # reattaches the original disk else machine.provider.driver.resize_disk(defined_disk["Location"], disk_config.size.to_i) 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 943d446ef..b9c0d74ba 100644 --- a/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb +++ b/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb @@ -280,6 +280,37 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do let(:vmdk_disk_file) { "/home/vagrant/VirtualBox VMs/ubuntu-18.04-amd64-disk001.vmdk" } it "converts the disk to vdi, resizes it, and converts back to vmdk" do + expect(FileUtils).to receive(:mv).and_return(true) + + expect(driver).to receive(:get_port_and_device).with("12345"). + and_return(attach_info) + + 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(:remove_disk).with(attach_info[:port], attach_info[:device]). + and_return(true) + expect(driver).to receive(:close_medium).with("12345") + + expect(driver).to receive(:vdi_to_vmdk).with(vdi_disk_file). + 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) + expect(driver).to receive(:close_medium).with(vdi_disk_file).and_return(true) + + expect(driver).to receive(:list_hdds).and_return(all_disks) + + subject.resize_disk(machine, disk_config, all_disks[0]) + end + + it "reattaches original disk if something goes wrong" do + # fix this to properly "mv" file + expect(FileUtils).to receive(:mv).and_return(true) + expect(driver).to receive(:get_port_and_device).with("12345"). and_return(attach_info) From efde9595c4cb0569cc7f36a8d6d21139c2b826c6 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 12 Feb 2020 09:32:38 -0800 Subject: [PATCH 117/127] Add error case handling for resizing vmdk disks --- .../virtualbox/cap/configure_disks_test.rb | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) 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 b9c0d74ba..8743ff180 100644 --- a/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb +++ b/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb @@ -37,7 +37,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do "Parent UUID"=>"base", "State"=>"created", "Type"=>"normal (base)", - "Location"=>"/home/vagrant/VirtualBox VMs/vagrant-sandbox_1579888721946_75923/ubuntu-18.04-amd64-disk001.vmdk", + "Location"=>"/home/vagrant/VirtualBox VMs/ubuntu-18.04-amd64-disk001.vmdk", "Disk Name"=>"ubuntu-18.04-amd64-disk001", "Storage format"=>"VMDK", "Capacity"=>"65536 MBytes", @@ -46,7 +46,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do "Parent UUID"=>"base", "State"=>"created", "Type"=>"normal (base)", - "Location"=>"/home/vagrant/VirtualBox VMs/vagrant-sandbox_1579888721946_75923/disk-0.vdi", + "Location"=>"/home/vagrant/VirtualBox VMs/disk-0.vdi", "Disk Name"=>"disk-0", "Storage format"=>"VDI", "Capacity"=>"10240 MBytes", @@ -55,7 +55,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do "Parent UUID"=>"base", "State"=>"created", "Type"=>"normal (base)", - "Location"=>"/home/vagrant/VirtualBox VMs/vagrant-sandbox_1579888721946_75923/disk-1.vdi", + "Location"=>"/home/vagrant/VirtualBox VMs/disk-1.vdi", "Disk Name"=>"disk-1", "Storage format"=>"VDI", "Capacity"=>"5120 MBytes", @@ -117,7 +117,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do "Parent UUID"=>"base", "State"=>"created", "Type"=>"normal (base)", - "Location"=>"/home/vagrant/VirtualBox VMs/vagrant-sandbox_1579888721946_75923/ubuntu-18.04-amd64-disk001.vmdk", + "Location"=>"/home/vagrant/VirtualBox VMs/ubuntu-18.04-amd64-disk001.vmdk", "Disk Name"=>"ubuntu-18.04-amd64-disk001", "Storage format"=>"VMDK", "Capacity"=>"65536 MBytes", @@ -138,7 +138,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do "Parent UUID"=>"base", "State"=>"created", "Type"=>"normal (base)", - "Location"=>"/home/vagrant/VirtualBox VMs/vagrant-sandbox_1579888721946_75923/ubuntu-18.04-amd64-disk001.vmdk", + "Location"=>"/home/vagrant/VirtualBox VMs/ubuntu-18.04-amd64-disk001.vmdk", "Disk Name"=>"ubuntu-18.04-amd64-disk001", "Storage format"=>"VMDK", "Capacity"=>"65536 MBytes", @@ -147,7 +147,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do "Parent UUID"=>"base", "State"=>"created", "Type"=>"normal (base)", - "Location"=>"/home/vagrant/VirtualBox VMs/vagrant-sandbox_1579888721946_75923/disk-0.vdi", + "Location"=>"/home/vagrant/VirtualBox VMs/disk-0.vdi", "Disk Name"=>"disk-0", "Storage format"=>"VDI", "Capacity"=>"10240 MBytes", @@ -172,7 +172,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do "Parent UUID"=>"base", "State"=>"created", "Type"=>"normal (base)", - "Location"=>"/home/vagrant/VirtualBox VMs/vagrant-sandbox_1579888721946_75923/ubuntu-18.04-amd64-disk001.vmdk", + "Location"=>"/home/vagrant/VirtualBox VMs/ubuntu-18.04-amd64-disk001.vmdk", "Disk Name"=>"ubuntu-18.04-amd64-disk001", "Storage format"=>"VMDK", "Capacity"=>"65536 MBytes", @@ -181,7 +181,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do "Parent UUID"=>"base", "State"=>"created", "Type"=>"normal (base)", - "Location"=>"/home/vagrant/VirtualBox VMs/vagrant-sandbox_1579888721946_75923/disk-0.vdi", + "Location"=>"/home/vagrant/VirtualBox VMs/disk-0.vdi", "Disk Name"=>"disk-0", "Storage format"=>"VDI", "Capacity"=>"10240 MBytes", @@ -280,7 +280,8 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do let(:vmdk_disk_file) { "/home/vagrant/VirtualBox VMs/ubuntu-18.04-amd64-disk001.vmdk" } it "converts the disk to vdi, resizes it, and converts back to vmdk" do - expect(FileUtils).to receive(:mv).and_return(true) + expect(FileUtils).to receive(:mv).with(vmdk_disk_file, "#{vmdk_disk_file}.backup"). + and_return(true) expect(driver).to receive(:get_port_and_device).with("12345"). and_return(attach_info) @@ -304,12 +305,15 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do expect(driver).to receive(:list_hdds).and_return(all_disks) + expect(FileUtils).to receive(:remove).with("#{vmdk_disk_file}.backup", force: true). + and_return(true) + subject.resize_disk(machine, disk_config, all_disks[0]) end it "reattaches original disk if something goes wrong" do - # fix this to properly "mv" file - expect(FileUtils).to receive(:mv).and_return(true) + expect(FileUtils).to receive(:mv).with(vmdk_disk_file, "#{vmdk_disk_file}.backup"). + and_return(true) expect(driver).to receive(:get_port_and_device).with("12345"). and_return(attach_info) @@ -324,16 +328,16 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do and_return(true) expect(driver).to receive(:close_medium).with("12345") - expect(driver).to receive(:vdi_to_vmdk).with(vdi_disk_file). - and_return(vmdk_disk_file) + allow(driver).to receive(:vdi_to_vmdk).and_raise(Exception) + + expect(FileUtils).to receive(:mv).with("#{vmdk_disk_file}.backup", vmdk_disk_file, force: true). + and_return(true) 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(driver).to receive(:list_hdds).and_return(all_disks) - - subject.resize_disk(machine, disk_config, all_disks[0]) + expect{subject.resize_disk(machine, disk_config, all_disks[0])}.to raise_error(Exception) end end From d5e12dab170e3a454ff34713aa5a137eeb2f8580 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 12 Feb 2020 09:56:41 -0800 Subject: [PATCH 118/127] Store backup disk location as local var --- plugins/providers/virtualbox/cap/configure_disks.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 092645006..56a089040 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -196,6 +196,7 @@ module VagrantPlugins 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" # clone disk to vdi formatted disk vdi_disk_file = machine.provider.driver.vmdk_to_vdi(defined_disk["Location"]) @@ -209,7 +210,7 @@ module VagrantPlugins machine.provider.driver.remove_disk(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"], "#{defined_disk["Location"]}.backup") + FileUtils.mv(defined_disk["Location"], backup_disk_location) # we have to close here, otherwise we can't re-clone after # resizing the vdi disk @@ -224,8 +225,7 @@ module VagrantPlugins machine.ui.error("Vagrant has encountered an exception while trying to resize a disk. It will now attempt to reattach the original disk, as to prevent any data loss.") machine.ui.error("The original disk is located at : #{original_disk["Location"]}") # move backup to original name - FileUtils.mv("#{original_disk["Location"]}.backup", - original_disk["Location"], force: true) + 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") @@ -238,7 +238,7 @@ module VagrantPlugins raise e ensure # Remove backup disk file if all goes well - FileUtils.remove("#{original_disk["Location"]}.backup", force: true) + FileUtils.remove(backup_disk_location, force: true) end # Remove cloned resized volume format From 2174f4c2199830ca696589769b7eb8b6664bb4f9 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 12 Feb 2020 10:24:38 -0800 Subject: [PATCH 119/127] Write up recovery message for when vagrant fails to resize vmdk disks --- plugins/providers/virtualbox/cap/configure_disks.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 56a089040..f84a8b3d3 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -221,9 +221,9 @@ module VagrantPlugins machine.provider.driver.attach_disk(disk_info[:port], disk_info[:device], vmdk_disk_file, "hdd") rescue Exception => e LOGGER.warn("Vagrant encountered an error while trying to resize a disk. Vagrant will now attempt to reattach and preserve the original disk...") - # TODO: Actually write some good recovery steps for this scenario machine.ui.error("Vagrant has encountered an exception while trying to resize a disk. It will now attempt to reattach the original disk, as to prevent any data loss.") - machine.ui.error("The original disk is located at : #{original_disk["Location"]}") + machine.ui.error("The original disk is located at: #{original_disk["Location"]}") + machine.ui.error("If Vagrant fails to reattach the original disk, it is recommended that you open the VirtualBox GUI and navigate to the current guests settings for '#{machine.name}' and look at the 'storage' section. Here is where you can reattach a missing disk if Vagrant fails to do so...") # move backup to original name FileUtils.mv(backup_disk_location, original_disk["Location"], force: true) # Attach disk @@ -235,6 +235,7 @@ module VagrantPlugins machine.provider.driver.close_medium(vdi_disk_file) end + machine.ui.warn("Disk has been reattached. Vagrant will now continue on an raise the exception receieved") raise e ensure # Remove backup disk file if all goes well From c5f45344871d07867be4af6f924a9e6d45aef4dc Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 12 Feb 2020 13:21:09 -0800 Subject: [PATCH 120/127] Add virtualbox capability for validating disk extension types --- plugins/kernel_v2/config/disk.rb | 16 ++++++----- .../virtualbox/cap/validate_disk_ext.rb | 27 +++++++++++++++++++ plugins/providers/virtualbox/plugin.rb | 10 +++++++ 3 files changed, 47 insertions(+), 6 deletions(-) create mode 100644 plugins/providers/virtualbox/cap/validate_disk_ext.rb diff --git a/plugins/kernel_v2/config/disk.rb b/plugins/kernel_v2/config/disk.rb index e443f5f8a..90ca1c781 100644 --- a/plugins/kernel_v2/config/disk.rb +++ b/plugins/kernel_v2/config/disk.rb @@ -11,7 +11,6 @@ module VagrantPlugins #------------------------------------------------------------------- DEFAULT_DISK_TYPES = [:disk, :dvd, :floppy].freeze - DEFAULT_DISK_EXT = ["vdi", "vmdk", "vhd", "vhdx"].freeze # Note: This value is for internal use only # @@ -136,13 +135,18 @@ module VagrantPlugins types: DEFAULT_DISK_TYPES.join(', ')) end - if !DEFAULT_DISK_EXT.include?(@disk_ext) - errors << I18n.t("vagrant.config.disk.invalid_ext", ext: @disk_ext, - name: @name, exts: DEFAULT_DISK_EXT.join(', ') ) - end - if @disk_ext @disk_ext = @disk_ext.downcase + + if machine.provider.capability?(:validate_disk_ext) + if !machine.provider.capability(:validate_disk_ext, @disk_ext) + errors << I18n.t("vagrant.config.disk.invalid_ext", ext: @disk_ext, + name: @name, + exts: machine.provider.capability(:get_default_disk_ext).join(', ')) + end + else + @logger.warn("No provider capability defined to validate 'disk_ext' type") + end end if @size && !@size.is_a?(Integer) diff --git a/plugins/providers/virtualbox/cap/validate_disk_ext.rb b/plugins/providers/virtualbox/cap/validate_disk_ext.rb new file mode 100644 index 000000000..3715a91d0 --- /dev/null +++ b/plugins/providers/virtualbox/cap/validate_disk_ext.rb @@ -0,0 +1,27 @@ +require "log4r" + +module VagrantPlugins + module ProviderVirtualBox + module Cap + module ValidateDiskExt + LOGGER = Log4r::Logger.new("vagrant::plugins::virtualbox::validate_disk_ext") + + # The default set of disk formats that VirtualBox supports + DEFAULT_DISK_EXT = ["vdi", "vmdk", "vhd"].freeze + + # @param [Vagrant::Machine] machine + # @param [String] disk_ext + # @return [Bool] + def self.validate_disk_ext(machine, disk_ext) + DEFAULT_DISK_EXT.include?(disk_ext) + end + + # @param [Vagrant::Machine] machine + # @return [Array] + def self.get_default_disk_ext(machine) + DEFAULT_DISK_EXT + end + end + end + end +end diff --git a/plugins/providers/virtualbox/plugin.rb b/plugins/providers/virtualbox/plugin.rb index e5b3e03a5..9cb46b54e 100644 --- a/plugins/providers/virtualbox/plugin.rb +++ b/plugins/providers/virtualbox/plugin.rb @@ -49,6 +49,16 @@ module VagrantPlugins Cap::CleanupDisks end + provider_capability(:virtualbox, :validate_disk_ext) do + require_relative "cap/validate_disk_ext" + Cap::ValidateDiskExt + end + + provider_capability(:virtualbox, :get_default_disk_ext) do + require_relative "cap/validate_disk_ext" + Cap::ValidateDiskExt + end + provider_capability(:virtualbox, :snapshot_list) do require_relative "cap" Cap From c179b2fb229cde0f2f3b8a212a37fd64306a24d5 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 12 Feb 2020 13:51:59 -0800 Subject: [PATCH 121/127] Include updated unit tests for disk ext update --- test/unit/plugins/kernel_v2/config/disk_test.rb | 6 +++++- test/unit/plugins/kernel_v2/config/vm_test.rb | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/test/unit/plugins/kernel_v2/config/disk_test.rb b/test/unit/plugins/kernel_v2/config/disk_test.rb index 88640d274..6b6879f37 100644 --- a/test/unit/plugins/kernel_v2/config/disk_test.rb +++ b/test/unit/plugins/kernel_v2/config/disk_test.rb @@ -9,7 +9,9 @@ describe VagrantPlugins::Kernel_V2::VagrantConfigDisk do subject { described_class.new(type) } - let(:machine) { double("machine") } + let(:provider) { double("provider") } + let(:machine) { double("machine", provider: provider) } + def assert_invalid errors = subject.validate(machine) @@ -30,6 +32,8 @@ describe VagrantPlugins::Kernel_V2::VagrantConfigDisk do 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) end describe "with defaults" do diff --git a/test/unit/plugins/kernel_v2/config/vm_test.rb b/test/unit/plugins/kernel_v2/config/vm_test.rb index 58b058295..8d0896771 100644 --- a/test/unit/plugins/kernel_v2/config/vm_test.rb +++ b/test/unit/plugins/kernel_v2/config/vm_test.rb @@ -7,7 +7,8 @@ describe VagrantPlugins::Kernel_V2::VMConfig do subject { described_class.new } - let(:machine) { double("machine") } + let(:provider) { double("provider") } + let(:machine) { double("machine", provider: provider) } def assert_invalid errors = subject.validate(machine) @@ -37,6 +38,9 @@ describe VagrantPlugins::Kernel_V2::VMConfig do allow(machine).to receive(:provider_config).and_return(nil) allow(machine).to receive(:provider_options).and_return({}) + 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) + subject.box = "foo" end From dafb60ad4fbfbfc7d1e7c07626b57ee67756d2ee Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 12 Feb 2020 15:38:34 -0800 Subject: [PATCH 122/127] Update experimental feature flag for disk --- plugins/kernel_v2/config/vm.rb | 4 ++-- plugins/providers/virtualbox/cap/cleanup_disks.rb | 2 +- plugins/providers/virtualbox/cap/configure_disks.rb | 2 +- test/unit/plugins/kernel_v2/config/vm_test.rb | 2 +- website/source/docs/disks/usage.html.md | 2 +- website/source/docs/disks/virtualbox/index.html.md | 2 +- website/source/docs/disks/virtualbox/usage.html.md | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index a2b9f35f8..744649a12 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -433,8 +433,8 @@ module VagrantPlugins # Add provider config disk_config.add_provider_config(provider_options, &block) - if !Vagrant::Util::Experimental.feature_enabled?("virtualbox_disk_hdd") - @logger.warn("Disk config defined, but experimental feature is not enabled. To use this feature, enable it with the experimental flag `virtualbox_disk_hdd`. Disk will not be added to internal config, and will be ignored.") + if !Vagrant::Util::Experimental.feature_enabled?("disks") + @logger.warn("Disk config defined, but experimental feature is not enabled. To use this feature, enable it with the experimental flag `disks`. Disk will not be added to internal config, and will be ignored.") return end diff --git a/plugins/providers/virtualbox/cap/cleanup_disks.rb b/plugins/providers/virtualbox/cap/cleanup_disks.rb index 1416f042b..d9e11f7ff 100644 --- a/plugins/providers/virtualbox/cap/cleanup_disks.rb +++ b/plugins/providers/virtualbox/cap/cleanup_disks.rb @@ -13,7 +13,7 @@ module VagrantPlugins def self.cleanup_disks(machine, defined_disks, disk_meta_file) return if disk_meta_file.values.flatten.empty? - return if !Vagrant::Util::Experimental.feature_enabled?("virtualbox_disk_hdd") + return if !Vagrant::Util::Experimental.feature_enabled?("disks") handle_cleanup_disk(machine, defined_disks, disk_meta_file["disk"]) # TODO: Floppy and DVD disks diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index f84a8b3d3..491b20691 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -18,7 +18,7 @@ module VagrantPlugins def self.configure_disks(machine, defined_disks) return {} if defined_disks.empty? - return {} if !Vagrant::Util::Experimental.feature_enabled?("virtualbox_disk_hdd") + 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 diff --git a/test/unit/plugins/kernel_v2/config/vm_test.rb b/test/unit/plugins/kernel_v2/config/vm_test.rb index 8d0896771..c170c873f 100644 --- a/test/unit/plugins/kernel_v2/config/vm_test.rb +++ b/test/unit/plugins/kernel_v2/config/vm_test.rb @@ -556,7 +556,7 @@ describe VagrantPlugins::Kernel_V2::VMConfig do describe "#disk" do before(:each) do allow(Vagrant::Util::Experimental).to receive(:feature_enabled?). - with("virtualbox_disk_hdd").and_return("true") + with("disks").and_return("true") end it "stores the disks" do diff --git a/website/source/docs/disks/usage.html.md b/website/source/docs/disks/usage.html.md index 0cf074924..3fc3b4b42 100644 --- a/website/source/docs/disks/usage.html.md +++ b/website/source/docs/disks/usage.html.md @@ -16,7 +16,7 @@ description: |- This feature currently reqiures the experimental flag to be used. To explicitly enable this feature, you can set the experimental flag to: ``` - VAGRANT_EXPERIMENTAL="virtualbox_disk_hdd" + VAGRANT_EXPERIMENTAL="disks" ``` Please note that `VAGRANT_EXPERIMENTAL` is an environment variable. For more diff --git a/website/source/docs/disks/virtualbox/index.html.md b/website/source/docs/disks/virtualbox/index.html.md index 826343541..cee399420 100644 --- a/website/source/docs/disks/virtualbox/index.html.md +++ b/website/source/docs/disks/virtualbox/index.html.md @@ -17,7 +17,7 @@ description: |- This feature currently reqiures the experimental flag to be used. To explicitly enable this feature, you can set the experimental flag to: ``` - VAGRANT_EXPERIMENTAL="virtualbox_disk_hdd" + VAGRANT_EXPERIMENTAL="disks" ``` Please note that `VAGRANT_EXPERIMENTAL` is an environment variable. For more diff --git a/website/source/docs/disks/virtualbox/usage.html.md b/website/source/docs/disks/virtualbox/usage.html.md index f1b9444e0..fc179c5b8 100644 --- a/website/source/docs/disks/virtualbox/usage.html.md +++ b/website/source/docs/disks/virtualbox/usage.html.md @@ -17,7 +17,7 @@ description: |- This feature currently reqiures the experimental flag to be used. To explicitly enable this feature, you can set the experimental flag to: ``` - VAGRANT_EXPERIMENTAL="virtualbox_disk_hdd" + VAGRANT_EXPERIMENTAL="disks" ``` Please note that `VAGRANT_EXPERIMENTAL` is an environment variable. For more From baabf6650f863a0ffa0a3c2e61c42f95d4919a82 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 12 Feb 2020 15:59:37 -0800 Subject: [PATCH 123/127] Check for provider capability with disk_ext types before using it --- plugins/kernel_v2/config/disk.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/plugins/kernel_v2/config/disk.rb b/plugins/kernel_v2/config/disk.rb index 90ca1c781..5c3daf44f 100644 --- a/plugins/kernel_v2/config/disk.rb +++ b/plugins/kernel_v2/config/disk.rb @@ -140,9 +140,14 @@ module VagrantPlugins if machine.provider.capability?(:validate_disk_ext) if !machine.provider.capability(:validate_disk_ext, @disk_ext) + if machine.provider.capability?(:get_default_disk_ext) + disk_exts = machine.provider.capability(:get_default_disk_ext).join(', ') + else + disk_exts = "not found" + end errors << I18n.t("vagrant.config.disk.invalid_ext", ext: @disk_ext, name: @name, - exts: machine.provider.capability(:get_default_disk_ext).join(', ')) + exts: disk_exts) end else @logger.warn("No provider capability defined to validate 'disk_ext' type") From 37f8df174cf0a8c7c07860a7b5f9ad02f4d78ff0 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 12 Feb 2020 16:00:23 -0800 Subject: [PATCH 124/127] Freeze all values in valid disk ext default types --- plugins/providers/virtualbox/cap/validate_disk_ext.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/providers/virtualbox/cap/validate_disk_ext.rb b/plugins/providers/virtualbox/cap/validate_disk_ext.rb index 3715a91d0..e820c39ab 100644 --- a/plugins/providers/virtualbox/cap/validate_disk_ext.rb +++ b/plugins/providers/virtualbox/cap/validate_disk_ext.rb @@ -7,7 +7,7 @@ module VagrantPlugins LOGGER = Log4r::Logger.new("vagrant::plugins::virtualbox::validate_disk_ext") # The default set of disk formats that VirtualBox supports - DEFAULT_DISK_EXT = ["vdi", "vmdk", "vhd"].freeze + DEFAULT_DISK_EXT = ["vdi", "vmdk", "vhd"].map(&:freeze).freeze # @param [Vagrant::Machine] machine # @param [String] disk_ext From 42133ad34e645095b02b677cb846bf266204c3cc Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 12 Feb 2020 16:22:02 -0800 Subject: [PATCH 125/127] Move strings into locales file --- .../virtualbox/cap/configure_disks.rb | 26 ++++++++++--------- templates/locales/en.yml | 17 ++++++++++++ 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 491b20691..19df52b04 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -25,7 +25,7 @@ module VagrantPlugins raise Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit end - machine.ui.info("Configuring storage mediums...") + machine.ui.info(I18n.t("vagrant.cap.configure_disks.start")) current_disks = machine.provider.driver.list_hdds @@ -37,10 +37,10 @@ module VagrantPlugins configured_disks[:disk] << disk_data unless disk_data.empty? elsif disk.type == :floppy # TODO: Write me - machine.ui.warn("Floppy disk configuration not yet supported. Skipping disk #{disk.name}...") + machine.ui.info(I18n.t("vagrant.cap.configure_disks.floppy_not_supported", name: disk.name)) elsif disk.type == :dvd # TODO: Write me - machine.ui.warn("DVD disk configuration not yet supported. Skipping disk #{disk.name}...") + machine.ui.info(I18n.t("vagrant.cap.configure_disks.dvd_not_supported", name: disk.name)) end end @@ -118,7 +118,7 @@ module VagrantPlugins defined_disk_size = defined_disk["Capacity"].split(" ").first.to_f if defined_disk_size > requested_disk_size - machine.ui.warn("VirtualBox does not support shrinking disk size. Cannot shrink '#{disk_config.name}' disks size") + machine.ui.warn(I18n.t("vagrant.cap.configure_disks.shrink_size_not_supported", name: disk_config.name)) return false elsif defined_disk_size < requested_disk_size return true @@ -132,7 +132,7 @@ module VagrantPlugins # @param [Vagrant::Machine] machine # @param [Kernel_V2::VagrantConfigDisk] disk_config def self.create_disk(machine, disk_config) - machine.ui.detail("Disk '#{disk_config.name}' not found in guest. Creating and attaching disk to guest...") + 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. disk_provider_config = disk_config.provider_config[:virtualbox] if disk_config.provider_config @@ -187,7 +187,7 @@ module VagrantPlugins end def self.resize_disk(machine, disk_config, defined_disk) - machine.ui.detail("Disk '#{disk_config.name}' needs to be resized. Resizing disk...", prefix: true) + machine.ui.detail(I18n.t("vagrant.cap.configure_disks.resize_disk", name: disk_config.name), prefix: true) 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") @@ -219,11 +219,12 @@ 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") - rescue Exception => e + rescue Exception 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("Vagrant has encountered an exception while trying to resize a disk. It will now attempt to reattach the original disk, as to prevent any data loss.") - machine.ui.error("The original disk is located at: #{original_disk["Location"]}") - machine.ui.error("If Vagrant fails to reattach the original disk, it is recommended that you open the VirtualBox GUI and navigate to the current guests settings for '#{machine.name}' and look at the 'storage' section. Here is where you can reattach a missing disk if Vagrant fails to do so...") + + machine.ui.error(I18n.t("vagrant.cap.configure_disks.recovery_from_resize", + location: original_disk["Location"], + name: machine.name)) # move backup to original name FileUtils.mv(backup_disk_location, original_disk["Location"], force: true) # Attach disk @@ -235,8 +236,9 @@ module VagrantPlugins machine.provider.driver.close_medium(vdi_disk_file) end - machine.ui.warn("Disk has been reattached. Vagrant will now continue on an raise the exception receieved") - raise e + machine.ui.warn(I18n.t("vagrant.cap.configure_disks.recovery_attached_disks")) + + raise ensure # Remove backup disk file if all goes well FileUtils.remove(backup_disk_location, force: true) diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 6b6e5a9e9..bf738f202 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -2173,6 +2173,23 @@ en: #------------------------------------------------------------------------------- # Translations for Vagrant middleware actions #------------------------------------------------------------------------------- + cap: + 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: |- + Disk '%{name}' not found in guest. Creating and attaching disk to guest... + resize_disk: |- + Disk '%{name}' needs to be resized. Resizing disk... + recovery_from_resize: |- + Vagrant has encountered an exception while trying to resize a disk. It will now attempt to reattach the original disk, as to prevent any data loss. + The original disk is located at %{location} + If Vagrant fails to reattach the original disk, it is recommended that you open the VirtualBox GUI and navigate to the current guests settings for '%{name}' and look at the 'storage' section. Here is where you can reattach a missing disk if Vagrant fails to do so... + recovery_attached_disks: |- + Disk has been reattached. Vagrant will now continue on an raise the exception receieved actions: runner: waiting_cleanup: "Waiting for cleanup before exiting..." From 625bbf9cc84fc997ab8cefcf346542f1f462fb68 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 12 Feb 2020 16:26:18 -0800 Subject: [PATCH 126/127] Add more specific rescue exceptions for when errors occur for resizing disks --- plugins/providers/virtualbox/cap/configure_disks.rb | 2 +- .../plugins/providers/virtualbox/cap/configure_disks_test.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 19df52b04..8dcfc961f 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -219,7 +219,7 @@ 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") - rescue Exception + 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", 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 8743ff180..05ac77e68 100644 --- a/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb +++ b/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb @@ -328,7 +328,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do and_return(true) expect(driver).to receive(:close_medium).with("12345") - allow(driver).to receive(:vdi_to_vmdk).and_raise(Exception) + 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) From 96d7c19c0699e3dd6e33aa82b6b7f3abcd36958c Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 13 Feb 2020 09:42:18 -0800 Subject: [PATCH 127/127] Abstract out recovery method for resizing disk failures This commit moves out the recovery steps for failures when Vagrant attempts to resize disks. It wraps itsemf in a begin/rescue in case an additional error occurs, we can still surface the original error that caused the resizing to fail. --- .../virtualbox/cap/configure_disks.rb | 49 ++++++++++++++----- 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 8dcfc961f..5d6fe5f51 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -186,6 +186,10 @@ module VagrantPlugins dsk_info end + # @param [Vagrant::Machine] machine + # @param [Config::Disk] disk_config - the current disk to configure + # @param [Hash] defined_disk - current disk as represented by VirtualBox + # @return [Hash] - disk_metadata def self.resize_disk(machine, disk_config, defined_disk) machine.ui.detail(I18n.t("vagrant.cap.configure_disks.resize_disk", name: disk_config.name), prefix: true) @@ -221,22 +225,10 @@ module VagrantPlugins machine.provider.driver.attach_disk(disk_info[:port], disk_info[:device], vmdk_disk_file, "hdd") 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)) - # 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") - - # Remove cloned disk if still hanging around - if vdi_disk_file - machine.provider.driver.close_medium(vdi_disk_file) - end - - machine.ui.warn(I18n.t("vagrant.cap.configure_disks.recovery_attached_disks")) + recover_from_resize(machine, disk_info, backup_disk_location, original_disk, vdi_disk_file) raise ensure @@ -258,6 +250,37 @@ module VagrantPlugins disk_metadata end + + # Recovery method for when an exception occurs during the process of resizing disks + # + # It attempts to move back the backup disk into place, and reattach it to the guest before + # raising the original error + # + # @param [Vagrant::Machine] machine + # @param [Hash] disk_info - The disk device and port number to attach back to + # @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) + 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") + + # Remove cloned disk if still hanging around + if vdi_disk_file + machine.provider.driver.close_medium(vdi_disk_file) + end + + # We recovered! + machine.ui.warn(I18n.t("vagrant.cap.configure_disks.recovery_attached_disks")) + rescue => e + LOGGER.error("Vagrant encountered an error while trying to recover. It will now show the original error and continue...") + LOGGER.error(e) + end + end end end end