beckermj/vaguerent
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Feature: ISO attachment for VirtualBox

Browse Source 
This builds on the existing disk functionality, and adds some special
IDE controller-related flavor.

Considerations for IDE controllers:
- Primary/secondary attachments, so that each port can have two devices
  attached
- Adding the ability to address a specific controller name for disk
  attachment

This also prevents a user from attaching multiple instances of the same
ISO file, because VirtualBox will assign each of these the same UUID
which makes disconnection difficult. However, if multiple copies of the
ISO are attached to different devices, removing the DVD config will
cause the duplicate devices to be removed.

We may want to consider additional work to make the storage controllers
truly generic.
This commit is contained in:
Jeff Bonhag 2020-05-12 16:52:41 -04:00
parent 34747cafd6
commit c52eb1b44c
No known key found for this signature in database
GPG Key ID: 32966F3FB5AC1129
10 changed files with 317 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
plugins/kernel_v2/config/disk.rb
View File

@ -41,7 +41,8 @@ module VagrantPlugins
# @return [Integer,String]
attr_accessor :size
# Path to the location of the disk file (Optional)
# Path to the location of the disk file (Optional for `:disk` type,
# required for `:dvd` type.)
#
# @return [String]
attr_accessor :file
@ -175,6 +176,9 @@ module VagrantPlugins
errors << I18n.t("vagrant.config.disk.invalid_size", name: @name, machine: machine.name)
end
if @type == :dvd && !@file
errors << I18n.t("vagrant.config.disk.dvd_type_file_required", name: @name, machine: machine.name)
end
if @file
if !@file.is_a?(String)

62
plugins/providers/virtualbox/cap/cleanup_disks.rb
View File

@ -16,7 +16,8 @@ module VagrantPlugins
return if !Vagrant::Util::Experimental.feature_enabled?("disks")
handle_cleanup_disk(machine, defined_disks, disk_meta_file["disk"])
# TODO: Floppy and DVD disks
handle_cleanup_dvd(machine, defined_disks, disk_meta_file["dvd"])
# TODO: Floppy disks
end
protected
@ -28,23 +29,52 @@ module VagrantPlugins
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? || d["uuid"] == primary_disk
next
else
LOGGER.warn("Found disk not in Vagrantfile config: '#{d["name"]}'. Removing disk from guest #{machine.name}")
disk_info = machine.provider.driver.get_port_and_device(d["uuid"])
machine.ui.warn(I18n.t("vagrant.cap.cleanup_disks.disk_cleanup", name: d["name"]), prefix: true)
if disk_info.empty?
LOGGER.warn("Disk '#{d["name"]}' not attached to guest, but still exists.")
unless disk_meta.nil?
disk_meta.each do |d|
dsk = defined_disks.select { |dk| dk.name == d["name"] }
if !dsk.empty? || d["uuid"] == primary_disk
next
else
machine.provider.driver.remove_disk(disk_info[:port], disk_info[:device])
end
LOGGER.warn("Found disk not in Vagrantfile config: '#{d["name"]}'. Removing disk from guest #{machine.name}")
disk_info = machine.provider.driver.get_port_and_device(d["uuid"])
machine.provider.driver.close_medium(d["uuid"])
machine.ui.warn(I18n.t("vagrant.cap.cleanup_disks.disk_cleanup", name: d["name"]), prefix: true)
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
end
end
# @param [Vagrant::Machine] machine
# @param [VagrantPlugins::Kernel_V2::VagrantConfigDisk] defined_dvds
# @param [Hash] dvd_meta - A hash of all the previously defined dvds from the last configure_disk action
def self.handle_cleanup_dvd(machine, defined_dvds, dvd_meta)
controller = "IDE Controller"
vm_info = machine.provider.driver.show_vm_info
unless dvd_meta.nil?
dvd_meta.each do |d|
dsk = defined_dvds.select { |dk| dk.name == d["name"] }
if !dsk.empty?
next
else
LOGGER.warn("Found dvd not in Vagrantfile config: '#{d["name"]}'. Removing dvd from guest #{machine.name}")
(0..1).each do |port|
(0..1).each do |device|
if d["uuid"] == vm_info["#{controller}-ImageUUID-#{port}-#{device}"]
machine.ui.warn("DVD '#{d["name"]}' no longer exists in Vagrant config. Removing medium from guest...", prefix: true)
machine.provider.driver.remove_disk(port.to_s, device.to_s, controller)
end
end
end
end
end
end
end

