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

Refactor disk configuration to use dynamic names

Browse Source 
This commit adds a new VirtualBox provider helper method to return a
list of storage controllers so Vagrant can find a storage controller
with the desired characteristics (type IDE or SATA).

This still needs to get wired up to the disk cleanup method.
This commit is contained in:
Jeff Bonhag 2020-05-27 12:28:47 -04:00
parent 8985369eab
commit 8407d79100
No known key found for this signature in database
GPG Key ID: 32966F3FB5AC1129
5 changed files with 146 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -9,8 +9,8 @@ module VagrantPlugins
module ConfigureDisks
LOGGER = Log4r::Logger.new("vagrant::plugins::virtualbox::configure_disks")
# The max amount of disks that can be attached to a single device in a controller
MAX_DISK_NUMBER = 30.freeze
SATA_CONTROLLER_TYPES = ["IntelAhci"].freeze
IDE_CONTROLLER_TYPES = ["PIIX4", "PIIX3", "ICH6"].freeze
# @param [Vagrant::Machine] machine
# @param [VagrantPlugins::Kernel_V2::VagrantConfigDisk] defined_disks
@ -20,8 +20,15 @@ module VagrantPlugins
return {} if !Vagrant::Util::Experimental.feature_enabled?("disks")
if defined_disks.size > MAX_DISK_NUMBER
# you can only attach up to 30 disks per controller, INCLUDING the primary disk
disks_defined = defined_disks.select { |d| d.type == :disk }
controller = sata_controller(machine)
if disks_defined.any? && controller && disks_defined.size > controller[:maxportcount]
raise Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit
end
dvds_defined = defined_disks.select { |d| d.type == :dvd }
controller = ide_controller(machine)
if dvds_defined.any? && controller && dvds_defined.size > controller[:maxportcount]
raise Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit
end
@ -59,8 +66,9 @@ module VagrantPlugins
# Ensure we grab the proper primary disk
# We can't rely on the order of `all_disks`, as they will not
# always come in port order, but primary is always Port 0 Device 0.
vm_info = machine.provider.driver.show_vm_info
primary_uuid = vm_info["SATA Controller-ImageUUID-0-0"]
controller = sata_controller(machine)
primary = controller[:attachments].detect { |a| a[:port] == "0" && a[:device] == "0" }
primary_uuid = primary[:uuid]
current_disk = all_disks.select { |d| d["UUID"] == primary_uuid }.first
else
@ -108,6 +116,14 @@ module VagrantPlugins
disk_metadata
end
def self.sata_controller(machine)
machine.provider.driver.storage_controllers.detect { |c| SATA_CONTROLLER_TYPES.include?(c[:type]) }
end
def self.ide_controller(machine)
machine.provider.driver.storage_controllers.detect { |c| IDE_CONTROLLER_TYPES.include?(c[:type]) }
end
# Helper method to get the UUID of a specific controller attachment
#
# @param [Vagrant::Machine] machine - the current machine
@ -125,26 +141,21 @@ module VagrantPlugins
# @param [Config::Disk] dvd - the current disk to configure
# @return [Hash] - dvd_metadata
def self.handle_configure_dvd(machine, dvd)
controller = "IDE Controller"
vm_info = machine.provider.driver.show_vm_info
controller = ide_controller(machine)
# Check if this DVD file is already attached
(0..1).each do |port|
(0..1).each do |device|
if vm_info["#{controller}-#{port}-#{device}"] == dvd.file
LOGGER.warn("DVD '#{dvd.file}' is already connected to guest '#{machine.name}', skipping...")
return {uuid: vm_info["#{controller}-ImageUUID-#{port}-#{device}"], name: dvd.name}
end
end
end
# New attachment
disk_info = get_next_port(machine, controller)
disk_info = get_next_port(machine, controller[:name])
machine.provider.driver.attach_disk(disk_info[:port], disk_info[:device], dvd.file, "dvddrive")
attachment_uuid = attachment(machine, controller, disk_info[:port], disk_info[:device])
# Refresh the controller information
controller = ide_controller(machine)
attachment = controller[:attachments].detect { |c| c[:port] == disk_info[:port] &&
c[:device] == disk_info[:device] }
{uuid: attachment_uuid, name: dvd.name}
if attachment
{uuid: attachment[:uuid], name: dvd.name}
else
{}
end
end
# Check to see if current disk is configured based on defined_disks
@ -208,29 +219,32 @@ module VagrantPlugins
# @param [Vagrant::Machine] machine
# @param [String] controller name (defaults to "SATA Controller")
# @return [Hash] dsk_info - The next available port and device on a given controller
def self.get_next_port(machine, controller="SATA Controller")
vm_info = machine.provider.driver.show_vm_info
def self.get_next_port(machine, controller_name="SATA Controller")
controller = machine.provider.driver.storage_controllers.detect { |c| c[:name] == controller_name }
# definitely need an error for this
dsk_info = {}
if controller == "SATA Controller"
disk_images = vm_info.select { |v| v.include?("ImageUUID") && v.include?(controller) }
used_ports = disk_images.keys.map { |k| k.split('-') }.map {|v| v[2].to_i}
next_available_port = ((0..(MAX_DISK_NUMBER-1)).to_a - used_ports).first
if controller[:type] == "IntelAhci" # SATA Controller
used_ports = controller[:attachments].map { |a| a[:port].to_i }
next_available_port = ((0..(controller[:maxportcount]-1)).to_a - used_ports).first
dsk_info[:port] = next_available_port.to_s
dsk_info[:device] = "0"
else
# IDE Controllers have primary/secondary devices, so find the first port
# with an empty device
(0..1).each do |port|
break if dsk_info[:port] && dsk_info[:device]
(0..(controller[:maxportcount]-1)).each do |port|
# Skip this port if it's full
port_attachments = controller[:attachments].select { |a| a[:port] == port.to_s }
next if port_attachments.count == 2
(0..1).each do |device|
if vm_info["#{controller}-ImageUUID-#{port}-#{device}"].to_s.empty?
dsk_info[:port] = port.to_s
dsk_info[:device] = device.to_s
break
end
dsk_info[:port] = port.to_s
# Check for a free device
if port_attachments.detect { |a| a[:device] == "0" }
dsk_info[:device] = "1"
else
dsk_info[:device] = "0"
end
end
end

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

