Repository: vmware/govmomi
Pull request #3588 (2024-10-09 23:40:36)

fix: update controller's device list in AssignController
## Description

This PR updates the `AssignController` function to add the assigned device to the controller's device list. This ensures that [PickController()](https://github.com/vmware/govmomi/blob/main/object/virtual_device_list.go#L454-L479) function returns the appropriate controller with available slots, as it's based on the current number of devices attached to the controller.

Closes: N/A

## How Has This Been Tested?

- Updated the existing `TestAssignController` to validate the controller's device list.
- Deployed a VM with multiple devices, invoking `AssignController` during a single VM reconfiguration task. Verified that the devices were properly assigned to the controller without exceeding the maximum allowed slots:

```console
$ govc device.ls -vm vm-iso-3-cdroms cdrom-\*
cdrom-3002  VirtualCdrom  ISO [sharedVmfs-0] contentlib-...
cdrom-3000  VirtualCdrom  ISO [sharedVmfs-0] contentlib-...
cdrom-3001  VirtualCdrom  ISO [sharedVmfs-0] contentlib-...

$ govc device.info -json -vm vm-iso-3-cdroms ide-\* | jq '.devices[] | {name, device}'
{
  "name": "ide-200",
  "device": [
    3000,
    3001
  ]
}
{
  "name": "ide-201",
  "device": [
    3002
  ]
}
```

## Guidelines

Please read and follow the `CONTRIBUTION` [guidelines] of this project.

[guidelines]: https://github.com/vmware/govmomi/blob/main/CONTRIBUTING.md


dougm: vcsim may change the Device.Key, in which case we need to change the Controller.Device reference too:
```diff
diff --git a/simulator/virtual_machine.go b/simulator/virtual_machine.go
index a596798d..1bda935c 100644
--- a/simulator/virtual_machine.go
+++ b/simulator/virtual_machine.go
@@ -1279,6 +1279,7 @@ func (vm *VirtualMachine) configureDevice(
 	d := device.GetVirtualDevice()
 	var controller types.BaseVirtualController
 
+	key := d.Key
 	if d.Key <= 0 {
 		// Keys can't be negative; Key 0 is reserved
 		d.Key = devices.NewKey()
@@ -1293,6 +1294,19 @@ func (vm *VirtualMachine) configureDevice(
 		d.Key++
 	}
 
+	// Update device controller's key reference
+	if key != d.Key {
+		if p := devices.FindByKey(d.ControllerKey); p != nil {
+			c := p.(types.BaseVirtualController).GetVirtualController()
+			for i := range c.Device {
+				if c.Device[i] == key {
+					c.Device[i] = d.Key
+					break
+				}
+			}
+		}
+	}
+
 	label := devices.Name(device)
 	summary := label
 	dc := ctx.Map.getEntityDatacenter(ctx.Map.Get(*vm.Parent).(mo.Entity))
```

Patch:
diff --git a/object/virtual_device_list.go b/object/virtual_device_list.go
index 295171af..4d4335ea 100644
--- a/object/virtual_device_list.go
+++ b/object/virtual_device_list.go
@@ -1,5 +1,5 @@
 /*
-Copyright (c) 2015-2017 VMware, Inc. All Rights Reserved.
+Copyright (c) 2015-2024 VMware, Inc. All Rights Reserved.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -545,6 +545,8 @@ func (l VirtualDeviceList) AssignController(device types.BaseVirtualDevice, c ty
 	if d.Key == 0 {
 		d.Key = l.newRandomKey()
 	}
+
+	c.GetVirtualController().Device = append(c.GetVirtualController().Device, d.Key)
 }
 
 // newRandomKey returns a random negative device key.
diff --git a/simulator/virtual_machine.go b/simulator/virtual_machine.go
index a596798d..470b59c8 100644
--- a/simulator/virtual_machine.go
+++ b/simulator/virtual_machine.go
@@ -1279,12 +1279,26 @@ func (vm *VirtualMachine) configureDevice(
 	d := device.GetVirtualDevice()
 	var controller types.BaseVirtualController
 
+	key := d.Key
 	if d.Key <= 0 {
 		// Keys can't be negative; Key 0 is reserved
 		d.Key = devices.NewKey()
 		d.Key *= -1
 	}
 
+	// Update device controller's key reference
+	if key != d.Key {
+		if device := devices.FindByKey(d.ControllerKey); device != nil {
+			c := device.(types.BaseVirtualController).GetVirtualController()
+			for i := range c.Device {
+				if c.Device[i] == key {
+					c.Device[i] = d.Key
+					break
+				}
+			}
+		}
+	}
+
 	// Choose a unique key
 	for {
 		if devices.FindByKey(d.Key) == nil {


Test patch:
diff --git a/object/virtual_device_list_test.go b/object/virtual_device_list_test.go
index 466fb02d..9cfc3273 100644
--- a/object/virtual_device_list_test.go
+++ b/object/virtual_device_list_test.go
@@ -1,5 +1,5 @@
 /*
-Copyright (c) 2015 VMware, Inc. All Rights Reserved.
+Copyright (c) 2015-2024 VMware, Inc. All Rights Reserved.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -744,6 +744,12 @@ func TestAssignController(t *testing.T) {
 	if disk.Key >= 0 {
 		t.Errorf("device key %d should be negative", disk.Key)
 	}
+
+	// AssignController should add the disk to the controller's device list.
+	cd := scsi.(*types.VirtualLsiLogicController).Device[0]
+	if cd != disk.Key {
+		t.Errorf("expected controller device key: %d, got: %d\n", disk.Key, cd)
+	}
 }
 
 func TestCreateSCSIController(t *testing.T) {