Update virtualbox configure_disks to use storage controller attachments

This commit updates how Vagrant manages disk state with the virtualbox
provider. Instead of using the raw structure from `list_hdds` for
all_disks, it instead uses that data that now lives inside the
controllers attachment structure.
This commit is contained in:
Brian Cain 2020-07-14 09:22:55 -07:00
parent f33d513969
commit ece5449dd2
No known key found for this signature in database
GPG key ID: 9FC4639B2E4510A0
2 changed files with 124 additions and 115 deletions

View file

@ -93,12 +93,9 @@ module VagrantPlugins
current_disk = nil
if disk.primary
storage_controllers = machine.provider.driver.read_storage_controllers
primary = storage_controllers.get_primary_attachment
primary_uuid = primary[:uuid]
current_disk = all_disks.select { |d| d["UUID"] == primary_uuid }.first
current_disk = storage_controllers.get_primary_attachment
else
current_disk = all_disks.detect { |d| d["Disk Name"] == disk.name }
current_disk = all_disks.detect { |d| d[:disk_name] == disk.name }
end
current_disk
@ -113,18 +110,7 @@ module VagrantPlugins
def self.handle_configure_disk(machine, disk, controller_name)
storage_controllers = machine.provider.driver.read_storage_controllers
controller = storage_controllers.get_controller(controller_name)
# Filter current guest disks from all disks
#
# NOTE: This is required beyond just using the entries in
# `controller.attachments` because `list_hdds` gives us more information
# about a disk such as its capacity and disk type, however `list_hdds`
# lists every single harddisk in VirtualBox, so we need to filter on
# our known UUIDs in controller attachments to obtain this info
all_disks = []
machine.provider.driver.list_hdds.each do |hdd|
all_disks << hdd if controller.attachments.detect { |v| v[:uuid] == hdd["UUID"] }
end
all_disks = controller.attachments
disk_metadata = {}
@ -140,7 +126,7 @@ module VagrantPlugins
else
# TODO: What if it needs to be resized?
disk_info = machine.provider.driver.get_port_and_device(current_disk["UUID"])
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, controller)
@ -148,7 +134,7 @@ module VagrantPlugins
dsk_info[:port],
dsk_info[:device],
"hdd",
current_disk["Location"])
current_disk[:location])
disk_metadata[:port] = dsk_info[:port]
disk_metadata[:device] = dsk_info[:device]
else
@ -157,7 +143,7 @@ module VagrantPlugins
disk_metadata[:device] = disk_info[:device]
end
disk_metadata[:uuid] = current_disk["UUID"]
disk_metadata[:uuid] = current_disk[:uuid]
disk_metadata[:name] = disk.name
disk_metadata[:controller] = controller.name
end
@ -220,7 +206,7 @@ module VagrantPlugins
# @return [Boolean]
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
defined_disk_size = defined_disk[:capacity].split(" ").first.to_f
if defined_disk_size > requested_disk_size
machine.ui.warn(I18n.t("vagrant.cap.configure_disks.shrink_size_not_supported", name: disk_config.name))
@ -336,17 +322,17 @@ module VagrantPlugins
machine.ui.detail(I18n.t("vagrant.cap.configure_disks.resize_disk", name: disk_config.name), prefix: true)
# grab disk to be resized port and device number
disk_info = machine.provider.driver.get_port_and_device(defined_disk["UUID"])
disk_info = machine.provider.driver.get_port_and_device(defined_disk[:uuid])
if defined_disk["Storage format"] == "VMDK"
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")
# original disk information in case anything goes wrong during clone/resize
original_disk = defined_disk
backup_disk_location = "#{original_disk["Location"]}.backup"
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"])
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)
@ -355,12 +341,12 @@ module VagrantPlugins
# remove and close original volume
machine.provider.driver.remove_disk(controller.name, disk_info[:port], disk_info[:device])
# Create a backup of the original disk if something goes wrong
LOGGER.warn("Making a backup of the original disk at #{defined_disk["Location"]}")
FileUtils.mv(defined_disk["Location"], backup_disk_location)
LOGGER.warn("Making a backup of the original disk at #{defined_disk[:location]}")
FileUtils.mv(defined_disk[:location], backup_disk_location)
# we have to close here, otherwise we can't re-clone after
# resizing the vdi disk
machine.provider.driver.close_medium(defined_disk["UUID"])
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)
@ -372,7 +358,7 @@ module VagrantPlugins
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"],
location: original_disk[:location],
name: machine.name))
recover_from_resize(machine, disk_info, backup_disk_location, original_disk, vdi_disk_file, controller)
raise
@ -385,13 +371,16 @@ module VagrantPlugins
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
storage_controllers = machine.provider.driver.read_storage_controllers
updated_controller = storage_controllers.get_controller(controller.name)
new_disk_info = updated_controller.attachments.detect { |h| h[:location] == defined_disk[:location] }
defined_disk = new_disk_info
else
machine.provider.driver.resize_disk(defined_disk["Location"], disk_config.size.to_i)
machine.provider.driver.resize_disk(defined_disk[:location], disk_config.size.to_i)
end
disk_metadata = { uuid: defined_disk["UUID"], name: disk_config.name, controller: controller.name,
disk_metadata = { uuid: defined_disk[:uuid], name: disk_config.name, controller: controller.name,
port: disk_info[:port], device: disk_info[:device] }
disk_metadata
@ -411,13 +400,13 @@ module VagrantPlugins
def self.recover_from_resize(machine, disk_info, backup_disk_location, original_disk, vdi_disk_file, controller)
begin
# move backup to original name
FileUtils.mv(backup_disk_location, original_disk["Location"], force: true)
FileUtils.mv(backup_disk_location, original_disk[:location], force: true)
# Attach disk
machine.provider.driver.attach_disk(controller.name,
disk_info[:port],
disk_info[:device],
"hdd",
original_disk["Location"])
original_disk[:location])
# Remove cloned disk if still hanging around
if vdi_disk_file

