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

Add support for SCSI controllers

Browse Source 
SCSI controllers are a lot like SATA controllers. This commit also
changes some controller detection logic to take boot priority into
account when selecting an appropriate controller.
This commit is contained in:
Jeff Bonhag 2020-07-02 12:50:12 -04:00
parent 21954c29af
commit 3cb01415e4
No known key found for this signature in database
GPG Key ID: 32966F3FB5AC1129
6 changed files with 183 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -277,13 +277,13 @@ module VagrantPlugins
def self.get_next_port(machine, controller)
dsk_info = {}
if controller.sata?
if controller.devices_per_port == 1
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"
elsif controller.ide?
elsif controller.devices_per_port == 2
# IDE Controllers have primary/secondary devices, so find the first port
# with an empty device
(0..(controller.maxportcount - 1)).each do |port|

39
plugins/providers/virtualbox/model/storage_controller.rb
View File

@ -4,11 +4,17 @@ module VagrantPlugins
# Represents a storage controller for VirtualBox. Storage controllers
# have a type, a name, and can have hard disks or optical drives attached.
class StorageController
SATA_CONTROLLER_TYPES = ["IntelAhci"].map(&:freeze).freeze
IDE_CONTROLLER_TYPES = ["PIIX4", "PIIX3", "ICH6"].map(&:freeze).freeze
SATA_CONTROLLER_TYPES = ["IntelAhci"].map(&:freeze).freeze
SCSI_CONTROLLER_TYPES = [ "LsiLogic", "BusLogic"].map(&:freeze).freeze
SATA_DEVICES_PER_PORT = 1.freeze
IDE_DEVICES_PER_PORT = 2.freeze
SATA_DEVICES_PER_PORT = 1.freeze
SCSI_DEVICES_PER_PORT = 1.freeze
IDE_BOOT_PRIORITY = 1.freeze
SATA_BOOT_PRIORITY = 2.freeze
SCSI_BOOT_PRIORITY = 3.freeze
# The name of the storage controller.
#
@ -39,6 +45,15 @@ module VagrantPlugins
# @return [Integer]
attr_reader :limit
# The boot priority of the storage controller. This does not seem to
# depend on the controller number returned by `showvminfo`.
# Experimentation has determined that VirtualBox will try to boot from
# the first controller it finds with a hard disk, in this order:
# IDE, SATA, SCSI
#
# @return [Integer]
attr_reader :boot_priority
# The list of disks/ISOs attached to each storage controller.
#
# @return [Array<Hash>]
@ -53,9 +68,15 @@ module VagrantPlugins
if IDE_CONTROLLER_TYPES.include?(@type)
@storage_bus = :ide
@devices_per_port = IDE_DEVICES_PER_PORT
@boot_priority = IDE_BOOT_PRIORITY
elsif SATA_CONTROLLER_TYPES.include?(@type)
@storage_bus = :sata
@devices_per_port = SATA_DEVICES_PER_PORT
@boot_priority = SATA_BOOT_PRIORITY
elsif SCSI_CONTROLLER_TYPES.include?(@type)
@storage_bus = :scsi
@devices_per_port = SCSI_DEVICES_PER_PORT
@boot_priority = SCSI_BOOT_PRIORITY
else
@storage_bus = :unknown
@devices_per_port = 1
@ -81,6 +102,13 @@ module VagrantPlugins
end
end
# Returns true if the storage controller has a supported type.
#
# @return [Boolean]
def supported?
[:ide, :sata, :scsi].include?(@storage_bus)
end
# Returns true if the storage controller is a IDE type controller.
#
# @return [Boolean]
@ -94,6 +122,13 @@ module VagrantPlugins
def sata?
@storage_bus == :sata
end
# Returns true if the storage controller is a SCSI type controller.
#
# @return [Boolean]
def scsi?
@storage_bus == :scsi
end
end
end
end

28
plugins/providers/virtualbox/model/storage_controller_array.rb
View File

