Merge pull request vmware#2882 from Syuparn/issue-2881

fix: generate negative device key in AssignController

object/virtual_device_list.go

@@ -69,13 +69,7 @@ func EthernetCardTypes() VirtualDeviceList {
 		&types.VirtualSriovEthernetCard{},
 	}).Select(func(device types.BaseVirtualDevice) bool {
 		c := device.(types.BaseVirtualEthernetCard).GetVirtualEthernetCard()
-
-		key := rand.Int31() * -1
-		if key == 0 {
-			key = -1
-		}
-
-		c.GetVirtualDevice().Key = key
+		c.GetVirtualDevice().Key = VirtualDeviceList{}.newRandomKey()
 		return true
 	})
 }
@@ -463,10 +457,22 @@ func (l VirtualDeviceList) AssignController(device types.BaseVirtualDevice, c ty
 	d.UnitNumber = new(int32)
 	*d.UnitNumber = l.newUnitNumber(c)
 	if d.Key == 0 {
-		d.Key = int32(rand.Uint32()) * -1
+		d.Key = l.newRandomKey()
 	}
 }
 
+// newRandomKey returns a random negative device key.
+// The generated key can be used for devices you want to add so that it does not collide with existing ones.
+func (l VirtualDeviceList) newRandomKey() int32 {
+	// NOTE: rand.Uint32 cannot be used here because conversion from uint32 to int32 may change the sign
+	key := rand.Int31() * -1
+	if key == 0 {
+		return -1
+	}
+
+	return key
+}
+
 // CreateDisk creates a new VirtualDisk device which can be added to a VM.
 func (l VirtualDeviceList) CreateDisk(c types.BaseVirtualController, ds types.ManagedObjectReference, name string) *types.VirtualDisk {
 	// If name is not specified, one will be chosen for you.

object/virtual_device_list_test.go

@@ -723,6 +723,29 @@ func TestPickController(t *testing.T) {
 	}
 }
 
+func TestAssignController(t *testing.T) {
+	scsi, _ := devices.CreateSCSIController("scsi")
+	disk := &types.VirtualDisk{
+		CapacityInBytes: 512 * 1024,
+		VirtualDevice: types.VirtualDevice{
+			Backing: new(types.VirtualDiskFlatVer2BackingInfo), // Leave fields empty to test defaults
+		},
+	}
+
+	devices := VirtualDeviceList([]types.BaseVirtualDevice{scsi})
+	devices.AssignController(disk, scsi.(*types.VirtualLsiLogicController))
+
+	if disk.ControllerKey != scsi.GetVirtualDevice().Key {
+		t.Errorf("expected controller key: %d, got: %d\n", scsi.GetVirtualDevice().Key, disk.ControllerKey)
+	}
+
+	// if disk does not have device key, AssignController gives a random negative key to the disk
+	// so that it will not collide with existing device keys
+	if disk.Key >= 0 {
+		t.Errorf("device key %d should be negative", disk.Key)
+	}
+}
+
 func TestCreateSCSIController(t *testing.T) {
 	for _, l := range []VirtualDeviceList{SCSIControllerTypes(), devices} {
 		_, err := l.CreateSCSIController("enoent")