View file

@ -31,49 +31,75 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
let(:attachments) { [{:port=>"0", :device=>"0",
:uuid=>"12345",
:storage_format=>"VMDK",
:capacity=>"65536 MBytes",
:disk_name=>"ubuntu-18.04-amd64-disk001",
:location=>"/home/vagrant/VirtualBox VMs/ubuntu-18.04-amd64-disk001.vmdk"},
{:port=>"1", :device=>"0",
:uuid=>"67890",
:storage_format=>"VDI",
:capacity=>"10240 MBytes",
:disk_name=>"disk-0",
:location=>"/home/vagrant/VirtualBox VMs/disk-0.vdi"},
{:port=>"2", :device=>"0",
:uuid=>"10111",
:storage_format=>"VDI",
:capacity=>"10240 MBytes",
:disk_name=>"disk-1",
:location=>"/home/vagrant/VirtualBox VMs/disk-1.vdi"}] }
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),
let(:defined_disks) { [double("disk", name: "vagrant_primary", size: Vagrant::Util::Numeric::string_to_bytes("65GB"), primary: true, type: :disk),
double("disk", name: "disk-0", size: Vagrant::Util::Numeric::string_to_bytes("10GB"), primary: false, type: :disk),
double("disk", name: "disk-1", size: "10GB", 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/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/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/disk-1.vdi",
"Disk Name"=>"disk-1",
"Storage format"=>"VDI",
"Capacity"=>"5120 MBytes",
"Encryption"=>"disabled"}] }
let(:all_disks) { [{:port=>"0", :device=>"0",
:uuid=>"12345",
:storage_format=>"VMDK",
:capacity=>"65536 MBytes",
:disk_name=>"ubuntu-18.04-amd64-disk001",
:location=>"/home/vagrant/VirtualBox VMs/ubuntu-18.04-amd64-disk001.vmdk"},
{:port=>"1", :device=>"0",
:uuid=>"67890",
:storage_format=>"VDI",
:capacity=>"10240 MBytes",
:disk_name=>"disk-0",
:location=>"/home/vagrant/VirtualBox VMs/disk-0.vdi"},
{:port=>"2", :device=>"0",
:uuid=>"10111",
:storage_format=>"VDI",
:capacity=>"10240 MBytes",
:disk_name=>"disk-1",
:location=>"/home/vagrant/VirtualBox VMs/disk-1.vdi"}] }
let(:list_hdds_result) { [{"UUID"=>"12345",
"Parent UUID"=>"base",
"State"=>"created",
"Type"=>"normal (base)",
"Location"=>"/home/vagrant/VirtualBox VMs/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/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/disk-1.vdi",
"Disk Name"=>"disk-1",
"Storage format"=>"VDI",
"Capacity"=>"5120 MBytes",
"Encryption"=>"disabled"}] }
let(:subject) { described_class }
@ -212,21 +238,19 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
describe "#handle_configure_disk" do
context "when creating a new disk" do
let(:all_disks) { [{"UUID"=>"12345",
"Parent UUID"=>"base",
"State"=>"created",
"Type"=>"normal (base)",
"Location"=>"/home/vagrant/VirtualBox VMs/ubuntu-18.04-amd64-disk001.vmdk",
"Disk Name"=>"ubuntu-18.04-amd64-disk001",
"Storage format"=>"VMDK",
"Capacity"=>"65536 MBytes",
"Encryption"=>"disabled"}] }
let(:all_disks) { [{:port=>"0", :device=>"0",
:uuid=>"12345",
:storage_format=>"VMDK",
:capacity=>"65536 MBytes",
:disk_name=>"ubuntu-18.04-amd64-disk001",
:location=>"/home/vagrant/VirtualBox VMs/ubuntu-18.04-amd64-disk001.vmdk"}] }
let(:disk_meta) { {uuid: "67890", name: "disk-0", controller: "controller", port: "1", device: "1"} }
it "creates a new disk if it doesn't yet exist" do
expect(subject).to receive(:create_disk).with(machine, defined_disks[1], controller)
.and_return(disk_meta)
expect(controller).to receive(:attachments).and_return(all_disks)
subject.handle_configure_disk(machine, defined_disks[1], controller.name)
end
@ -234,6 +258,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
it "includes disk attachment info in metadata" do
expect(subject).to receive(:create_disk).with(machine, defined_disks[1], controller)
.and_return(disk_meta)
expect(controller).to receive(:attachments).and_return(all_disks)
disk_metadata = subject.handle_configure_disk(machine, defined_disks[1], controller.name)
expect(disk_metadata).to have_key(:controller)
@ -244,26 +269,22 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
end
context "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/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/disk-0.vdi",
"Disk Name"=>"disk-0",
"Storage format"=>"VDI",
"Capacity"=>"10240 MBytes",
"Encryption"=>"disabled"}] }
let(:all_disks) { [{:port=>"0", :device=>"0",
:uuid=>"12345",
:storage_format=>"VMDK",
:capacity=>"65536 MBytes",
:disk_name=>"ubuntu-18.04-amd64-disk001",
:location=>"/home/vagrant/VirtualBox VMs/ubuntu-18.04-amd64-disk001.vmdk"},
{:port=>"1", :device=>"0",
:uuid=>"67890",
:storage_format=>"VDI",
:capacity=>"10240 MBytes",
:disk_name=>"disk-0",
:location=>"/home/vagrant/VirtualBox VMs/disk-0.vdi"}] }
it "resizes a disk" do
expect(controller).to receive(:attachments).and_return(all_disks)
expect(subject).to receive(:get_current_disk).
with(machine, defined_disks[1], all_disks).and_return(all_disks[1])
@ -278,24 +299,18 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
end
context "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/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/disk-0.vdi",
"Disk Name"=>"disk-0",
"Storage format"=>"VDI",
"Capacity"=>"10240 MBytes",
"Encryption"=>"disabled"}] }
let(:all_disks) { [{:port=>"0", :device=>"0",
:uuid=>"12345",
:storage_format=>"VMDK",
:capacity=>"65536 MBytes",
:disk_name=>"ubuntu-18.04-amd64-disk001",
:location=>"/home/vagrant/VirtualBox VMs/ubuntu-18.04-amd64-disk001.vmdk"},
{:port=>"1", :device=>"0",
:uuid=>"67890",
:storage_format=>"VDI",
:capacity=>"10240 MBytes",
:disk_name=>"disk-0",
:location=>"/home/vagrant/VirtualBox VMs/disk-0.vdi"}] }
let(:disk_info) { {port: "1", device: "0"} }
@ -309,6 +324,8 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
:location=>"/home/vagrant/VirtualBox VMs/disk-0.vdi"}] }
it "reattaches disk if vagrant defined disk exists but is not attached to guest" do
expect(controller).to receive(:attachments).and_return(all_disks)
expect(subject).to receive(:get_current_disk).
with(machine, defined_disks[1], all_disks).and_return(all_disks[1])
@ -322,12 +339,14 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
(disk_info[:port].to_i + 1).to_s,
disk_info[:device],
"hdd",
all_disks[1]["Location"])
all_disks[1][:location])
subject.handle_configure_disk(machine, defined_disks[1], controller.name)
end
it "does nothing if all disks are properly configured" do
expect(controller).to receive(:attachments).and_return(all_disks)
expect(subject).to receive(:get_current_disk).
with(machine, defined_disks[1], all_disks).and_return(all_disks[1])
@ -459,7 +478,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
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"]).
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)
@ -475,7 +494,8 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
with(controller.name, attach_info[:port], attach_info[:device], "hdd", vmdk_disk_file).and_return(true)
expect(driver).to receive(:close_medium).with(vdi_disk_file).and_return(true)
expect(driver).to receive(:list_hdds).and_return(all_disks)
expect(driver).to receive(:read_storage_controllers)
expect(storage_controllers).to receive(:get_controller)
expect(FileUtils).to receive(:remove).with("#{vmdk_disk_file}.backup", force: true).
and_return(true)
@ -490,7 +510,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
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"]).
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)
@ -512,9 +532,9 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
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)
expect(driver).to receive(:resize_disk).with(all_disks[1][:location], disk_config.size.to_i)
expect(driver).to receive(:get_port_and_device).with(all_disks[1]["UUID"]).
expect(driver).to receive(:get_port_and_device).with(all_disks[1][:uuid]).
and_return({port: "1", device: "0"})
subject.resize_disk(machine, disk_config, all_disks[1], controller)