@ -27,17 +27,12 @@ module VagrantPlugins
#
# @return [VagrantPlugins::ProviderVirtualBox::Model::StorageController]
def get_primary_controller
controller = nil
ide_controller = detect { |c| c.ide? }
if ide_controller && ide_controller.attachments.any? { |a| hdd?(a) }
controller = ide_controller
else
controller = detect { |c| c.sata? }
ordered = sort { |a, b| a.boot_priority <=> b.boot_priority }
controller = ordered.detect do |c|
c.supported? && c.attachments.any? { |a| hdd?(a) }
end
if !controller
supported_types = StorageController::SATA_CONTROLLER_TYPES + StorageController::IDE_CONTROLLER_TYPES
raise Vagrant::Errors::VirtualBoxDisksNoSupportedControllers,
supported_types: supported_types.join(" ,")
end
@ -62,15 +57,14 @@ module VagrantPlugins
attachment
end
# Find a suitable storage controller for attaching dvds. Will raise an
# exception if no suitable controller can be found.
# Returns the first supported storage controller for attaching dvds.
# Will raise an exception if no suitable controller can be found.
#
# @return [VagrantPlugins::ProviderVirtualBox::Model::StorageController]
def get_dvd_controller
controller = detect { |c| c.ide? } || detect { |c| c.sata? }
ordered = sort { |a, b| a.boot_priority <=> b.boot_priority }
controller = ordered.detect { |c| c.supported? }
if !controller
supported_types = StorageController::SATA_CONTROLLER_TYPES + StorageController::IDE_CONTROLLER_TYPES
raise Vagrant::Errors::VirtualBoxDisksNoSupportedControllers,
supported_types: supported_types.join(" ,")
end
@ -92,6 +86,14 @@ module VagrantPlugins
VagrantPlugins::ProviderVirtualBox::Cap::ValidateDiskExt.validate_disk_ext(nil, ext)
end
end
# Returns a list of all the supported controller types.
#
# @return [Array<String>]
def supported_types
StorageController::SATA_CONTROLLER_TYPES + StorageController::IDE_CONTROLLER_TYPES +
StorageController::SCSI_CONTROLLER_TYPES
end
end
end
end

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

@ -27,7 +27,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
let(:storage_controllers) { double("storage controllers") }
let(:controller) { double("controller", name: "controller", limit: 30, sata?: true, maxportcount: 30) }
let(:controller) { double("controller", name: "controller", maxportcount: 30, devices_per_port: 1, limit: 30) }
let(:attachments) { [{port: "0", device: "0", uuid: "12345"},
{port: "1", device: "0", uuid: "67890"}]}
@ -120,8 +120,8 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
# hashicorp/bionic64
context "with more than one storage controller" do
let(:controller1) { double("controller1", name: "IDE Controller", limit: 4) }
let(:controller2) { double("controller2", name: "SATA Controller", limit: 30) }
let(:controller1) { double("controller1", name: "IDE Controller", maxportcount: 2, devices_per_port: 2, limit: 4) }
let(:controller2) { double("controller2", name: "SATA Controller", maxportcount: 30, devices_per_port: 1, limit: 30) }
before do
allow(storage_controllers).to receive(:size).and_return(2)
@ -372,16 +372,43 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
expect(dsk_info[:device]).to eq("0")
end
context "with empty IDE controller" do
let(:empty_controller) { double("controller", ide?: true, sata?: false, attachments: [], maxportcount: 2, devices_per_port: 2) }
context "with IDE controller" do
let(:controller) {
double("controller", name: "IDE", maxportcount: 2, devices_per_port: 2, limit: 4)
}
let(:attachments) { [] }
it "attaches to port 0, device 0" do
dsk_info = subject.get_next_port(machine, empty_controller)
dsk_info = subject.get_next_port(machine, controller)
expect(dsk_info[:port]).to eq("0")
expect(dsk_info[:device]).to eq("0")
end
context "with 1 device" do
let(:attachments) { [{port:"0", device: "0"}] }
it "attaches to the next device on that port" do
dsk_info = subject.get_next_port(machine, controller)
expect(dsk_info[:port]).to eq("0")
expect(dsk_info[:device]).to eq("1")
end
end
end
context "with SCSI controller" do
let(:controller) {
double("controller", name: "SCSI", maxportcount: 16, devices_per_port: 1, limit: 16)
}
let(:attachments) { [] }
it "determines the next available port and device to use" do
dsk_info = subject.get_next_port(machine, controller)
expect(dsk_info[:port]).to eq("0")
expect(dsk_info[:device]).to eq("0")
end
end
end
describe "#resize_disk" do

79
test/unit/plugins/providers/virtualbox/model/storage_controller_array_test.rb
View File

