Create #get_controller method in driver

This makes it easier to check if the required controller can be found,
and automatically raise an error if it is not.

Add a #storage_bus method to the StorageController class as a shorthand
way to check the general storage controller type.
This commit is contained in:
Jeff Bonhag 2020-06-02 14:04:09 -04:00
parent 3a515cc7d6
commit 958023dbb9
No known key found for this signature in database
GPG Key ID: 32966F3FB5AC1129
6 changed files with 77 additions and 56 deletions

View File

@ -19,20 +19,16 @@ module VagrantPlugins
disks_defined = defined_disks.select { |d| d.type == :disk }
if disks_defined.any?
controller = machine.provider.driver.storage_controllers.detect { |c| c.sata_controller? }
if controller.nil?
raise Vagrant::Errors::VirtualBoxDisksControllerNotFound, disk_type: ':disk', controller_type: 'SATA'
elsif disks_defined.size > controller.maxportcount
controller = machine.provider.driver.get_controller('SATA')
if disks_defined.size > controller.maxportcount
raise Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit
end
end
dvds_defined = defined_disks.select { |d| d.type == :dvd }
if dvds_defined.any?
controller = machine.provider.driver.storage_controllers.detect { |c| c.ide_controller? }
if controller.nil?
raise Vagrant::Errors::VirtualBoxDisksControllerNotFound, disk_type: ':dvd', controller_type: 'IDE'
elsif dvds_defined.size > controller.maxportcount
controller = machine.provider.driver.get_controller('IDE')
if disks_defined.size > controller.maxportcount
raise Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit
end
end
@ -71,7 +67,7 @@ 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.
controller = machine.provider.driver.storage_controllers.detect { |c| c.sata_controller? }
controller = machine.provider.driver.get_controller('SATA')
primary = controller.attachments.detect { |a| a[:port] == "0" && a[:device] == "0" }
primary_uuid = primary[:uuid]
@ -107,7 +103,7 @@ module VagrantPlugins
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")
controller = machine.provider.driver.storage_controllers.detect { |c| c.sata_controller? }
controller = machine.provider.driver.get_controller('SATA')
dsk_info = get_next_port(machine, controller.name)
machine.provider.driver.attach_disk(dsk_info[:port],
dsk_info[:device],
@ -128,13 +124,13 @@ module VagrantPlugins
# @param [Config::Disk] dvd - the current disk to configure
# @return [Hash] - dvd_metadata
def self.handle_configure_dvd(machine, dvd)
controller = machine.provider.driver.storage_controllers.detect { |c| c.ide_controller? }
controller = machine.provider.driver.get_controller('IDE')
disk_info = get_next_port(machine, controller.name)
machine.provider.driver.attach_disk(disk_info[:port], disk_info[:device], dvd.file, "dvddrive")
# Refresh the controller information
controller = machine.provider.driver.storage_controllers.detect { |c| c.ide_controller? }
controller = machine.provider.driver.get_controller('IDE')
attachment = controller.attachments.detect { |a| a[:port] == disk_info[:port] &&
a[:device] == disk_info[:device] }
@ -211,7 +207,7 @@ module VagrantPlugins
# TODO: definitely need an error for this
dsk_info = {}
if controller.sata_controller?
if controller.storage_bus == 'SATA'
used_ports = controller.attachments.map { |a| a[:port].to_i }
next_available_port = ((0..(controller.maxportcount-1)).to_a - used_ports).first

View File

@ -937,7 +937,7 @@ module VagrantPlugins
# Helper method to get a list of storage controllers added to the
# current VM
#
# @return [Array<StorageController>]
# @return [Array<VagrantPlugins::ProviderVirtualBox::Model::StorageController>]
def storage_controllers
vm_info = show_vm_info
count = vm_info.count { |key, value| key.match(/^storagecontrollername/) }
@ -960,6 +960,22 @@ module VagrantPlugins
end
end
# Get the controller that uses the specified storage bus.
#
# A VirtualBoxDisksControllerNotFound error is raised if a compatible
# storage controller cannot be found.
#
# @param [String] storage_bus - for example, 'IDE' or 'SATA'
# @return [VagrantPlugins::ProviderVirtualBox::Model::StorageController] controller
def get_controller(storage_bus)
controller = storage_controllers.detect { |c| c.storage_bus == storage_bus }
if controller.nil?
raise Vagrant::Errors::VirtualBoxDisksControllerNotFound, storage_bus: storage_bus
end
controller
end
protected
def valid_ip_address?(ip)

View File

@ -18,6 +18,12 @@ module VagrantPlugins
# @return [String]
attr_reader :type
# The storage bus associated with the storage controller, which can be
# inferred from its specific type.
#
# @return [String]
attr_reader :storage_bus
# The maximum number of avilable ports for the storage controller. For
# SATA controllers, this indicates the number of disks that can be
# attached. For IDE controllers, this indicates that n*2 disks can be
@ -34,19 +40,20 @@ module VagrantPlugins
def initialize(name, type, maxportcount, attachments)
@name = name
@type = type
if SATA_CONTROLLER_TYPES.include?(@type)
@storage_bus = 'SATA'
elsif IDE_CONTROLLER_TYPES.include?(@type)
@storage_bus = 'IDE'
else
@storage_bus = 'Unknown'
end
@maxportcount = maxportcount.to_i
attachments ||= []
@attachments = attachments
end
def sata_controller?
SATA_CONTROLLER_TYPES.include?(@type)
end
def ide_controller?
IDE_CONTROLLER_TYPES.include?(@type)
end
end
end
end

View File

@ -1679,9 +1679,10 @@ en:
Vagrant. VirtualBox 4.2.16+ fixes this problem. Please upgrade
VirtualBox.
virtualbox_disks_controller_not_found: |-
The current disk configuration requires a controller of type '%{controller_type}'.
Please add the proper controller to your guest using provider-specific
customizations, or remove the '%{disk_type}' disk configurations.
The current disk configuration requires a controller with storage bus
'%{storage_bus}', but no such controller was found. Please add the
proper controller to your guest using provider-specific
customizations.
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.

View File

@ -31,17 +31,8 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
"SATA Controller-ImageUUID-0-0" => "12345",
"SATA Controller-ImageUUID-1-0" => "67890"} }
let(:sata_controller) { double("sata_controller",
name: "SATA Controller",
maxportcount: 30,
sata_controller?: true,
ide_controller?: false) }
let(:ide_controller) { double("ide_controller",
name: "IDE Controller",
maxportcount: 2,
ide_controller?: true,
sata_controller?: false) }
let(:sata_controller) { double("controller", name: "SATA Controller", storage_bus: "SATA", maxportcount: 30) }
let(:ide_controller) { double("controller", name: "IDE Controller", storage_bus: "IDE", maxportcount: 2) }
let(:attachments) { [{port: "0", device: "0", uuid: "12345"},
{port: "1", device: "0", uuid: "67890"}]}
@ -84,8 +75,12 @@ 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(sata_controller).to receive(:attachments).and_return(attachments)
allow(driver).to receive(:storage_controllers).and_return([sata_controller])
allow(driver).to receive(:get_controller).with("IDE").and_return(ide_controller)
allow(driver).to receive(:get_controller).with("SATA").and_return(sata_controller)
allow(driver).to receive(:storage_controllers).and_return([ide_controller, sata_controller])
end
describe "#configure_disks" do
@ -113,9 +108,10 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
end
end
context "missing SATA controller" do
context "no SATA controller" do
before do
allow(driver).to receive(:storage_controllers).and_return([])
allow(driver).to receive(:get_controller).with("SATA").
and_raise(Vagrant::Errors::VirtualBoxDisksControllerNotFound, storage_bus: "SATA")
end
it "raises an error" do
@ -125,10 +121,6 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
end
context "with dvd type" do
before do
allow(driver).to receive(:storage_controllers).and_return([ide_controller])
end
let(:defined_disks) { [double("dvd", type: :dvd)] }
let(:dvd_data) { {uuid: "1234", name: "dvd"} }
@ -138,9 +130,10 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
subject.configure_disks(machine, defined_disks)
end
context "missing IDE controller" do
context "no IDE controller" do
before do
allow(driver).to receive(:storage_controllers).and_return([])
allow(driver).to receive(:get_controller).with("IDE").
and_raise(Vagrant::Errors::VirtualBoxDisksControllerNotFound, storage_bus: "IDE")
end
it "raises an error" do
@ -332,7 +325,6 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
before do
allow(ide_controller).to receive(:attachments).and_return(attachments)
allow(driver).to receive(:storage_controllers).and_return([ide_controller])
end
it "determines the next available port and device to use" do
@ -456,7 +448,6 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
allow(ide_controller).to receive(:attachments).and_return(
[port: "0", device: "0", uuid: "12345"]
)
allow(driver).to receive(:storage_controllers).and_return([ide_controller])
end
it "returns the UUID of the newly-attached dvd" do

View File

@ -10,19 +10,29 @@ describe VagrantPlugins::ProviderVirtualBox::Model::StorageController do
subject { described_class.new(name, type, maxportcount, attachments) }
describe "#sata_controller?" do
let(:type) { "IntelAhci" }
describe "#initialize" do
context "with SATA controller type" do
let(:type) { "IntelAhci" }
it "is true for a SATA type" do
expect(subject.sata_controller?).to be(true)
it "is recognizes a SATA controller" do
expect(subject.storage_bus).to eq('SATA')
end
end
end
describe "#ide_controller?" do
let(:type) { "PIIX4" }
context "with IDE controller type" do
let(:type) { "PIIX4" }
it "is true for an IDE type" do
expect(subject.ide_controller?).to be(true)
it "recognizes an IDE controller" do
expect(subject.storage_bus).to eq('IDE')
end
end
context "with some other type" do
let(:type) { "foo" }
it "is unknown" do
expect(subject.storage_bus).to eq('Unknown')
end
end
end
end