From d82cd5569ade345733fff8fdf52ed967d189246a Mon Sep 17 00:00:00 2001 From: davidjumani Date: Thu, 17 Feb 2022 08:14:33 +0530 Subject: [PATCH] Respect VM UEFI details on first boot (#5990) * Update settings only if API call is successful * Validate template UEFI detail settings * Read boot mode and type from vm details * Cleanup * Honour boot type over templae settings * Addressing comments * Explicitly thow exception --- .../api/command/user/vm/DeployVMCmd.java | 2 +- .../cloud/template/TemplateManagerImpl.java | 30 ++++++++++++++++++- .../java/com/cloud/vm/UserVmManagerImpl.java | 22 +++++++++----- 3 files changed, 45 insertions(+), 9 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java index 4a9862c3caa..90b999d269d 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java @@ -300,7 +300,7 @@ public class DeployVMCmd extends BaseAsyncCreateCustomIdCmd implements SecurityG } } } - if (ApiConstants.BootType.UEFI.equals(getBootType())) { + if (getBootType() != null) { customparameterMap.put(getBootType().toString(), getBootMode().toString()); } diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index 731b8a4daaa..bf486d1d375 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -20,6 +20,7 @@ import java.net.MalformedURLException; import java.net.URISyntaxException; import java.net.URL; import java.util.ArrayList; +import java.util.Arrays; import java.util.Date; import java.util.HashMap; import java.util.List; @@ -2117,7 +2118,7 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, return template; } - template = _tmpltDao.createForUpdate(id); + template = _tmpltDao.findById(id); if (name != null) { template.setName(name); @@ -2197,6 +2198,8 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, template.setTemplateType(templateType); } + validateDetails(template, details); + if (cleanupDetails) { template.setDetails(null); _tmpltDetailsDao.removeDetails(id); @@ -2211,6 +2214,31 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, return _tmpltDao.findById(id); } + void validateDetails(VMTemplateVO template, Map details) { + if (MapUtils.isEmpty(details)) { + return; + } + String bootMode = details.get(ApiConstants.BootType.UEFI.toString()); + if (bootMode == null) { + return; + } + if (template.isDeployAsIs()) { + String msg = String.format("Deploy-as-is template %s [%s] can not have the UEFI setting. Settings are read directly from the template", + template.getName(), template.getUuid()); + throw new InvalidParameterValueException(msg); + } + try { + String mode = bootMode.trim().toUpperCase(); + ApiConstants.BootMode.valueOf(mode); + details.put(ApiConstants.BootType.UEFI.toString(), mode); + return; + } catch (IllegalArgumentException e) { + String msg = String.format("Invalid %s: %s specified. Valid values are: %s", + ApiConstants.BOOT_MODE, bootMode, Arrays.toString(ApiConstants.BootMode.values())); + s_logger.error(msg); + throw new InvalidParameterValueException(msg); } + } + void verifyTemplateId(Long id) { // Don't allow to modify system template if (id.equals(Long.valueOf(1))) { diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index d553ab47d40..e0d34c1f861 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -4336,6 +4336,18 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir vm.setUserVmType(type); _vmDao.persist(vm); for (String key : customParameters.keySet()) { + // BIOS was explicitly passed as the boot type, so honour it + if (key.equalsIgnoreCase(ApiConstants.BootType.BIOS.toString())) { + vm.details.remove(ApiConstants.BootType.UEFI.toString()); + continue; + } + + // Deploy as is, Don't care about the boot type or template settings + if (key.equalsIgnoreCase(ApiConstants.BootType.UEFI.toString()) && template.isDeployAsIs()) { + vm.details.remove(ApiConstants.BootType.UEFI.toString()); + continue; + } + if (key.equalsIgnoreCase(VmDetailConstants.CPU_NUMBER) || key.equalsIgnoreCase(VmDetailConstants.CPU_SPEED) || key.equalsIgnoreCase(VmDetailConstants.MEMORY)) { @@ -4344,11 +4356,6 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } else { vm.setDetail(key, customParameters.get(key)); } - - if (key.equalsIgnoreCase(ApiConstants.BootType.UEFI.toString())) { - vm.setDetail(key, customParameters.get(key)); - continue; - } } vm.setDetail(VmDetailConstants.DEPLOY_VM, "true"); @@ -4702,8 +4709,9 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir podId = adminCmd.getPodId(); clusterId = adminCmd.getClusterId(); } - if (MapUtils.isNotEmpty(details) && details.containsKey(ApiConstants.BootType.UEFI.toString())) { - addVmUefiBootOptionsToParams(additionalParams, ApiConstants.BootType.UEFI.toString(), details.get(ApiConstants.BootType.UEFI.toString())); + UserVmDetailVO uefiDetail = userVmDetailsDao.findDetail(cmd.getEntityId(), ApiConstants.BootType.UEFI.toString()); + if (uefiDetail != null) { + addVmUefiBootOptionsToParams(additionalParams, uefiDetail.getName(), uefiDetail.getValue()); } if (cmd.getBootIntoSetup() != null) { additionalParams.put(VirtualMachineProfile.Param.BootIntoSetup, cmd.getBootIntoSetup());