From 35b20b2367a1a8e1103baf02f976df1737a7c02d Mon Sep 17 00:00:00 2001 From: Nicolas Vazquez Date: Tue, 6 Apr 2021 07:58:52 -0300 Subject: [PATCH] vmware: Fix worker VM hardware version format (#4851) This PR fixes a small bug when explicitly setting VM hardware versions lower than version 10. Vmware expects the hardware version in format: vmx-DD where DD is a two-digit representation of the virtual hardware version. For hardware version lower than 10, CloudStack was not using to digits for the hardware version number, which ended up on an error while creating worker VMs. (vmx-8 for example instead of vmx-08) --- .../manager/VmwareStorageManagerImpl.java | 6 ++++-- .../vmware/mo/HypervisorHostHelper.java | 6 +++--- .../vmware/mo/VirtualMachineMO.java | 13 +++++++++++++ .../vmware/mo/VirtualMachineMOTest.java | 19 +++++++++++++++++++ 4 files changed, 39 insertions(+), 5 deletions(-) diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java index 3e2b5a02f6c..a98032c7fdd 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java @@ -647,7 +647,8 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager { } // 4 MB is the minimum requirement for VM memory in VMware - vmMo.cloneFromCurrentSnapshot(workerVmName, 0, 4, volumeDeviceInfo.second(), VmwareHelper.getDiskDeviceDatastore(volumeDeviceInfo.first()), null); + String vmxFormattedVirtualHardwareVersion = VirtualMachineMO.getVmxFormattedVirtualHardwareVersion(vmMo.getVirtualHardwareVersion()); + vmMo.cloneFromCurrentSnapshot(workerVmName, 0, 4, volumeDeviceInfo.second(), VmwareHelper.getDiskDeviceDatastore(volumeDeviceInfo.first()), vmxFormattedVirtualHardwareVersion); clonedVm = vmMo.getRunningHost().findVmOnHyperHost(workerVmName); if (clonedVm == null) { String msg = "Unable to create dummy VM to export volume. volume path: " + volumePath; @@ -965,7 +966,8 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager { if (clonedWorkerVMNeeded) { // 4 MB is the minimum requirement for VM memory in VMware - vmMo.cloneFromCurrentSnapshot(workerVmName, 0, 4, volumeDeviceInfo.second(), VmwareHelper.getDiskDeviceDatastore(volumeDeviceInfo.first()), null); + String vmxFormattedVirtualHardwareVersion = VirtualMachineMO.getVmxFormattedVirtualHardwareVersion(vmMo.getVirtualHardwareVersion()); + vmMo.cloneFromCurrentSnapshot(workerVmName, 0, 4, volumeDeviceInfo.second(), VmwareHelper.getDiskDeviceDatastore(volumeDeviceInfo.first()), vmxFormattedVirtualHardwareVersion); clonedVm = vmMo.getRunningHost().findVmOnHyperHost(workerVmName); if (clonedVm == null) { String msg = "Unable to create dummy VM to export volume. volume path: " + volumePath; diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java index 1b94ca881b4..3dd65e6c3a0 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java @@ -1637,7 +1637,7 @@ public class HypervisorHostHelper { return controllerSpec; } - public static VirtualMachineMO createWorkerVM(VmwareHypervisorHost hyperHost, DatastoreMO dsMo, String vmName, String hardwareVersion) throws Exception { + public static VirtualMachineMO createWorkerVM(VmwareHypervisorHost hyperHost, DatastoreMO dsMo, String vmName, String vmxFormattedHardwareVersion) throws Exception { // Allow worker VM to float within cluster so that we will have better chance to // create it successfully @@ -1651,8 +1651,8 @@ public class HypervisorHostHelper { VirtualMachineMO workingVM = null; VirtualMachineConfigSpec vmConfig = new VirtualMachineConfigSpec(); vmConfig.setName(vmName); - if (hardwareVersion != null){ - vmConfig.setVersion(("vmx-" + hardwareVersion)); + if (StringUtils.isNotBlank(vmxFormattedHardwareVersion)){ + vmConfig.setVersion(vmxFormattedHardwareVersion); } else { ClusterMO clusterMo = new ClusterMO(hyperHost.getContext(), hyperHost.getHyperHostCluster()); DatacenterMO dataCenterMo = new DatacenterMO(hyperHost.getContext(), hyperHost.getHyperHostDatacenter()); diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java index 5f61b31171c..74e6308d741 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java @@ -34,6 +34,7 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; +import com.cloud.utils.exception.CloudRuntimeException; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang.StringUtils; import org.apache.log4j.Logger; @@ -3292,6 +3293,18 @@ public class VirtualMachineMO extends BaseMO { return vhOption.getHwVersion(); } + /** + * Return a hardware version string in the format expected by Vmware + * Format: "vmx-DD" where DD represents the hardware version number + * @param virtualHardwareVersion numeric virtual hardware version + */ + public static String getVmxFormattedVirtualHardwareVersion(int virtualHardwareVersion) { + if (virtualHardwareVersion < 1) { + throw new CloudRuntimeException("Invalid hardware version: " + virtualHardwareVersion); + } + return String.format("vmx-%02d", virtualHardwareVersion); + } + public VirtualHardwareOption getVirtualHardwareOption() throws Exception { VirtualMachineConfigOption vmConfigOption = _context.getService().queryConfigOption(getEnvironmentBrowser(), null, null); return vmConfigOption.getHardwareOptions(); diff --git a/vmware-base/src/test/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMOTest.java b/vmware-base/src/test/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMOTest.java index 0e61b591bba..d1afb90d98e 100644 --- a/vmware-base/src/test/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMOTest.java +++ b/vmware-base/src/test/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMOTest.java @@ -19,6 +19,7 @@ package com.cloud.hypervisor.vmware.mo; import com.cloud.hypervisor.vmware.util.VmwareClient; import com.cloud.hypervisor.vmware.util.VmwareContext; +import com.cloud.utils.exception.CloudRuntimeException; import com.vmware.vim25.ManagedObjectReference; import com.vmware.vim25.VirtualDevice; import com.vmware.vim25.VirtualLsiLogicController; @@ -27,6 +28,7 @@ import com.vmware.vim25.VirtualSCSIController; import com.vmware.vim25.VirtualSCSISharing; import org.junit.After; import org.junit.AfterClass; +import org.junit.Assert; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; @@ -117,4 +119,21 @@ public class VirtualMachineMOTest { } } + + @Test + public void testGetVmxFormattedVirtualHardwareVersionOneDigit() { + String vmxHwVersion = VirtualMachineMO.getVmxFormattedVirtualHardwareVersion(8); + Assert.assertEquals("vmx-08", vmxHwVersion); + } + + @Test + public void testGetVmxFormattedVirtualHardwareVersionTwoDigits() { + String vmxHwVersion = VirtualMachineMO.getVmxFormattedVirtualHardwareVersion(11); + Assert.assertEquals("vmx-11", vmxHwVersion); + } + + @Test(expected = CloudRuntimeException.class) + public void testGetVmxFormattedVirtualHardwareVersionInvalid() { + VirtualMachineMO.getVmxFormattedVirtualHardwareVersion(-1); + } } \ No newline at end of file