CLOUDSTACK-10107: For VMware VMs add devices without unit number (#2288)

When VMs are deployed or nics are plugged, using a static unit number
may cause device configuration errors. This fixes a previous limitation
that more than 7 nics/networks could not be added to a VM.

Per the API docs, `unitNumber` need not be set:
https://www.vmware.com/support/developer/converter-sdk/conv55_apireference/vim.vm.device.VirtualDevice.html

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java

@@ -1085,18 +1085,15 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
                 ManagedObjectReference dvsMor = dataCenterMo.getDvSwitchMor(networkInfo.first());
                 dvSwitchUuid = dataCenterMo.getDvSwitchUuid(dvsMor);
                 s_logger.info("Preparing NIC device on dvSwitch : " + dvSwitchUuid);
-                nic =
+                nic = VmwareHelper.prepareDvNicDevice(vmMo, networkInfo.first(), nicDeviceType, networkInfo.second(), dvSwitchUuid,
-                        VmwareHelper.prepareDvNicDevice(vmMo, networkInfo.first(), nicDeviceType, networkInfo.second(), dvSwitchUuid, nicTo.getMac(), deviceNumber,
+                        nicTo.getMac(), deviceNumber + 1, true, true);
-                                deviceNumber + 1, true, true);
             } else {
                 s_logger.info("Preparing NIC device on network " + networkInfo.second());
-                nic =
+                nic = VmwareHelper.prepareNicDevice(vmMo, networkInfo.first(), nicDeviceType, networkInfo.second(),
-                        VmwareHelper.prepareNicDevice(vmMo, networkInfo.first(), nicDeviceType, networkInfo.second(), nicTo.getMac(), deviceNumber, deviceNumber + 1, true,
+                        nicTo.getMac(), deviceNumber + 1, true, true);
-                                true);
             }
 
             VirtualMachineConfigSpec vmConfigSpec = new VirtualMachineConfigSpec();
-            //VirtualDeviceConfigSpec[] deviceConfigSpecArray = new VirtualDeviceConfigSpec[1];
             VirtualDeviceConfigSpec deviceConfigSpec = new VirtualDeviceConfigSpec();
             deviceConfigSpec.setDevice(nic);
             deviceConfigSpec.setOperation(VirtualDeviceConfigSpecOperation.ADD);
@@ -1754,7 +1751,6 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
             int i = 0;
             int ideUnitNumber = 0;
             int scsiUnitNumber = 0;
-            int nicUnitNumber = 0;
             int ideControllerKey = vmMo.getIDEDeviceControllerKey();
             int scsiControllerKey = vmMo.getGenericScsiDeviceControllerKeyNoException();
             int controllerKey;
@@ -2016,21 +2012,21 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
                         ManagedObjectReference dvsMor = dataCenterMo.getDvSwitchMor(networkInfo.first());
                         dvSwitchUuid = dataCenterMo.getDvSwitchUuid(dvsMor);
                         s_logger.info("Preparing NIC device on dvSwitch : " + dvSwitchUuid);
-                        nic =
+                        nic = VmwareHelper.prepareDvNicDevice(vmMo, networkInfo.first(), nicDeviceType, networkInfo.second(), dvSwitchUuid,
-                                VmwareHelper.prepareDvNicDevice(vmMo, networkInfo.first(), nicDeviceType, networkInfo.second(), dvSwitchUuid, nicTo.getMac(), nicUnitNumber++,
+                                nicTo.getMac(), i + 1, true, true);
-                                        i + 1, true, true);
                         if (nicTo.getUuid() != null) {
                             nicUuidToDvSwitchUuid.put(nicTo.getUuid(), dvSwitchUuid);
                         }
                     } else {
                         s_logger.info("Preparing NIC device on network " + networkInfo.second());
-                        nic =
+                        nic = VmwareHelper.prepareNicDevice(vmMo, networkInfo.first(), nicDeviceType, networkInfo.second(),
-                                VmwareHelper.prepareNicDevice(vmMo, networkInfo.first(), nicDeviceType, networkInfo.second(), nicTo.getMac(), nicUnitNumber++, i + 1, true, true);
+                                nicTo.getMac(), i + 1, true, true);
                     }
                 }
                 else{
                     //if NSX API VERSION >= 4.2, connect to br-int (nsx.network), do not create portgroup else previous behaviour
-                    nic = VmwareHelper.prepareNicOpaque(vmMo, nicDeviceType, networkInfo.second(), nicTo.getMac(), nicUnitNumber++, i + 1, true, true);
+                    nic = VmwareHelper.prepareNicOpaque(vmMo, nicDeviceType, networkInfo.second(),
+                            nicTo.getMac(), i + 1, true, true);
                 }
 
                 deviceConfigSpecArray[i] = new VirtualDeviceConfigSpec();