@ -934,6 +934,35 @@ module VagrantPlugins
destination
end
# Helper method to get a list of storage controllers added to the
# current VM
def storage_controllers
vm_info = show_vm_info
count = vm_info.count { |key, value| key.match(/^storagecontrollername/) }
(0..count - 1).map do |n|
# basic controller metadata
name = vm_info["storagecontrollername#{n}"]
type = vm_info["storagecontrollertype#{n}"]
maxportcount = vm_info["storagecontrollermaxportcount#{n}"].to_i
# build attachments array
attachments = []
vm_info.each do |k, v|
if /^#{name}-ImageUUID-(\d+)-(\d+)$/ =~ k
attachments << {port: $1.to_s, device: $2.to_s, uuid: v}
end
end
{
name: name,
type: type,
maxportcount: maxportcount,
attachments: attachments
}
end
end
protected
def valid_ip_address?(ip)

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

@ -33,9 +33,13 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::CleanupDisks do
let(:vm_info) { {"SATA Controller-ImageUUID-0-0" => "12345",
"SATA Controller-ImageUUID-1-0" => "67890"} }
let(:storage_controllers) {
}
before do
allow(Vagrant::Util::Experimental).to receive(:feature_enabled?).and_return(true)
allow(driver).to receive(:show_vm_info).and_return(vm_info)
allow(driver).to receive(:storage_controllers).and_return(storage_controllers)
end
describe "#cleanup_disks" do

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

