From 99d132eb480a072d79dbd2b30f67b0164f6b66c2 Mon Sep 17 00:00:00 2001 From: Marcus Sorensen Date: Fri, 27 Jan 2023 03:23:53 -0700 Subject: [PATCH] Fix UEFI detection on KVM and prevent deployments on non UEFI enabled hosts (#6423) (#221) * Do not allow UEFI deployments on non UEFI enabled hosts * Fix UEFI detection on KVM * Refactor * Improvement agent: Detect existing hosts with UEFI support (#6139) * agent: Pass uefi enabled status as part of ready command * Cleanup * Fix checkstyle * Save uefi status if different Co-authored-by: Marcus Sorensen --- .../java/com/cloud/agent/api/ReadyAnswer.java | 15 +++++++++++ .../cloud/agent/manager/AgentManagerImpl.java | 15 +++++++++++ .../resource/LibvirtComputingResource.java | 6 ++++- .../wrapper/LibvirtReadyCommandWrapper.java | 25 ++++++++++++++++++- .../vmware/VmwareServerDiscoverer.java | 5 ---- .../vmware/resource/VmwareResource.java | 12 ++++++++- .../discoverer/XcpServerDiscoverer.java | 3 --- .../resource/CitrixResourceBase.java | 4 --- .../xenbase/CitrixReadyCommandWrapper.java | 13 +++++++++- .../deploy/DeploymentPlanningManagerImpl.java | 7 +++--- .../discoverer/LibvirtServerDiscoverer.java | 8 ------ 11 files changed, 85 insertions(+), 28 deletions(-) diff --git a/core/src/main/java/com/cloud/agent/api/ReadyAnswer.java b/core/src/main/java/com/cloud/agent/api/ReadyAnswer.java index 700f64b4b15..42f18a23aba 100644 --- a/core/src/main/java/com/cloud/agent/api/ReadyAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/ReadyAnswer.java @@ -19,7 +19,12 @@ package com.cloud.agent.api; +import java.util.Map; + public class ReadyAnswer extends Answer { + + private Map detailsMap; + protected ReadyAnswer() { } @@ -31,4 +36,14 @@ public class ReadyAnswer extends Answer { super(cmd, false, details); } + public ReadyAnswer(ReadyCommand cmd, Map detailsMap) { + super(cmd, true, null); + this.detailsMap = detailsMap; + } + + public Map getDetailsMap() { + return detailsMap; + } + + } diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java index c15edcf7309..f54dc7cedaf 100644 --- a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java @@ -595,6 +595,21 @@ public class AgentManagerImpl extends ManagerBase implements AgentManager, Handl // return the attache instead of null, even it is disconnectede handleDisconnectWithoutInvestigation(attache, Event.AgentDisconnected, true, true); } + if (answer instanceof ReadyAnswer) { + ReadyAnswer readyAnswer = (ReadyAnswer)answer; + Map detailsMap = readyAnswer.getDetailsMap(); + if (detailsMap != null) { + String uefiEnabled = detailsMap.get(Host.HOST_UEFI_ENABLE); + s_logger.debug(String.format("Got HOST_UEFI_ENABLE [%s] for hostId [%s]:", uefiEnabled, host.getUuid())); + if (uefiEnabled != null) { + _hostDao.loadDetails(host); + if (!uefiEnabled.equals(host.getDetails().get(Host.HOST_UEFI_ENABLE))) { + host.getDetails().put(Host.HOST_UEFI_ENABLE, uefiEnabled); + _hostDao.saveDetails(host); + } + } + } + } agentStatusTransitTo(host, Event.Ready, _nodeId); attache.ready(); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 46fed5252cf..eec52f6b2db 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -1396,9 +1396,13 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv s_logger.debug("agent.hooks.libvirt_vm_on_stop.method is " + _agentHooksVmOnStopMethod); } + public boolean isUefiPropertiesFileLoaded() { + return !_uefiProperties.isEmpty(); + } + private void loadUefiProperties() throws FileNotFoundException { - if (_uefiProperties != null && _uefiProperties.getProperty("guest.loader.legacy") != null) { + if (isUefiPropertiesFileLoaded()) { return; } final File file = PropertiesUtil.findConfigFile("uefi.properties"); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java index c48f91f3eb3..a45f11bea83 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java @@ -19,18 +19,41 @@ package com.cloud.hypervisor.kvm.resource.wrapper; +import java.util.HashMap; +import java.util.Map; + import com.cloud.agent.api.Answer; import com.cloud.agent.api.ReadyAnswer; import com.cloud.agent.api.ReadyCommand; +import com.cloud.host.Host; import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; import com.cloud.resource.CommandWrapper; import com.cloud.resource.ResourceWrapper; +import com.cloud.utils.script.Script; + +import org.apache.log4j.Logger; @ResourceWrapper(handles = ReadyCommand.class) public final class LibvirtReadyCommandWrapper extends CommandWrapper { + private static final Logger s_logger = Logger.getLogger(LibvirtReadyCommandWrapper.class); + @Override public Answer execute(final ReadyCommand command, final LibvirtComputingResource libvirtComputingResource) { - return new ReadyAnswer(command); + Map hostDetails = new HashMap(); + + if (hostSupportsUefi() && libvirtComputingResource.isUefiPropertiesFileLoaded()) { + hostDetails.put(Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString()); + } + + return new ReadyAnswer(command, hostDetails); + } + + private boolean hostSupportsUefi() { + String cmd = "rpm -qa | grep -i ovmf"; + s_logger.debug("Running command : " + cmd); + int result = Script.runSimpleBashScriptForExitValue(cmd); + s_logger.debug("Got result : " + result); + return result == 0; } } \ No newline at end of file diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/VmwareServerDiscoverer.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/VmwareServerDiscoverer.java index f1f3b709cfc..6a8bcda23e5 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/VmwareServerDiscoverer.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/VmwareServerDiscoverer.java @@ -42,7 +42,6 @@ import com.cloud.exception.DiscoveredWithErrorException; import com.cloud.exception.DiscoveryException; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.ResourceInUseException; -import com.cloud.host.Host; import com.cloud.host.HostVO; import com.cloud.hypervisor.Hypervisor; import com.cloud.hypervisor.Hypervisor.HypervisorType; @@ -367,10 +366,6 @@ public class VmwareServerDiscoverer extends DiscovererBase implements Discoverer details.put("url", hostMo.getHostName()); details.put("username", username); details.put("password", password); - boolean uefiLegacySupported = hostMo.isUefiLegacySupported(); - if (uefiLegacySupported) { - details.put(Host.HOST_UEFI_ENABLE, "true"); - } String guid = morHost.getType() + ":" + morHost.getValue() + "@" + url.getHost(); details.put("guid", guid); diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 3732804f9a8..7b675491ac5 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -212,6 +212,7 @@ import com.cloud.configuration.Resource.ResourceType; import com.cloud.dc.Vlan; import com.cloud.exception.CloudException; import com.cloud.exception.InternalErrorException; +import com.cloud.host.Host; import com.cloud.host.Host.Type; import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.hypervisor.guru.VMwareGuru; @@ -3926,8 +3927,17 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa try { VmwareContext context = getServiceContext(); VmwareHypervisorHost hyperHost = getHyperHost(context); + + Map hostDetails = new HashMap(); + ManagedObjectReference morHost = hyperHost.getMor(); + HostMO hostMo = new HostMO(context, morHost); + boolean uefiLegacySupported = hostMo.isUefiLegacySupported(); + if (uefiLegacySupported) { + hostDetails.put(Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString()); + } + if (hyperHost.isHyperHostConnected()) { - return new ReadyAnswer(cmd); + return new ReadyAnswer(cmd, hostDetails); } else { return new ReadyAnswer(cmd, "Host is not in connect state"); } diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/discoverer/XcpServerDiscoverer.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/discoverer/XcpServerDiscoverer.java index bbf3686750d..095ba810841 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/discoverer/XcpServerDiscoverer.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/discoverer/XcpServerDiscoverer.java @@ -321,9 +321,6 @@ public class XcpServerDiscoverer extends DiscovererBase implements Discoverer, L details.put("username", username); params.put("username", username); details.put("password", password); - if (isUefiSupported(prodVersion)) { - details.put(com.cloud.host.Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString()); - } params.put("password", password); params.put("zone", Long.toString(dcId)); params.put("guid", record.uuid); diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java index 19625a2f09a..212eca8569e 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java @@ -16,7 +16,6 @@ // under the License. package com.cloud.hypervisor.xenserver.resource; -import static com.cloud.hypervisor.xenserver.discoverer.XcpServerDiscoverer.isUefiSupported; import static com.cloud.utils.NumbersUtil.toHumanReadableSize; import java.io.BufferedReader; @@ -1779,9 +1778,6 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe } details.put("product_brand", productBrand); details.put("product_version", _host.getProductVersion()); - if (isUefiSupported(_host.getProductVersion())) { - details.put(com.cloud.host.Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString()); - } if (hr.softwareVersion.get("product_version_text_short") != null) { details.put("product_version_text_short", hr.softwareVersion.get("product_version_text_short")); cmd.setHypervisorVersion(hr.softwareVersion.get("product_version_text_short")); diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixReadyCommandWrapper.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixReadyCommandWrapper.java index 78359f0ab91..121bc210c06 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixReadyCommandWrapper.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixReadyCommandWrapper.java @@ -19,14 +19,19 @@ package com.cloud.hypervisor.xenserver.resource.wrapper.xenbase; +import java.util.Map; +import java.util.HashMap; import java.util.Set; import org.apache.log4j.Logger; import org.apache.xmlrpc.XmlRpcException; +import static com.cloud.hypervisor.xenserver.discoverer.XcpServerDiscoverer.isUefiSupported; + import com.cloud.agent.api.Answer; import com.cloud.agent.api.ReadyAnswer; import com.cloud.agent.api.ReadyCommand; +import com.cloud.hypervisor.xenserver.resource.CitrixHelper; import com.cloud.hypervisor.xenserver.resource.CitrixResourceBase; import com.cloud.resource.CommandWrapper; import com.cloud.resource.ResourceWrapper; @@ -44,6 +49,7 @@ public final class CitrixReadyCommandWrapper extends CommandWrapper hostDetails = new HashMap(); // Ignore the result of the callHostPlugin. Even if unmounting the // snapshots dir fails, let Ready command // succeed. @@ -55,6 +61,11 @@ public final class CitrixReadyCommandWrapper extends CommandWrapper vms = host.getResidentVMs(conn); citrixResourceBase.destroyPatchVbd(conn, vms); + + final Host.Record hr = host.getRecord(conn); + if (isUefiSupported(CitrixHelper.getProductVersion(hr))) { + hostDetails.put(com.cloud.host.Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString()); + } } catch (final Exception e) { } try { @@ -70,6 +81,6 @@ public final class CitrixReadyCommandWrapper extends CommandWrapper, Configurable { } HostVO host = _hostDao.findById(hostIdSpecified); if (host != null && StringUtils.isNotBlank(uefiFlag) && "yes".equalsIgnoreCase(uefiFlag)) { - _hostDao.loadDetails(host); - if (MapUtils.isNotEmpty(host.getDetails()) && host.getDetails().containsKey(Host.HOST_UEFI_ENABLE) && "false".equalsIgnoreCase(host.getDetails().get(Host.HOST_UEFI_ENABLE))) { + DetailVO uefiHostDetail = _hostDetailsDao.findDetail(host.getId(), Host.HOST_UEFI_ENABLE); + if (uefiHostDetail == null || "false".equalsIgnoreCase(uefiHostDetail.getValue())) { s_logger.debug("Cannot deploy to specified host as host does n't support uefi vm deployment, returning."); return null; diff --git a/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java b/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java index 818c14f5b25..fb14e2c33f3 100644 --- a/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java +++ b/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java @@ -214,8 +214,6 @@ public abstract class LibvirtServerDiscoverer extends DiscovererBase implements @Override public Map> find(long dcId, Long podId, Long clusterId, URI uri, String username, String password, List hostTags) throws DiscoveryException { - boolean isUefiSupported = false; - ClusterVO cluster = _clusterDao.findById(clusterId); if (cluster == null || cluster.getHypervisorType() != getHypervisorType()) { if (s_logger.isInfoEnabled()) @@ -277,11 +275,6 @@ public abstract class LibvirtServerDiscoverer extends DiscovererBase implements return null; } - if (SSHCmdHelper.sshExecuteCmd(sshConnection, "rpm -qa | grep -i ovmf", 3)) { - s_logger.debug("It's UEFI enabled KVM machine"); - isUefiSupported = true; - } - List netInfos = _networkMgr.getPhysicalNetworkInfo(dcId, getHypervisorType()); String kvmPrivateNic = null; String kvmPublicNic = null; @@ -364,7 +357,6 @@ public abstract class LibvirtServerDiscoverer extends DiscovererBase implements Map hostDetails = connectedHost.getDetails(); hostDetails.put("password", password); hostDetails.put("username", username); - hostDetails.put(Host.HOST_UEFI_ENABLE, isUefiSupported == true ? Boolean.toString(true) : Boolean.toString(false)); _hostDao.saveDetails(connectedHost); return resources; } catch (DiscoveredWithErrorException e) {