78
plugins/providers/virtualbox/cap/configure_disks.rb
View File

@ -39,8 +39,8 @@ module VagrantPlugins
# TODO: Write me
machine.ui.info(I18n.t("vagrant.cap.configure_disks.floppy_not_supported", name: disk.name))
elsif disk.type == :dvd
# TODO: Write me
machine.ui.info(I18n.t("vagrant.cap.configure_disks.dvd_not_supported", name: disk.name))
dvd_data = handle_configure_dvd(machine, disk)
configured_disks[:dvd] << dvd_data unless dvd_data.empty?
end
end
@ -108,6 +108,45 @@ module VagrantPlugins
disk_metadata
end
# Helper method to get the UUID of a specific controller attachment
#
# @param [Vagrant::Machine] machine - the current machine
# @param [String] controller_name - the name of the controller to examine
# @param [String] port - port to look up
# @param [String] device - device to look up
def self.attachment(machine, controller_name, port, device)
vm_info = machine.provider.driver.show_vm_info
vm_info["#{controller_name}-ImageUUID-#{port}-#{device}"]
end
# Handles all disk configs of type `:dvd`
#
# @param [Vagrant::Machine] machine - the current machine
# @param [Config::Disk] dvd - the current disk to configure
# @return [Hash] - dvd_metadata
def self.handle_configure_dvd(machine, dvd)
controller = "IDE Controller"
vm_info = machine.provider.driver.show_vm_info
# Check if this DVD file is already attached
(0..1).each do |port|
(0..1).each do |device|
if vm_info["#{controller}-#{port}-#{device}"] == dvd.file
LOGGER.warn("DVD '#{dvd.file}' is already connected to guest '#{machine.name}', skipping...")
return {uuid: vm_info["#{controller}-ImageUUID-#{port}-#{device}"], name: dvd.name}
end
end
end
# New attachment
disk_info = get_next_port(machine, controller)
machine.provider.driver.attach_disk(disk_info[:port], disk_info[:device], dvd.file, "dvddrive")
attachment_uuid = attachment(machine, controller, disk_info[:port], disk_info[:device])
{uuid: attachment_uuid, name: dvd.name}
end
# Check to see if current disk is configured based on defined_disks
#
# @param [Kernel_V2::VagrantConfigDisk] disk_config
@ -167,19 +206,38 @@ module VagrantPlugins
# device = disk_info[3]
#
# @param [Vagrant::Machine] machine
# @param [String] controller name (defaults to "SATA Controller")
# @return [Hash] dsk_info - The next available port and device on a given controller
def self.get_next_port(machine)
def self.get_next_port(machine, controller="SATA Controller")
vm_info = machine.provider.driver.show_vm_info
dsk_info = {device: "0", port: "0"}
dsk_info = {}
disk_images = vm_info.select { |v| v.include?("ImageUUID") && v.include?("SATA Controller") }
used_ports = disk_images.keys.map { |k| k.split('-') }.map {|v| v[2].to_i}
next_available_port = ((0..(MAX_DISK_NUMBER-1)).to_a - used_ports).first
if controller == "SATA Controller"
disk_images = vm_info.select { |v| v.include?("ImageUUID") && v.include?(controller) }
used_ports = disk_images.keys.map { |k| k.split('-') }.map {|v| v[2].to_i}
next_available_port = ((0..(MAX_DISK_NUMBER-1)).to_a - used_ports).first
dsk_info[:port] = next_available_port.to_s
if dsk_info[:port].empty?
dsk_info[:port] = next_available_port.to_s
dsk_info[:device] = "0"
else
# IDE Controllers have primary/secondary devices, so find the first port
# with an empty device
(0..1).each do |port|
break if dsk_info[:port] && dsk_info[:device]
(0..1).each do |device|
if vm_info["#{controller}-ImageUUID-#{port}-#{device}"].to_s.empty?
dsk_info[:port] = port.to_s
dsk_info[:device] = device.to_s
break
end
end
end
end
if dsk_info[:port].to_s.empty?
# This likely only occurs if additional disks have been added outside of Vagrant configuration
LOGGER.warn("There are no more available ports to attach disks to for the SATA Controller. Clear up some space on the SATA controller to attach new disks.")
LOGGER.warn("There are no more available ports to attach disks to for the controller '#{controller}'. Clear up some space on the controller '#{controller}' to attach new disks.")
raise Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit
end

25
plugins/providers/virtualbox/driver/version_5_0.rb
View File