@ -25,9 +25,18 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
double(:state)
end
let(:vm_info) { {"SATA Controller-ImageUUID-0-0" => "12345",
let(:vm_info) { {"storagecontrollername0" => "SATA Controller",
"storagecontrollertype0" => "IntelAhci",
"storagecontrollermaxportcount0" => "30",
"SATA Controller-ImageUUID-0-0" => "12345",
"SATA Controller-ImageUUID-1-0" => "67890"} }
let(:storage_controllers) { [{name: "SATA Controller",
type: "IntelAhci",
maxportcount: 30,
attachments: [{port: "0", device: "0", uuid: "12345"},
{port: "1", device: "0", uuid: "67890"}]}] }
let(:defined_disks) { [double("disk", name: "vagrant_primary", size: "5GB", primary: true, type: :disk),
double("disk", name: "disk-0", size: "5GB", primary: false, type: :disk),
double("disk", name: "disk-1", size: "5GB", primary: false, type: :disk),
@ -66,6 +75,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
before do
allow(Vagrant::Util::Experimental).to receive(:feature_enabled?).and_return(true)
allow(driver).to receive(:show_vm_info).and_return(vm_info)
allow(driver).to receive(:storage_controllers).and_return(storage_controllers)
end
describe "#configure_disks" do
@ -85,7 +95,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
end
context "with over the disk limit for a given device" do
let(:defined_disks) { (1..31).each { |i| double("disk-#{i}") }.to_a }
let(:defined_disks) { (1..31).map { |i| double("disk-#{i}", type: :disk) }.to_a }
it "raises an exception if the disks defined exceed the limit for a SATA Controller" do
expect{subject.configure_disks(machine, defined_disks)}.
@ -281,8 +291,11 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
end
context "with an IDE controller" do
let(:vm_info) { {"IDE Controller-ImageUUID-0-0" => "00000aaaaa",
"IDE Controller-ImageUUID-0-1" => "11111bbbbb" } }
let(:storage_controllers) { [{name: "IDE Controller",
type: "PIIX4",
maxportcount: 2,
attachments: [{port: "0", device: "0", uuid: "12345"},
{port: "0", device: "1", uuid: "67890"}]}] }
it "determines the next available port and device to use" do
dsk_info = subject.get_next_port(machine, "IDE Controller")
@ -292,10 +305,13 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
end
context "with a full IDE controller" do
let(:vm_info) { {"IDE Controller-ImageUUID-0-0" => "00000aaaaa",
"IDE Controller-ImageUUID-0-1" => "11111bbbbb",
"IDE Controller-ImageUUID-1-0" => "22222ccccc",
"IDE Controller-ImageUUID-1-1" => "33333ddddd"} }
let(:storage_controllers) { [{name: "IDE Controller",
type: "PIIX4",
maxportcount: 2,
attachments: [{port: "0", device: "0", uuid: "11111"},
{port: "0", device: "1", uuid: "22222"},
{port: "1", device: "0", uuid: "33333"},
{port: "1", device: "1", uuid: "44444"}]}] }
it "raises an error" do
expect { subject.get_next_port(machine, "IDE Controller") }
@ -399,9 +415,19 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
describe ".handle_configure_dvd" do
let(:dvd_config) { double("dvd", file: "/tmp/untitled.iso", name: "dvd1") }
let(:storage_controllers) { [{name: "IDE Controller",
type: "PIIX4",
maxportcount: 2,
attachments: []}] }
let(:single_attachment) { [{name: "IDE Controller",
type: "PIIX4",
maxportcount: 2,
attachments: [port: "0", device: "0", uuid: "12345"]}] }
before do
allow(subject).to receive(:get_next_port).with(machine, "IDE Controller").and_return({device: "0", port: "0"})
allow(driver).to receive(:storage_controllers).and_return(storage_controllers)
end
it "attaches to the next available port and device" do
@ -411,20 +437,14 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
end
it "returns the UUID of the newly-attached dvd" do
allow(subject).to receive(:attachment).with(machine, "IDE Controller", "0", "0").and_return("12345")
expect(driver).to receive(:storage_controllers).and_return(
storage_controllers,
single_attachment
)
expect(driver).to receive(:attach_disk).with("0", "0", "/tmp/untitled.iso", "dvddrive")
disk_meta = subject.handle_configure_dvd(machine, dvd_config)
expect(disk_meta[:uuid]).to eq("12345")
end
context "when an ISO file is already attached" do
let(:vm_info) { {"IDE Controller-0-0" => "/tmp/untitled.iso" } }
it "skips the attachment" do
expect(driver).not_to receive(:attach_disk)
subject.handle_configure_dvd(machine, dvd_config)
end
end
end
end

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

@ -118,4 +118,29 @@ OUTPUT
subject.remove_disk(anything, anything, "IDE Controller")
end
end
describe "#storage_controllers" do
before do
allow(subject).to receive(:show_vm_info).and_return(
{"storagecontrollername0" => "SATA Controller",
"storagecontrollertype0" => "IntelAhci",
"storagecontrollermaxportcount0" => "30",
"SATA Controller-ImageUUID-0-0" => "12345",
"SATA Controller-ImageUUID-1-0" => "67890"}
)
end
it "returns the storage controllers" do
expect(subject.storage_controllers.first[:name]).to eq("SATA Controller")
expect(subject.storage_controllers.first[:type]).to eq("IntelAhci")
expect(subject.storage_controllers.first[:maxportcount]).to eq(30)
end
it "includes attachments for each storage controller" do
expect(subject.storage_controllers.first[:attachments])
.to include(port: "0", device: "0", uuid: "12345")
expect(subject.storage_controllers.first[:attachments])
.to include(port: "1", device: "0", uuid: "67890")
end
end
end