From ed771058220f3895ec1615801a5528f296e1979e Mon Sep 17 00:00:00 2001 From: Dan Allegood Date: Thu, 4 Aug 2016 12:29:02 -0700 Subject: [PATCH] Improved SCSI controller handling (#7908) Govmomi tries to use the 7th slot in a scsi controller, which is not allowed. This patch will appropriately select the slot to attach a disk to as well as determine if a scsi controller is full. --- .../resource_vsphere_virtual_machine.go | 56 +++++++++++++++ .../resource_vsphere_virtual_machine_test.go | 70 +++++++++++++++++++ 2 files changed, 126 insertions(+) diff --git a/builtin/providers/vsphere/resource_vsphere_virtual_machine.go b/builtin/providers/vsphere/resource_vsphere_virtual_machine.go index 444e10877..d8e907212 100644 --- a/builtin/providers/vsphere/resource_vsphere_virtual_machine.go +++ b/builtin/providers/vsphere/resource_vsphere_virtual_machine.go @@ -1198,6 +1198,12 @@ func addHardDisk(vm *object.VirtualMachine, size, iops int64, diskType string, d } if err != nil || controller == nil { + // Check if max number of scsi controller are already used + diskControllers := getSCSIControllers(devices) + if len(diskControllers) >= 4 { + return fmt.Errorf("[ERROR] Maximum number of SCSI controllers created") + } + log.Printf("[DEBUG] Couldn't find a %v controller. Creating one..", controller_type) var c types.BaseVirtualDevice @@ -1268,6 +1274,14 @@ func addHardDisk(vm *object.VirtualMachine, size, iops int64, diskType string, d log.Printf("[DEBUG] addHardDisk - diskPath: %v", diskPath) disk := devices.CreateDisk(controller, datastore.Reference(), diskPath) + if strings.Contains(controller_type, "scsi") { + unitNumber, err := getNextUnitNumber(devices, controller) + if err != nil { + return err + } + *disk.UnitNumber = unitNumber + } + existing := devices.SelectByBackingInfo(disk.Backing) log.Printf("[DEBUG] disk: %#v\n", disk) @@ -1300,6 +1314,44 @@ func addHardDisk(vm *object.VirtualMachine, size, iops int64, diskType string, d } } +func getSCSIControllers(vmDevices object.VirtualDeviceList) []*types.VirtualController { + // get virtual scsi controllers of all supported types + var scsiControllers []*types.VirtualController + for _, device := range vmDevices { + devType := vmDevices.Type(device) + switch devType { + case "scsi", "lsilogic", "buslogic", "pvscsi", "lsilogic-sas": + if c, ok := device.(types.BaseVirtualController); ok { + scsiControllers = append(scsiControllers, c.GetVirtualController()) + } + } + } + return scsiControllers +} + +func getNextUnitNumber(devices object.VirtualDeviceList, c types.BaseVirtualController) (int32, error) { + key := c.GetVirtualController().Key + + var unitNumbers [16]bool + unitNumbers[7] = true + + for _, device := range devices { + d := device.GetVirtualDevice() + + if d.ControllerKey == key { + if d.UnitNumber != nil { + unitNumbers[*d.UnitNumber] = true + } + } + } + for i, taken := range unitNumbers { + if !taken { + return int32(i), nil + } + } + return -1, fmt.Errorf("[ERROR] getNextUnitNumber - controller is full") +} + // addCdrom adds a new virtual cdrom drive to the VirtualMachine and attaches an image (ISO) to it from a datastore path. func addCdrom(vm *object.VirtualMachine, datastore, path string) error { devices, err := vm.Device(context.TODO()) @@ -1902,6 +1954,10 @@ func (vm *virtualMachine) setupVirtualMachine(c *govmomi.Client) error { err = addHardDisk(newVM, vm.hardDisks[i].size, vm.hardDisks[i].iops, vm.hardDisks[i].initType, datastore, diskPath, vm.hardDisks[i].controller) if err != nil { + err2 := addHardDisk(newVM, vm.hardDisks[i].size, vm.hardDisks[i].iops, vm.hardDisks[i].initType, datastore, diskPath, vm.hardDisks[i].controller) + if err2 != nil { + return err2 + } return err } } diff --git a/builtin/providers/vsphere/resource_vsphere_virtual_machine_test.go b/builtin/providers/vsphere/resource_vsphere_virtual_machine_test.go index 56f2db226..5a1dafe24 100644 --- a/builtin/providers/vsphere/resource_vsphere_virtual_machine_test.go +++ b/builtin/providers/vsphere/resource_vsphere_virtual_machine_test.go @@ -328,6 +328,76 @@ func TestAccVSphereVirtualMachine_client_debug(t *testing.T) { }) } +const testAccCheckVSphereVirtualMachineConfig_diskSCSICapacity = ` +resource "vsphere_virtual_machine" "scsiCapacity" { + name = "terraform-test" +` + testAccTemplateBasicBody + ` + disk { + size = 1 + controller_type = "scsi-paravirtual" + name = "one" + } + disk { + size = 1 + controller_type = "scsi-paravirtual" + name = "two" + } + disk { + size = 1 + controller_type = "scsi-paravirtual" + name = "three" + } + disk { + size = 1 + controller_type = "scsi-paravirtual" + name = "four" + } + disk { + size = 1 + controller_type = "scsi-paravirtual" + name = "five" + } + disk { + size = 1 + controller_type = "scsi-paravirtual" + name = "six" + } + disk { + size = 1 + controller_type = "scsi-paravirtual" + name = "seven" + } +} +` + +func TestAccVSphereVirtualMachine_diskSCSICapacity(t *testing.T) { + var vm virtualMachine + basic_vars := setupTemplateBasicBodyVars() + config := basic_vars.testSprintfTemplateBody(testAccCheckVSphereVirtualMachineConfig_diskSCSICapacity) + + vmName := "vsphere_virtual_machine.scsiCapacity" + + test_exists, test_name, test_cpu, test_uuid, test_mem, test_num_disk, test_num_of_nic, test_nic_label := + TestFuncData{vm: vm, label: basic_vars.label, vmName: vmName, numDisks: "8"}.testCheckFuncBasic() + + log.Printf("[DEBUG] template= %s", testAccCheckVSphereVirtualMachineConfig_diskSCSICapacity) + log.Printf("[DEBUG] template config= %s", config) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckVSphereVirtualMachineDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: config, + Check: resource.ComposeTestCheckFunc( + test_exists, test_name, test_cpu, test_uuid, test_mem, test_num_disk, test_num_of_nic, test_nic_label, + ), + }, + }, + }) +} + const testAccCheckVSphereVirtualMachineConfig_initType = ` resource "vsphere_virtual_machine" "thin" { name = "terraform-test"