From e68bc29b71f27f6fe3feaba6977ec10a9612d393 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Fri, 27 Sep 2024 11:54:12 +0530 Subject: [PATCH 1/3] Add support for functionality in hypervisor class --- .../java/com/cloud/hypervisor/Hypervisor.java | 39 ++++++++++++++++--- .../user/template/RegisterTemplateCmd.java | 4 +- .../java/com/cloud/vm/UserVmManagerImpl.java | 10 +---- .../com/cloud/vm/UserVmManagerImplTest.java | 2 +- 4 files changed, 38 insertions(+), 17 deletions(-) diff --git a/api/src/main/java/com/cloud/hypervisor/Hypervisor.java b/api/src/main/java/com/cloud/hypervisor/Hypervisor.java index e1108b34a837..e12d469d2e0c 100644 --- a/api/src/main/java/com/cloud/hypervisor/Hypervisor.java +++ b/api/src/main/java/com/cloud/hypervisor/Hypervisor.java @@ -23,34 +23,52 @@ import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.Set; +import java.util.EnumSet; public class Hypervisor { public static class HypervisorType { + public static enum Functionality { + DirectDownloadTemplate, + RootDiskSizeOverride; + } + private static final Map hypervisorTypeMap = new LinkedHashMap<>(); public static final HypervisorType None = new HypervisorType("None"); //for storage hosts - public static final HypervisorType XenServer = new HypervisorType("XenServer", ImageFormat.VHD); - public static final HypervisorType KVM = new HypervisorType("KVM", ImageFormat.QCOW2); - public static final HypervisorType VMware = new HypervisorType("VMware", ImageFormat.OVA); + public static final HypervisorType XenServer = new HypervisorType("XenServer", ImageFormat.VHD, + EnumSet.of(Functionality.RootDiskSizeOverride)); + public static final HypervisorType KVM = new HypervisorType("KVM", ImageFormat.QCOW2, + EnumSet.of(Functionality.DirectDownloadTemplate, Functionality.RootDiskSizeOverride)); + public static final HypervisorType VMware = new HypervisorType("VMware", ImageFormat.OVA, + EnumSet.of(Functionality.RootDiskSizeOverride)); public static final HypervisorType Hyperv = new HypervisorType("Hyperv"); public static final HypervisorType VirtualBox = new HypervisorType("VirtualBox"); public static final HypervisorType Parralels = new HypervisorType("Parralels"); public static final HypervisorType BareMetal = new HypervisorType("BareMetal"); - public static final HypervisorType Simulator = new HypervisorType("Simulator"); + public static final HypervisorType Simulator = new HypervisorType("Simulator", null, + EnumSet.of(Functionality.RootDiskSizeOverride)); public static final HypervisorType Ovm = new HypervisorType("Ovm", ImageFormat.RAW); public static final HypervisorType Ovm3 = new HypervisorType("Ovm3", ImageFormat.RAW); public static final HypervisorType LXC = new HypervisorType("LXC"); - public static final HypervisorType Custom = new HypervisorType("Custom"); + public static final HypervisorType Custom = new HypervisorType("Custom", null, + EnumSet.of(Functionality.RootDiskSizeOverride)); public static final HypervisorType Any = new HypervisorType("Any"); /*If you don't care about the hypervisor type*/ private final String name; private final ImageFormat imageFormat; + private final Set supportedFunctionalities; public HypervisorType(String name) { - this(name, null); + this(name, null, EnumSet.noneOf(Functionality.class)); } public HypervisorType(String name, ImageFormat imageFormat) { + this(name, imageFormat, EnumSet.noneOf(Functionality.class)); + } + + public HypervisorType(String name, ImageFormat imageFormat, Set supportedFunctionalities) { this.name = name; this.imageFormat = imageFormat; + this.supportedFunctionalities = supportedFunctionalities; if (name.equals("Parralels")){ // typo in the original code hypervisorTypeMap.put("parallels", this); } else { @@ -102,6 +120,15 @@ public String name() { return name; } + /** + * Make this method to be part of the properties of the hypervisor type itself. + * + * @return true if the hypervisor plugin support the specified functionality + */ + public boolean isFunctionalitySupported(Functionality functionality) { + return supportedFunctionalities.contains(functionality); + } + @Override public int hashCode() { return Objects.hash(name); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/template/RegisterTemplateCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/template/RegisterTemplateCmd.java index a7826dedcd04..1f968b869b99 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/template/RegisterTemplateCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/template/RegisterTemplateCmd.java @@ -362,7 +362,9 @@ protected void validateParameters() { "Parameter zoneids cannot combine all zones (-1) option with other zones"); String customHypervisor = HypervisorGuru.HypervisorCustomDisplayName.value(); - if (isDirectDownload() && !(getHypervisor().equalsIgnoreCase(Hypervisor.HypervisorType.KVM.toString()) + if (isDirectDownload() && + !(Hypervisor.HypervisorType.getType(getHypervisor()) + .isFunctionalitySupported(Hypervisor.HypervisorType.Functionality.DirectDownloadTemplate) || getHypervisor().equalsIgnoreCase(customHypervisor))) { throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Parameter directdownload " + "is only allowed for KVM or %s templates", customHypervisor)); diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 219feeef5a91..193fac340ff0 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -686,14 +686,6 @@ public void setKubernetesServiceHelpers(final List kube HypervisorType.Simulator )); - protected static final List ROOT_DISK_SIZE_OVERRIDE_SUPPORTING_HYPERVISORS = Arrays.asList( - HypervisorType.KVM, - HypervisorType.XenServer, - HypervisorType.VMware, - HypervisorType.Simulator, - HypervisorType.Custom - ); - private static final List HYPERVISORS_THAT_CAN_DO_STORAGE_MIGRATION_ON_NON_USER_VMS = Arrays.asList(HypervisorType.KVM, HypervisorType.VMware); @Override @@ -4563,7 +4555,7 @@ protected long configureCustomRootDiskSize(Map customParameters, * @throws InvalidParameterValueException if the hypervisor does not support rootdisksize override */ protected void verifyIfHypervisorSupportsRootdiskSizeOverride(HypervisorType hypervisorType) { - if (!ROOT_DISK_SIZE_OVERRIDE_SUPPORTING_HYPERVISORS.contains(hypervisorType)) { + if (!hypervisorType.isFunctionalitySupported(HypervisorType.Functionality.RootDiskSizeOverride)) { throw new InvalidParameterValueException("Hypervisor " + hypervisorType + " does not support rootdisksize override"); } } diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 8316c57d67dd..d49dcd0f00c9 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -635,7 +635,7 @@ public void verifyIfHypervisorSupportRootdiskSizeOverrideTest() { int expectedExceptionCounter = hypervisorTypeArray.length - 5; for(int i = 0; i < hypervisorTypeArray.length; i++) { - if (UserVmManagerImpl.ROOT_DISK_SIZE_OVERRIDE_SUPPORTING_HYPERVISORS.contains(hypervisorTypeArray[i])) { + if (hypervisorTypeArray[i].isFunctionalitySupported(Hypervisor.HypervisorType.Functionality.RootDiskSizeOverride)) { userVmManagerImpl.verifyIfHypervisorSupportsRootdiskSizeOverride(hypervisorTypeArray[i]); } else { try { From d1982db8329d8afecc7e631ef05c28716405972e Mon Sep 17 00:00:00 2001 From: Vishesh Date: Mon, 30 Sep 2024 11:33:12 +0530 Subject: [PATCH 2/3] Address comments --- .../java/com/cloud/hypervisor/Hypervisor.java | 32 ++++++++++++------- .../java/com/cloud/vm/UserVmManagerImpl.java | 28 ++++++++-------- 2 files changed, 34 insertions(+), 26 deletions(-) diff --git a/api/src/main/java/com/cloud/hypervisor/Hypervisor.java b/api/src/main/java/com/cloud/hypervisor/Hypervisor.java index e12d469d2e0c..27ffef1c3708 100644 --- a/api/src/main/java/com/cloud/hypervisor/Hypervisor.java +++ b/api/src/main/java/com/cloud/hypervisor/Hypervisor.java @@ -20,38 +20,40 @@ import org.apache.commons.lang3.StringUtils; import java.util.LinkedHashMap; +import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.EnumSet; +import java.util.stream.Collectors; + +import static com.cloud.hypervisor.Hypervisor.HypervisorType.Functionality.DirectDownloadTemplate; +import static com.cloud.hypervisor.Hypervisor.HypervisorType.Functionality.RootDiskSizeOverride; +import static com.cloud.hypervisor.Hypervisor.HypervisorType.Functionality.VmStorageMigration; public class Hypervisor { public static class HypervisorType { - public static enum Functionality { + public enum Functionality { DirectDownloadTemplate, - RootDiskSizeOverride; + RootDiskSizeOverride, + VmStorageMigration } private static final Map hypervisorTypeMap = new LinkedHashMap<>(); public static final HypervisorType None = new HypervisorType("None"); //for storage hosts - public static final HypervisorType XenServer = new HypervisorType("XenServer", ImageFormat.VHD, - EnumSet.of(Functionality.RootDiskSizeOverride)); - public static final HypervisorType KVM = new HypervisorType("KVM", ImageFormat.QCOW2, - EnumSet.of(Functionality.DirectDownloadTemplate, Functionality.RootDiskSizeOverride)); - public static final HypervisorType VMware = new HypervisorType("VMware", ImageFormat.OVA, - EnumSet.of(Functionality.RootDiskSizeOverride)); + public static final HypervisorType XenServer = new HypervisorType("XenServer", ImageFormat.VHD, EnumSet.of(RootDiskSizeOverride, VmStorageMigration)); + public static final HypervisorType KVM = new HypervisorType("KVM", ImageFormat.QCOW2, EnumSet.of(DirectDownloadTemplate, RootDiskSizeOverride, VmStorageMigration)); + public static final HypervisorType VMware = new HypervisorType("VMware", ImageFormat.OVA, EnumSet.of(RootDiskSizeOverride, VmStorageMigration)); public static final HypervisorType Hyperv = new HypervisorType("Hyperv"); public static final HypervisorType VirtualBox = new HypervisorType("VirtualBox"); public static final HypervisorType Parralels = new HypervisorType("Parralels"); public static final HypervisorType BareMetal = new HypervisorType("BareMetal"); - public static final HypervisorType Simulator = new HypervisorType("Simulator", null, - EnumSet.of(Functionality.RootDiskSizeOverride)); + public static final HypervisorType Simulator = new HypervisorType("Simulator", null, EnumSet.of(RootDiskSizeOverride, VmStorageMigration)); public static final HypervisorType Ovm = new HypervisorType("Ovm", ImageFormat.RAW); public static final HypervisorType Ovm3 = new HypervisorType("Ovm3", ImageFormat.RAW); public static final HypervisorType LXC = new HypervisorType("LXC"); - public static final HypervisorType Custom = new HypervisorType("Custom", null, - EnumSet.of(Functionality.RootDiskSizeOverride)); + public static final HypervisorType Custom = new HypervisorType("Custom", null, EnumSet.of(RootDiskSizeOverride)); public static final HypervisorType Any = new HypervisorType("Any"); /*If you don't care about the hypervisor type*/ private final String name; private final ImageFormat imageFormat; @@ -99,6 +101,12 @@ public static HypervisorType valueOf(String name) { return hypervisorType; } + public static List getListOfHypervisorsSupportingFunctionality(Functionality functionality) { + return hypervisorTypeMap.values().stream() + .filter(hypervisor -> hypervisor.supportedFunctionalities.contains(functionality)) + .collect(Collectors.toList()); + } + /** * Returns the display name of a hypervisor type in case the custom hypervisor is used, * using the 'hypervisor.custom.display.name' setting. Otherwise, returns hypervisor name diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 193fac340ff0..3a4e7c531d93 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -679,15 +679,6 @@ public void setKubernetesServiceHelpers(final List kube private static final ConfigKey VmDestroyForcestop = new ConfigKey("Advanced", Boolean.class, "vm.destroy.forcestop", "false", "On destroy, force-stop takes this value ", true); - public static final List VM_STORAGE_MIGRATION_SUPPORTING_HYPERVISORS = new ArrayList<>(Arrays.asList( - HypervisorType.KVM, - HypervisorType.VMware, - HypervisorType.XenServer, - HypervisorType.Simulator - )); - - private static final List HYPERVISORS_THAT_CAN_DO_STORAGE_MIGRATION_ON_NON_USER_VMS = Arrays.asList(HypervisorType.KVM, HypervisorType.VMware); - @Override public UserVmVO getVirtualMachine(long vmId) { return _vmDao.findById(vmId); @@ -6598,9 +6589,15 @@ private VMInstanceVO preVmStorageMigrationCheck(Long vmId) { } HypervisorType hypervisorType = vm.getHypervisorType(); - if (vm.getType() != VirtualMachine.Type.User && !HYPERVISORS_THAT_CAN_DO_STORAGE_MIGRATION_ON_NON_USER_VMS.contains(hypervisorType)) { - throw new InvalidParameterValueException(String.format("Unable to migrate storage of non-user VMs for hypervisor [%s]. Operation only supported for the following" - + " hypervisors: [%s].", hypervisorType, HYPERVISORS_THAT_CAN_DO_STORAGE_MIGRATION_ON_NON_USER_VMS)); + if (vm.getType() != VirtualMachine.Type.User && + (!hypervisorType.isFunctionalitySupported(HypervisorType.Functionality.VmStorageMigration) + || hypervisorType.equals(HypervisorType.XenServer))) { + + List supportedHypervisors = HypervisorType.getListOfHypervisorsSupportingFunctionality(HypervisorType.Functionality.VmStorageMigration) + .stream().filter(hypervisor -> !hypervisor.equals(HypervisorType.XenServer)).collect(Collectors.toList()); + throw new InvalidParameterValueException(String.format( + "Unable to migrate storage of non-user VMs for hypervisor [%s]. Operation only supported for the following hypervisors: [%s].", + hypervisorType, supportedHypervisors)); } List vols = _volsDao.findByInstance(vm.getId()); @@ -7310,8 +7307,11 @@ public VirtualMachine migrateVirtualMachineWithVolume(Long vmId, Host destinatio throw new InvalidParameterValueException("Live Migration of GPU enabled VM is not supported"); } - if (!VM_STORAGE_MIGRATION_SUPPORTING_HYPERVISORS.contains(vm.getHypervisorType())) { - throw new InvalidParameterValueException(String.format("Unsupported hypervisor: %s for VM migration, we support XenServer/VMware/KVM only", vm.getHypervisorType())); + if (!vm.getHypervisorType().isFunctionalitySupported(HypervisorType.Functionality.VmStorageMigration)) { + throw new InvalidParameterValueException( + String.format("Unsupported hypervisor: %s for VM migration, we support [%s] only", + vm.getHypervisorType(), + HypervisorType.getListOfHypervisorsSupportingFunctionality(HypervisorType.Functionality.VmStorageMigration))); } if (_vmSnapshotDao.findByVm(vmId).size() > 0) { From de41a6f94d29db6e833f535624ecc440d1d2ee5b Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 3 Oct 2024 18:14:39 +0530 Subject: [PATCH 3/3] address comments --- .../java/com/cloud/vm/UserVmManagerImpl.java | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 3a4e7c531d93..04b64dd80a36 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -16,6 +16,7 @@ // under the License. package com.cloud.vm; +import static com.cloud.hypervisor.Hypervisor.HypervisorType.Functionality; import static com.cloud.storage.Volume.IOPS_LIMIT; import static com.cloud.utils.NumbersUtil.toHumanReadableSize; import static org.apache.cloudstack.api.ApiConstants.MAX_IOPS; @@ -4546,7 +4547,7 @@ protected long configureCustomRootDiskSize(Map customParameters, * @throws InvalidParameterValueException if the hypervisor does not support rootdisksize override */ protected void verifyIfHypervisorSupportsRootdiskSizeOverride(HypervisorType hypervisorType) { - if (!hypervisorType.isFunctionalitySupported(HypervisorType.Functionality.RootDiskSizeOverride)) { + if (!hypervisorType.isFunctionalitySupported(Functionality.RootDiskSizeOverride)) { throw new InvalidParameterValueException("Hypervisor " + hypervisorType + " does not support rootdisksize override"); } } @@ -6589,15 +6590,12 @@ private VMInstanceVO preVmStorageMigrationCheck(Long vmId) { } HypervisorType hypervisorType = vm.getHypervisorType(); - if (vm.getType() != VirtualMachine.Type.User && - (!hypervisorType.isFunctionalitySupported(HypervisorType.Functionality.VmStorageMigration) - || hypervisorType.equals(HypervisorType.XenServer))) { - - List supportedHypervisors = HypervisorType.getListOfHypervisorsSupportingFunctionality(HypervisorType.Functionality.VmStorageMigration) - .stream().filter(hypervisor -> !hypervisor.equals(HypervisorType.XenServer)).collect(Collectors.toList()); + List supportedHypervisorsForNonUserVMStorageMigration = HypervisorType.getListOfHypervisorsSupportingFunctionality(Functionality.VmStorageMigration) + .stream().filter(hypervisor -> !hypervisor.equals(HypervisorType.XenServer)).collect(Collectors.toList()); + if (vm.getType() != VirtualMachine.Type.User && !supportedHypervisorsForNonUserVMStorageMigration.contains(hypervisorType)) { throw new InvalidParameterValueException(String.format( "Unable to migrate storage of non-user VMs for hypervisor [%s]. Operation only supported for the following hypervisors: [%s].", - hypervisorType, supportedHypervisors)); + hypervisorType, supportedHypervisorsForNonUserVMStorageMigration)); } List vols = _volsDao.findByInstance(vm.getId()); @@ -7307,11 +7305,11 @@ public VirtualMachine migrateVirtualMachineWithVolume(Long vmId, Host destinatio throw new InvalidParameterValueException("Live Migration of GPU enabled VM is not supported"); } - if (!vm.getHypervisorType().isFunctionalitySupported(HypervisorType.Functionality.VmStorageMigration)) { + if (!vm.getHypervisorType().isFunctionalitySupported(Functionality.VmStorageMigration)) { throw new InvalidParameterValueException( String.format("Unsupported hypervisor: %s for VM migration, we support [%s] only", vm.getHypervisorType(), - HypervisorType.getListOfHypervisorsSupportingFunctionality(HypervisorType.Functionality.VmStorageMigration))); + HypervisorType.getListOfHypervisorsSupportingFunctionality(Functionality.VmStorageMigration))); } if (_vmSnapshotDao.findByVm(vmId).size() > 0) {