@ -29,13 +29,21 @@ module VagrantPlugins
# @param [Hash] opts - additional options
def attach_disk(port, device, file, type="hdd", **opts)
# Maybe only support SATA Controller for `:disk`???
controller = "SATA Controller"
if type == "hdd"
controller = "SATA Controller"
else
controller = "IDE Controller"
end
comment = "This disk is managed externally by Vagrant. Removing or adjusting settings could potentially cause issues with Vagrant."
execute('storageattach', @uuid, '--storagectl', controller, '--port',
port.to_s, '--device', device.to_s, '--type', type, '--medium',
file, '--comment', comment)
execute('storageattach', @uuid,
'--storagectl', controller,
'--port', port.to_s,
'--device', device.to_s,
'--type', type,
'--medium', file,
'--comment', comment)
end
def clear_forwarded_ports
@ -227,9 +235,12 @@ 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")
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

2
templates/locales/en.yml
View File

@ -1844,6 +1844,8 @@ en:
#-------------------------------------------------------------------------------
config:
disk:
dvd_type_file_required:
A 'file' option is required when defining a disk of type `:dvd` for guest '%{machine}'.
invalid_ext: |-
Disk type '%{ext}' is not a valid disk extention for '%{name}'. Please pick one of the following supported disk types: %{exts}
invalid_type: |-

29
test/unit/plugins/kernel_v2/config/disk_test.rb
View File

@ -15,17 +15,16 @@ describe VagrantPlugins::Kernel_V2::VagrantConfigDisk do
let(:machine) { double("machine", name: "name", provider: provider, env: env,
provider_name: :virtualbox) }
def assert_invalid
errors = subject.validate(machine)
if !errors.empty? { |v| !v.empty? }
if errors.empty?
raise "No errors: #{errors.inspect}"
end
end
def assert_valid
errors = subject.validate(machine)
if !errors.empty? { |v| v.empty? }
if !errors.empty?
raise "Errors: #{errors.inspect}"
end
end
@ -52,7 +51,7 @@ describe VagrantPlugins::Kernel_V2::VagrantConfigDisk do
expect(subject.type).to eq(type)
end
it "defaults to non-primray disk" do
it "defaults to non-primary disk" do
subject.finalize!
expect(subject.primary).to eq(false)
end
@ -79,4 +78,26 @@ describe VagrantPlugins::Kernel_V2::VagrantConfigDisk do
end
end
end
describe "config for dvd type" do
let(:iso_path) { "/tmp/untitled.iso" }
before do
subject.type = :dvd
subject.name = "untitled"
end
it "is valid with file path set" do
allow(File).to receive(:file?).with(iso_path).and_return(true)
subject.file = iso_path
subject.finalize!
assert_valid
end
it "is invalid if file path is unset" do
subject.finalize!
errors = subject.validate(machine)
assert_invalid
end
end
end

4
test/unit/plugins/kernel_v2/config/vm_trigger_test.rb
View File

@ -13,14 +13,14 @@ describe VagrantPlugins::Kernel_V2::VagrantConfigTrigger do
def assert_invalid
errors = subject.validate(machine)
if !errors.empty? { |v| !v.empty? }
if errors.empty?
raise "No errors: #{errors.inspect}"
end
end
def assert_valid
errors = subject.validate(machine)
if !errors.empty? { |v| v.empty? }
if !errors.empty?
raise "Errors: #{errors.inspect}"
end
end

44
test/unit/plugins/providers/virtualbox/cap/cleanup_disks_test.rb
View File