@ -3,18 +3,18 @@ require File.expand_path("../../base", __FILE__)
describe VagrantPlugins::ProviderVirtualBox::Model::StorageControllerArray do
include_context "unit"
let(:ide_controller) { double("ide_controller", name: "IDE Controller", ide?: true, sata?: false) }
let(:sata_controller) { double("sata_controller", name: "SATA Controller", sata?: true) }
let(:controller1) { double("controller1", name: "IDE Controller", supported?: true, boot_priority: 1) }
let(:controller2) { double("controller2", name: "SATA Controller", supported?: true, boot_priority: 2) }
let(:primary_disk) { {location: "/tmp/primary.vdi"} }
before do
subject.replace([ide_controller, sata_controller])
subject.replace([controller1, controller2])
end
describe "#get_controller" do
it "gets a controller by name" do
expect(subject.get_controller("IDE Controller")).to eq(ide_controller)
expect(subject.get_controller("IDE Controller")).to eq(controller1)
end
it "raises an exception if a matching storage controller can't be found" do
@ -26,34 +26,27 @@ describe VagrantPlugins::ProviderVirtualBox::Model::StorageControllerArray do
describe "#get_primary_controller" do
context "with a single supported controller" do
before do
subject.replace([ide_controller])
allow(ide_controller).to receive(:attachments).and_return([primary_disk])
subject.replace([controller1])
allow(controller1).to receive(:attachments).and_return([primary_disk])
end
it "returns the controller" do
expect(subject.get_primary_controller).to eq(ide_controller)
expect(subject.get_primary_controller).to eq(controller1)
end
end
context "with multiple controllers" do
before do
allow(ide_controller).to receive(:attachments).and_return([])
allow(sata_controller).to receive(:attachments).and_return([])
allow(controller1).to receive(:attachments).and_return([])
allow(controller2).to receive(:attachments).and_return([primary_disk])
end
it "returns the SATA controller by default" do
expect(subject.get_primary_controller).to eq(sata_controller)
it "returns the first supported controller with a disk attached" do
expect(subject.get_primary_controller).to eq(controller2)
end
it "returns the IDE controller if it has a hdd attached" do
allow(ide_controller).to receive(:attachments).and_return([primary_disk])
allow(subject).to receive(:hdd?).with(primary_disk).and_return(true)
expect(subject.get_primary_controller).to eq(ide_controller)
end
it "raises an error if the machine doesn't have a SATA or an IDE controller" do
subject.replace([])
it "raises an error if the primary disk is attached to an unsupported controller" do
allow(controller2).to receive(:supported?).and_return(false)
expect { subject.get_primary_controller }.
to raise_error(Vagrant::Errors::VirtualBoxDisksNoSupportedControllers)
@ -82,17 +75,57 @@ describe VagrantPlugins::ProviderVirtualBox::Model::StorageControllerArray do
let(:attachment) { {location: "/tmp/primary.vdi"} }
before do
allow(subject).to receive(:get_primary_controller).and_return(sata_controller)
allow(subject).to receive(:get_primary_controller).and_return(controller2)
end
it "returns the first attachment on the primary controller" do
allow(sata_controller).to receive(:get_attachment).with(port: "0", device: "0").and_return(attachment)
allow(controller2).to receive(:get_attachment).with(port: "0", device: "0").and_return(attachment)
expect(subject.get_primary_attachment).to be(attachment)
end
it "raises an exception if no attachment exists at port 0, device 0" do
allow(sata_controller).to receive(:get_attachment).with(port: "0", device: "0").and_return(nil)
allow(controller2).to receive(:get_attachment).with(port: "0", device: "0").and_return(nil)
expect { subject.get_primary_attachment }.to raise_error(Vagrant::Errors::VirtualBoxDisksPrimaryNotFound)
end
end
describe "#get_dvd_controller" do
context "with one controller" do
let(:controller) { double("controller", supported?: true) }
before do
subject.replace([controller])
end
it "returns the controller" do
expect(subject.get_dvd_controller).to be(controller)
end
it "raises an exception if the controller is unsupported" do
allow(controller).to receive(:supported?).and_return(false)
expect { subject.get_dvd_controller }.to raise_error(Vagrant::Errors::VirtualBoxDisksNoSupportedControllers)
end
end
context "with multiple controllers" do
let(:controller1) { double("controller", supported?: true, boot_priority: 2) }
let(:controller2) { double("controller", supported?: true, boot_priority: 1) }
before do
subject.replace([controller1, controller2])
end
it "returns the first supported controller" do
expect(subject.get_dvd_controller).to be(controller2)
end
it "raises an exception if no controllers are supported" do
allow(controller1).to receive(:supported?).and_return(false)
allow(controller2).to receive(:supported?).and_return(false)
expect { subject.get_dvd_controller }.to raise_error(Vagrant::Errors::VirtualBoxDisksNoSupportedControllers)
end
end
end
end

39
test/unit/plugins/providers/virtualbox/model/storage_controller_test.rb
View File

@ -19,6 +19,10 @@ describe VagrantPlugins::ProviderVirtualBox::Model::StorageController do
it "calculates the maximum number of attachments" do
expect(subject.limit).to eq(30)
end
it "sets the boot priority" do
expect(subject.boot_priority).to eq(2)
end
end
context "with IDE controller type" do
@ -32,6 +36,27 @@ describe VagrantPlugins::ProviderVirtualBox::Model::StorageController do
it "calculates the maximum number of attachments" do
expect(subject.limit).to eq(4)
end
it "sets the boot priority" do
expect(subject.boot_priority).to eq(1)
end
end
context "with SCSI controller type" do
let(:type) { "LsiLogic" }
let(:maxportcount) { 16 }
it "recognizes an SCSI controller" do
expect(subject.scsi?).to be(true)
end
it "calculates the maximum number of attachments" do
expect(subject.limit).to eq(16)
end
it "sets the boot priority" do
expect(subject.boot_priority).to eq(3)
end
end
context "with some other type" do
@ -43,4 +68,18 @@ describe VagrantPlugins::ProviderVirtualBox::Model::StorageController do
end
end
end
describe "#supported?" do
it "returns true if the controller type is supported" do
expect(subject.supported?).to be(true)
end
context "with unsupported type" do
let(:type) { "foo" }
it "returns false" do
expect(subject.supported?).to be(false)
end
end
end
end