Address Copilot reviews

Author: winterhazel

engine/schema/src/main/java/org/apache/cloudstack/storage/DiskControllerMappingVO.java

@@ -197,6 +197,11 @@ public boolean equals(Object obj) {
         return controllerReference.equals(that.getControllerReference());
     }
 
+    @Override
+    public int hashCode() {
+        return controllerReference.hashCode();
+    }
+
     @Override
     public String toString() {
         return ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, "name", "controllerReference",

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

@@ -2041,7 +2041,7 @@ protected void validateRequiredVirtualHardwareVersionForNewDiskControllers(Virtu
         if (!unsupportedDiskControllers.isEmpty()) {
             String names = unsupportedDiskControllers.stream().map(DiskControllerMappingVO::getName).collect(Collectors.joining(", "));
             String requiredVersions = unsupportedDiskControllers.stream().map(DiskControllerMappingVO::getMinHardwareVersion).collect(Collectors.joining(", "));
-            logger.debug("Virtual machine [{}] does not support disk controllers [{}], as its virtual hardware version is [{}] but the controllers require, respectfully, versions [{}].",
+            logger.debug("Virtual machine [{}] does not support disk controllers [{}], as its virtual hardware version is [{}] but the controllers require, respectively, versions [{}].",
                     vmMo, names, hardwareVersion, requiredVersions);
             throw new CloudRuntimeException(String.format("Disk controllers [%s] are not supported.", names));
         }

plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java

@@ -2104,7 +2104,7 @@ private Answer attachVolume(Command cmd, DiskTO disk, boolean isAttach, boolean
             if (isAttach) {
                 // Let's first find which disk controller should be used for the volume being attached.
                 //
-                // `controllerInfo` can not be null here. It is always defined when creating the `AttachComand` in
+                // `controllerInfo` can not be null here. It is always defined when creating the `AttachCommand` in
                 // `com.cloud.storage.VolumeApiServiceImpl#sendAttachVolumeCommand`.
                 //
                 // If `VmDetailConstants.ROOT_DISK_CONTROLLER` or `VmDetailConstants.DATA_DISK_CONTROLLER` are not present

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

@@ -77,6 +77,7 @@
 import com.vmware.vim25.VirtualSCSISharing;
 import com.vmware.vim25.VmwareDistributedVirtualSwitchPvlanSpec;
 import com.vmware.vim25.VmwareDistributedVirtualSwitchVlanIdSpec;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
 import org.apache.cloudstack.storage.DiskControllerMappingVO;
 import org.apache.cloudstack.utils.volume.VirtualMachineDiskInfo;
 import org.apache.cloudstack.vm.UnmanagedInstanceTO;
@@ -146,6 +147,7 @@ public class VmwareHelper {
     private static List<DiskControllerMappingVO> supportedDiskControllers;
 
     public static void setSupportedDiskControllers(List<DiskControllerMappingVO> controllers) {
+        controllers.sort(Comparator.comparing(DiskControllerMappingVO::getControllerReference));
         supportedDiskControllers = controllers;
     }
 
@@ -1172,7 +1174,7 @@ public static void addDiskControllersToVmConfigSpec(VirtualMachineConfigSpec vmC
                 continue;
             }
             for (int bus = 0; bus < diskController.getMaxControllerCount(); bus++) {
-                VirtualController controller = (VirtualController) controllerClass.newInstance();
+                VirtualController controller = (VirtualController) controllerClass.getDeclaredConstructor().newInstance();
                 controller.setBusNumber(bus);
                 controller.setKey(currentKey);
                 currentKey--;
@@ -1269,6 +1271,12 @@ public static void configureDiskControllerMappingsInVmwareBaseModule(List<DiskCo
             try {
                 if (!DiskControllerType.osdefault.toString().equals(mapping.getName())) {
                     Class.forName(mapping.getControllerReference());
+                    if (ObjectUtils.anyNull(mapping.getMaxDeviceCount(), mapping.getMaxControllerCount(), mapping.getVmdkAdapterType())) {
+                        LOGGER.debug("Disk controller mapping with name [{}] and controller reference [{}] is invalid " +
+                                        "because it has null required values (max_device_count, max_controller_count, and/or vmdk_adapter_type); ignoring it.",
+                                mapping.getName(), mapping.getControllerReference());
+                        continue;
+                    }
                 }
                 LOGGER.debug("Adding disk controller mapping with name [{}] and controller reference [{}] to the list of available disk controllers.",
                         mapping.getName(), mapping.getControllerReference());

vmware-base/src/test/java/com/cloud/hypervisor/vmware/util/VmwareHelperTest.java

@@ -338,8 +338,8 @@ public void addDiskControllersToVmConfigSpecTestMaximumAmmountOfControllersIsAdd
 
         VmwareHelper.addDiskControllersToVmConfigSpec(virtualMachineConfigSpecMock, requiredControllers, false);
 
-        int expectedControllerAmmount = nvmeMapping.getMaxControllerCount() + sataMapping.getMaxControllerCount();
-        Assert.assertEquals(expectedControllerAmmount, virtualMachineConfigSpecMock.getDeviceChange().size());
+        int expectedControllerAmount = nvmeMapping.getMaxControllerCount() + sataMapping.getMaxControllerCount();
+        Assert.assertEquals(expectedControllerAmount, virtualMachineConfigSpecMock.getDeviceChange().size());
 
         Set<Integer> usedKeys = new HashSet<>();
         Map<String, Set<Integer>> usedBusNumbers = new HashMap<>();
@@ -351,7 +351,7 @@ public void addDiskControllersToVmConfigSpecTestMaximumAmmountOfControllersIsAdd
             usedKeys.add(controller.getKey());
             usedBusNumbers.get(controller.getClass().getName()).add(controller.getBusNumber());
         }
-        Assert.assertEquals(expectedControllerAmmount, usedKeys.size());
+        Assert.assertEquals(expectedControllerAmount, usedKeys.size());
         Assert.assertEquals((int) nvmeMapping.getMaxControllerCount(), usedBusNumbers.get(nvmeMapping.getControllerReference()).size());
         Assert.assertEquals((int) sataMapping.getMaxControllerCount(), usedBusNumbers.get(sataMapping.getControllerReference()).size());
     }