@ -38,13 +38,13 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::CleanupDisks do
allow(driver).to receive(:show_vm_info).and_return(vm_info)
end
context "#cleanup_disks" do
describe "#cleanup_disks" do
it "returns if there's no data in meta file" do
subject.cleanup_disks(machine, defined_disks, disk_meta_file)
expect(subject).not_to receive(:handle_cleanup_disk)
end
describe "with disks to clean up" do
context "with disks to clean up" do
let(:disk_meta_file) { {disk: [{uuid: "1234", name: "storage"}], floppy: [], dvd: []} }
it "calls the cleanup method if a disk_meta file is defined" do
@ -55,9 +55,21 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::CleanupDisks do
subject.cleanup_disks(machine, defined_disks, disk_meta_file)
end
end
context "with dvd attached" do
let(:disk_meta_file) { {dvd: [{uuid: "12345", name: "iso"}]} }
it "calls the cleanup method if a disk_meta file is defined" do
expect(subject).to receive(:handle_cleanup_dvd).
with(machine, defined_disks, disk_meta_file["dvd"]).
and_return(true)
subject.cleanup_disks(machine, defined_disks, disk_meta_file)
end
end
end
context "#handle_cleanup_disk" do
describe "#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"} }
@ -73,7 +85,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::CleanupDisks do
subject.handle_cleanup_disk(machine, defined_disks, disk_meta_file[:disk])
end
describe "when the disk isn't attached to a guest" do
context "when the disk isn't attached to a guest" do
it "only closes the medium" do
allow(driver).to receive(:get_port_and_device).
with("67890").
@ -85,4 +97,28 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::CleanupDisks do
end
end
end
describe "#handle_cleanup_dvd" do
let(:vm_info) { {"IDE Controller-ImageUUID-0-0" => "1234" } }
let(:disk_meta_file) { {dvd: [{"uuid" => "1234", "name" => "iso"}]} }
let(:defined_disks) { [] }
it "removes the medium from guest" do
expect(driver).to receive(:remove_disk).with("0", "0", "IDE Controller").and_return(true)
subject.handle_cleanup_dvd(machine, defined_disks, disk_meta_file[:dvd])
end
context "multiple copies of the same ISO attached" do
let(:vm_info) { {"IDE Controller-ImageUUID-0-0" => "1234",
"IDE Controller-ImageUUID-0-1" => "1234"} }
it "removes all media with that UUID" do
expect(driver).to receive(:remove_disk).with("0", "0", "IDE Controller").and_return(true)
expect(driver).to receive(:remove_disk).with("0", "1", "IDE Controller").and_return(true)
subject.handle_cleanup_dvd(machine, defined_disks, disk_meta_file[:dvd])
end
end
end
end

101
test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb
View File