test/integration/smoke/test_nic.py

@@ -32,6 +32,8 @@ import signal
 import sys
 import logging
 import time
+import threading
+import Queue
 
 
 class TestNic(cloudstackTestCase):
@@ -316,6 +318,141 @@ class TestNic(cloudstackTestCase):
 
         self.assertTrue(found, "Nic not successfully added with specified mac address")
 
+
+    @attr(tags = ["devcloud", "advanced", "advancedns", "smoke"], required_hardware="true")
+    def test_03_nic_multiple_vmware(self):
+        """Test to adding multiple nics to a VMware VM and restarting VM
+
+           Refer to CLOUDSTACK-10107 for details, in this test we add 8 nics to
+           a VM and stop, start it to show that VMware VMs are not limited to
+           having up to 7 nics.
+        """
+
+        if self.hypervisor.lower() != "vmware":
+            self.skipTest("Skipping test applicable for VMware")
+
+        network_offering = NetworkOffering.create(
+            self.apiclient,
+            self.services["nw_off_isolated_persistent"]
+        )
+        self.cleanup.insert(0, network_offering)
+        network_offering.update(self.apiclient, state='Enabled')
+
+        offering = dict(self.services["network"])
+        offering["networkoffering"] = network_offering.id
+
+        networks = []
+
+        def createNetwork(idx):
+            offering["name"] = "Test Network%s" % idx
+            network = Network.create(
+                self.apiclient,
+                offering,
+                self.account.name,
+                self.account.domainid,
+                zoneid=self.services["network"]["zoneid"]
+            )
+            networks.append(network)
+            self.cleanup.insert(0, network)
+
+
+        class NetworkMaker(threading.Thread):
+            def __init__(self, queue=None, createNetwork=None):
+                threading.Thread.__init__(self)
+                self.queue = queue
+                self.createNetwork = createNetwork
+
+            def run(self):
+                while True:
+                    idx = self.queue.get()
+                    if idx is not None:
+                        self.createNetwork(idx)
+                    self.queue.task_done()
+
+        # Start multiple networks
+        tsize = 8
+        queue = Queue.Queue()
+        for _ in range(tsize):
+            worker = NetworkMaker(queue, createNetwork)
+            worker.setDaemon(True)
+            worker.start()
+
+        for idx in range(tsize):
+            queue.put(idx)
+        queue.join()
+
+        # Deploy a VM
+        vm = VirtualMachine.create(
+            self.apiclient,
+            self.services["small"],
+            accountid=self.account.name,
+            domainid=self.account.domainid,
+            serviceofferingid=self.service_offering.id,
+            networkids=[networks[0].id],
+            mode=self.zone.networktype
+        )
+        self.cleanup.insert(0, vm)
+
+        # Add nics to networks
+        for network in networks[1:]:
+            response = vm.add_nic(self.apiclient, network.id)
+            found = False
+            for nic in response.nic:
+                if nic.networkid == network.id:
+                    found = True
+                    break
+            self.assertTrue(found, "Nic not successfully added for the specific network")
+
+        # Stop VM
+        vm.stop(self.apiclient, forced=True)
+
+        vms = VirtualMachine.list(
+            self.apiclient,
+            id=vm.id
+        )
+        self.assertEqual(
+                validateList(vms)[0],
+                PASS,
+                "vms list validation failed")
+
+        vm_response = vms[0]
+        self.assertEqual(
+            vm_response.state,
+            "Stopped",
+            "Verify the VM is stopped"
+        )
+
+        # Start VM
+        vm.start(self.apiclient)
+
+        vms = VirtualMachine.list(
+            self.apiclient,
+            id=vm.id
+        )
+        self.assertEqual(
+                validateList(vms)[0],
+                PASS,
+                "vms list validation failed")
+
+        vm_response = vms[0]
+        self.assertEqual(
+            vm_response.state,
+            "Running",
+            "Verify the VM is running"
+        )
+
+        self.assertTrue(len(vm_response.nic) == len(networks), "Number of nics on VM not 8")
+
+        # Validate nics exist on each of the network
+        for network in networks:
+            found = False
+            for nic in vm_response.nic:
+                if nic.networkid == network.id:
+                    found = True
+                    break
+            self.assertTrue(found, "Nic not found for the specific network")
+
+
     def tearDown(self):
         try:
             for obj in self.cleanup:

vmware-base/src/com/cloud/hypervisor/vmware/util/VmwareHelper.java

@@ -89,7 +89,7 @@ public class VmwareHelper {
     }
 
     public static VirtualDevice prepareNicOpaque(VirtualMachineMO vmMo, VirtualEthernetCardType deviceType, String portGroupName,
-            String macAddress, int deviceNumber, int contextNumber, boolean conntected, boolean connectOnStart) throws Exception {
+            String macAddress, int contextNumber, boolean connected, boolean connectOnStart) throws Exception {
 
         assert(vmMo.getRunningHost().hasOpaqueNSXNetwork());
 
@@ -123,18 +123,17 @@ public class VmwareHelper {
 
         VirtualDeviceConnectInfo connectInfo = new VirtualDeviceConnectInfo();
         connectInfo.setAllowGuestControl(true);
-        connectInfo.setConnected(conntected);
+        connectInfo.setConnected(connected);
         connectInfo.setStartConnected(connectOnStart);
         nic.setAddressType("Manual");
         nic.setConnectable(connectInfo);
         nic.setMacAddress(macAddress);
-        nic.setUnitNumber(deviceNumber);
         nic.setKey(-contextNumber);
         return nic;
     }
 
     public static VirtualDevice prepareNicDevice(VirtualMachineMO vmMo, ManagedObjectReference morNetwork, VirtualEthernetCardType deviceType, String portGroupName,
-            String macAddress, int deviceNumber, int contextNumber, boolean conntected, boolean connectOnStart) throws Exception {
+            String macAddress, int contextNumber, boolean connected, boolean connectOnStart) throws Exception {
 
         VirtualEthernetCard nic;
         switch (deviceType) {
@@ -166,18 +165,17 @@ public class VmwareHelper {
 
         VirtualDeviceConnectInfo connectInfo = new VirtualDeviceConnectInfo();
         connectInfo.setAllowGuestControl(true);
-        connectInfo.setConnected(conntected);
+        connectInfo.setConnected(connected);
         connectInfo.setStartConnected(connectOnStart);
         nic.setAddressType("Manual");
         nic.setConnectable(connectInfo);
         nic.setMacAddress(macAddress);
-        nic.setUnitNumber(deviceNumber);
         nic.setKey(-contextNumber);
         return nic;
     }
 
     public static VirtualDevice prepareDvNicDevice(VirtualMachineMO vmMo, ManagedObjectReference morNetwork, VirtualEthernetCardType deviceType, String dvPortGroupName,
-            String dvSwitchUuid, String macAddress, int deviceNumber, int contextNumber, boolean conntected, boolean connectOnStart) throws Exception {
+            String dvSwitchUuid, String macAddress, int contextNumber, boolean connected, boolean connectOnStart) throws Exception {
 
         VirtualEthernetCard nic;
         switch (deviceType) {
@@ -210,16 +208,13 @@ public class VmwareHelper {
         dvPortConnection.setPortgroupKey(morNetwork.getValue());
         dvPortBacking.setPort(dvPortConnection);
         nic.setBacking(dvPortBacking);
-        nic.setKey(30);
 
         connectInfo.setAllowGuestControl(true);
-        connectInfo.setConnected(conntected);
+        connectInfo.setConnected(connected);
         connectInfo.setStartConnected(connectOnStart);
         nic.setAddressType("Manual");
         nic.setConnectable(connectInfo);
         nic.setMacAddress(macAddress);
-
-        nic.setUnitNumber(deviceNumber);
         nic.setKey(-contextNumber);
         return nic;
     }