@ -68,7 +68,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
allow(driver).to receive(:show_vm_info).and_return(vm_info)
end
context "#configure_disks" do
describe "#configure_disks" do
let(:dsk_data) { {uuid: "1234", name: "disk"} }
it "configures disks and returns the disks defined" do
allow(driver).to receive(:list_hdds).and_return([])
@ -77,14 +77,14 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
subject.configure_disks(machine, defined_disks)
end
describe "with no disks to configure" do
context "with no disks to configure" do
let(:defined_disks) { {} }
it "returns empty hash if no disks to configure" do
expect(subject.configure_disks(machine, defined_disks)).to eq({})
end
end
describe "with over the disk limit for a given device" do
context "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
@ -92,9 +92,20 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
to raise_error(Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit)
end
end
context "with dvd type" do
let(:defined_disks) { [double("dvd", type: :dvd)] }
let(:dvd_data) { {uuid: "1234", name: "dvd"} }
it "handles configuration of the dvd" do
allow(driver).to receive(:list_hdds).and_return([])
expect(subject).to receive(:handle_configure_dvd).and_return(dvd_data)
subject.configure_disks(machine, defined_disks)
end
end
end
context "#get_current_disk" do
describe "#get_current_disk" do
it "gets primary disk uuid if disk to configure is primary" do
primary_disk = subject.get_current_disk(machine, defined_disks.first, all_disks)
expect(primary_disk).to eq(all_disks.first)
@ -111,8 +122,8 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
end
end
context "#handle_configure_disk" do
describe "when creating a new disk" do
describe "#handle_configure_disk" do
context "when creating a new disk" do
let(:all_disks) { [{"UUID"=>"12345",
"Parent UUID"=>"base",
"State"=>"created",
@ -133,7 +144,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
end
end
describe "when a disk needs to be resized" do
context "when a disk needs to be resized" do
let(:all_disks) { [{"UUID"=>"12345",
"Parent UUID"=>"base",
"State"=>"created",
@ -167,7 +178,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
end
end
describe "if no additional disk configuration is required" do
context "if no additional disk configuration is required" do
let(:all_disks) { [{"UUID"=>"12345",
"Parent UUID"=>"base",
"State"=>"created",
@ -221,7 +232,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
end
end
context "#compare_disk_size" do
describe "#compare_disk_size" do
let(:disk_config_small) { double("disk", name: "disk-0", size: 1073741824.0, primary: false, type: :disk) }
let(:disk_config_large) { double("disk", name: "disk-0", size: 68719476736.0, primary: false, type: :disk) }
@ -235,7 +246,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
end
end
context "#create_disk" do
describe "#create_disk" do
let(:disk_config) { double("disk", name: "disk-0", size: 1073741824.0,
primary: false, type: :disk, disk_ext: "vdi",
provider_config: nil) }
@ -262,16 +273,39 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
end
end
context "#get_next_port" do
it "determines the next available port to use" do
describe ".get_next_port" do
it "determines the next available port and device to use" do
dsk_info = subject.get_next_port(machine)
expect(dsk_info[:device]).to eq("0")
expect(dsk_info[:port]).to eq("2")
expect(dsk_info[:device]).to eq("0")
end
context "with an IDE controller" do
let(:vm_info) { {"IDE Controller-ImageUUID-0-0" => "00000aaaaa",
"IDE Controller-ImageUUID-0-1" => "11111bbbbb" } }
it "determines the next available port and device to use" do
dsk_info = subject.get_next_port(machine, "IDE Controller")
expect(dsk_info[:port]).to eq("1")
expect(dsk_info[:device]).to eq("0")
end
end
context "with a full IDE controller" do
let(:vm_info) { {"IDE Controller-ImageUUID-0-0" => "00000aaaaa",
"IDE Controller-ImageUUID-0-1" => "11111bbbbb",
"IDE Controller-ImageUUID-1-0" => "22222ccccc",
"IDE Controller-ImageUUID-1-1" => "33333ddddd"} }
it "raises an error" do
expect { subject.get_next_port(machine, "IDE Controller") }
.to raise_error(Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit)
end
end
end
context "#resize_disk" do
describe "when a disk is vmdk format" do
describe "#resize_disk" do
context "when a disk is vmdk format" do
let(:disk_config) { double("disk", name: "vagrant_primary", size: 1073741824.0,
primary: false, type: :disk, disk_ext: "vmdk",
provider_config: nil) }
@ -341,7 +375,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
end
end
describe "when a disk is vdi format" do
context "when a disk is vdi format" do
let(:disk_config) { double("disk", name: "disk-0", size: 1073741824.0,
primary: false, type: :disk, disk_ext: "vdi",
provider_config: nil) }
@ -353,13 +387,44 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
end
end
context "#vmdk_to_vdi" do
describe "#vmdk_to_vdi" do
it "converts a disk from vmdk to vdi" do
end
end
context "#vdi_to_vmdk" do
describe "#vdi_to_vmdk" do
it "converts a disk from vdi to vmdk" do
end
end
describe ".handle_configure_dvd" do
let(:dvd_config) { double("dvd", file: "/tmp/untitled.iso", name: "dvd1") }
before do
allow(subject).to receive(:get_next_port).with(machine, "IDE Controller").and_return({device: "0", port: "0"})
end
it "attaches to the next available port and device" do
expect(driver).to receive(:attach_disk).with("0", "0", "/tmp/untitled.iso", "dvddrive")
subject.handle_configure_dvd(machine, dvd_config)
end
it "returns the UUID of the newly-attached dvd" do
allow(subject).to receive(:attachment).with(machine, "IDE Controller", "0", "0").and_return("12345")
expect(driver).to receive(:attach_disk).with("0", "0", "/tmp/untitled.iso", "dvddrive")
disk_meta = subject.handle_configure_dvd(machine, dvd_config)
expect(disk_meta[:uuid]).to eq("12345")
end
context "when an ISO file is already attached" do
let(:vm_info) { {"IDE Controller-0-0" => "/tmp/untitled.iso" } }
it "skips the attachment" do
expect(driver).not_to receive(:attach_disk)
subject.handle_configure_dvd(machine, dvd_config)
end
end
end
end

28
test/unit/plugins/providers/virtualbox/driver/version_5_0_test.rb
View File

@ -90,4 +90,32 @@ OUTPUT
end
end
end
describe "#attach_disk" do
it "attaches a dvddrive device to the IDE controller" do
expect(subject).to receive(:execute) do |*args|
storagectl = args[args.index("--storagectl") + 1]
expect(storagectl).to eq("IDE Controller")
end
subject.attach_disk(anything, anything, anything, "dvddrive")
end
end
describe "#remove_disk" do
it "removes a disk from the SATA Controller by default" do
expect(subject).to receive(:execute) do |*args|
storagectl = args[args.index("--storagectl") + 1]
expect(storagectl).to eq("SATA Controller")
end
subject.remove_disk(anything, anything)
end
it "can remove a disk from the specified controller" do
expect(subject).to receive(:execute) do |*args|
storagectl = args[args.index("--storagectl") + 1]
expect(storagectl).to eq("IDE Controller")
end
subject.remove_disk(anything, anything, "IDE Controller")
end
end
end