From d6f63ab933ac75237415c087e49b9d01e2ce765c Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 16 Jul 2025 15:04:46 +0530 Subject: [PATCH 1/2] server,kvm: detect boot options for vm import Fixes #11184 Signed-off-by: Abhishek Kumar --- .../kvm/resource/LibvirtDomainXMLParser.java | 36 +++++++++++++++++++ .../hypervisor/kvm/resource/LibvirtVMDef.java | 4 +-- ...rtGetUnmanagedInstancesCommandWrapper.java | 6 ++++ .../vm/UnmanagedVMsManagerImpl.java | 3 ++ 4 files changed, 47 insertions(+), 2 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java index c4d5ac48f6f8..39025e4b7751 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java @@ -48,6 +48,7 @@ import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.WatchDogDef; import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.WatchDogDef.WatchDogAction; import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.WatchDogDef.WatchDogModel; +import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.GuestDef; public class LibvirtDomainXMLParser { protected Logger logger = LogManager.getLogger(getClass()); @@ -63,6 +64,8 @@ public class LibvirtDomainXMLParser { private LibvirtVMDef.CpuTuneDef cpuTuneDef; private LibvirtVMDef.CpuModeDef cpuModeDef; private String name; + private GuestDef.BootType bootType; + private GuestDef.BootMode bootMode; public boolean parseDomainXML(String domXML) { DocumentBuilder builder; @@ -516,6 +519,14 @@ public LibvirtVMDef.CpuModeDef getCpuModeDef() { return cpuModeDef; } + public GuestDef.BootType getBootType() { + return bootType; + } + + public GuestDef.BootMode getBootMode() { + return bootMode; + } + private void extractCpuTuneDef(final Element rootElement) { NodeList cpuTunesList = rootElement.getElementsByTagName("cputune"); if (cpuTunesList.getLength() > 0) { @@ -569,4 +580,29 @@ private void extractCpuModeDef(final Element rootElement){ } } } + + private void extractBootDef(final Element rootElement) { + Element osElement = (Element) rootElement.getElementsByTagName("os").item(0); + if (osElement != null) { + NodeList loaderList = osElement.getElementsByTagName("loader"); + if (loaderList.getLength() > 0) { + Element loader = (Element) loaderList.item(0); + String type = loader.getAttribute("type"); + String secure = loader.getAttribute("secure"); + if ("pflash".equalsIgnoreCase(type) || loader.getTextContent().toLowerCase().contains("uefi")) { + bootType = GuestDef.BootType.UEFI; + } else { + bootType = GuestDef.BootType.BIOS; + } + if ("yes".equalsIgnoreCase(secure)) { + bootMode = GuestDef.BootMode.SECURE; + } else { + bootMode = GuestDef.BootMode.LEGACY; + } + } else { + bootType = GuestDef.BootType.BIOS; + bootMode = GuestDef.BootMode.LEGACY; + } + } + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java index 93ad084b4379..5174563f675a 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java @@ -65,7 +65,7 @@ public String toString() { } } - enum BootType { + public enum BootType { UEFI("UEFI"), BIOS("BIOS"); String _type; @@ -80,7 +80,7 @@ public String toString() { } } - enum BootMode { + public enum BootMode { LEGACY("LEGACY"), SECURE("SECURE"); String _mode; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetUnmanagedInstancesCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetUnmanagedInstancesCommandWrapper.java index 70b1715cc20a..e807bf3c8211 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetUnmanagedInstancesCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetUnmanagedInstancesCommandWrapper.java @@ -135,6 +135,12 @@ private UnmanagedInstanceTO getUnmanagedInstance(LibvirtComputingResource libvir instance.setNics(getUnmanagedInstanceNics(parser.getInterfaces())); instance.setDisks(getUnmanagedInstanceDisks(parser.getDisks(),libvirtComputingResource, conn, domain.getName())); instance.setVncPassword(getFormattedVncPassword(parser.getVncPasswd())); + if (parser.getBootType() != null) { + instance.setBootType(parser.getBootType().toString()); + } + if (parser.getBootMode() != null) { + instance.setBootMode(parser.getBootMode().toString()); + } return instance; } catch (Exception e) { diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index 03c0ad06dc95..ce8fa87c6223 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -1138,6 +1138,9 @@ private UserVm importVirtualMachineInternal(final UnmanagedInstanceTO unmanagedI allDetails.put(VmDetailConstants.CPU_SPEED, String.valueOf(validatedServiceOffering.getSpeed())); } } + if (!template.isDeployAsIs() && unmanagedInstance.getBootType() != null) { + allDetails.put(unmanagedInstance.getBootType(), unmanagedInstance.getBootMode()); + } if (!migrateAllowed && host != null && !hostSupportsServiceOfferingAndTemplate(host, validatedServiceOffering, template)) { throw new InvalidParameterValueException(String.format("Service offering: %s or template: %s is not compatible with host: %s of unmanaged VM: %s", serviceOffering.getUuid(), template.getUuid(), host.getUuid(), instanceName)); From 0eb8030bda2d4b8971d5aa49dbcf72b24c534e47 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 28 Jul 2025 17:51:23 +0530 Subject: [PATCH 2/2] tests and changes Signed-off-by: Abhishek Kumar --- .../kvm/resource/LibvirtDomainXMLParser.java | 40 ++++----- .../resource/LibvirtDomainXMLParserTest.java | 90 +++++++++++++++++++ .../vm/UnmanagedVMsManagerImpl.java | 3 - 3 files changed, 109 insertions(+), 24 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java index 39025e4b7751..8b26bc94e218 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java @@ -391,6 +391,7 @@ public boolean parseDomainXML(String domXML) { } extractCpuTuneDef(rootElement); extractCpuModeDef(rootElement); + extractBootDef(rootElement); return true; } catch (ParserConfigurationException e) { logger.debug(e.toString()); @@ -581,28 +582,25 @@ private void extractCpuModeDef(final Element rootElement){ } } - private void extractBootDef(final Element rootElement) { + protected void extractBootDef(final Element rootElement) { + bootType = GuestDef.BootType.BIOS; + bootMode = GuestDef.BootMode.LEGACY; Element osElement = (Element) rootElement.getElementsByTagName("os").item(0); - if (osElement != null) { - NodeList loaderList = osElement.getElementsByTagName("loader"); - if (loaderList.getLength() > 0) { - Element loader = (Element) loaderList.item(0); - String type = loader.getAttribute("type"); - String secure = loader.getAttribute("secure"); - if ("pflash".equalsIgnoreCase(type) || loader.getTextContent().toLowerCase().contains("uefi")) { - bootType = GuestDef.BootType.UEFI; - } else { - bootType = GuestDef.BootType.BIOS; - } - if ("yes".equalsIgnoreCase(secure)) { - bootMode = GuestDef.BootMode.SECURE; - } else { - bootMode = GuestDef.BootMode.LEGACY; - } - } else { - bootType = GuestDef.BootType.BIOS; - bootMode = GuestDef.BootMode.LEGACY; - } + if (osElement == null) { + return; + } + NodeList loaderList = osElement.getElementsByTagName("loader"); + if (loaderList.getLength() == 0) { + return; + } + Element loader = (Element) loaderList.item(0); + String type = loader.getAttribute("type"); + String secure = loader.getAttribute("secure"); + if ("pflash".equalsIgnoreCase(type) || loader.getTextContent().toLowerCase().contains("uefi")) { + bootType = GuestDef.BootType.UEFI; + } + if ("yes".equalsIgnoreCase(secure)) { + bootMode = GuestDef.BootMode.SECURE; } } } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParserTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParserTest.java index 921bbf0c9a87..de50cf342024 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParserTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParserTest.java @@ -20,8 +20,13 @@ package com.cloud.hypervisor.kvm.resource; import java.io.File; +import java.io.IOException; +import java.io.StringReader; import java.util.List; +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.ParserConfigurationException; + import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.ChannelDef; import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.DiskDef; import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.InterfaceDef; @@ -31,10 +36,15 @@ import junit.framework.TestCase; import org.apache.cloudstack.utils.qemu.QemuObject; +import org.apache.cloudstack.utils.security.ParserUtils; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.junit.MockitoJUnitRunner; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.xml.sax.InputSource; +import org.xml.sax.SAXException; @RunWith(MockitoJUnitRunner.class) public class LibvirtDomainXMLParserTest extends TestCase { @@ -386,4 +396,84 @@ public void testDomainXMLParserWithoutModelName() { Assert.assertEquals("CPU cores count is parsed", 4, libvirtDomainXMLParser.getCpuModeDef().getCoresPerSocket()); Assert.assertEquals("CPU threads count is parsed", 2, libvirtDomainXMLParser.getCpuModeDef().getThreadsPerCore()); } + + private LibvirtDomainXMLParser parseElementFromXML(String xml) { + LibvirtDomainXMLParser parser = new LibvirtDomainXMLParser(); + DocumentBuilder builder; + try { + builder = ParserUtils.getSaferDocumentBuilderFactory().newDocumentBuilder(); + InputSource is = new InputSource(); + is.setCharacterStream(new StringReader(xml)); + Document doc = builder.parse(is); + Element element = doc.getDocumentElement(); + parser.extractBootDef(element); + } catch (ParserConfigurationException | IOException | SAXException e) { + Assert.fail("Failed to parse XML: " + e.getMessage()); + } + return parser; + } + + @Test + public void extractBootDefParsesUEFISecureBootCorrectly() { + String xml = "" + + "" + + "/path/to/uefi/loader" + + "" + + ""; + + LibvirtDomainXMLParser parser = parseElementFromXML(xml); + + assertEquals(LibvirtVMDef.GuestDef.BootType.UEFI, parser.getBootType()); + assertEquals(LibvirtVMDef.GuestDef.BootMode.SECURE, parser.getBootMode()); + } + + @Test + public void extractBootDefParsesUEFILegacyBootCorrectly() { + String xml = "" + + "" + + "/path/to/uefi/loader" + + "" + + ""; + + LibvirtDomainXMLParser parser = parseElementFromXML(xml); + + assertEquals(LibvirtVMDef.GuestDef.BootType.UEFI, parser.getBootType()); + assertEquals(LibvirtVMDef.GuestDef.BootMode.LEGACY, parser.getBootMode()); + } + + @Test + public void extractBootDefDefaultsToBIOSLegacyWhenNoLoaderPresent() { + String xml = "" + + "" + + "hvm" + + "" + + ""; + + LibvirtDomainXMLParser parser = parseElementFromXML(xml); + + assertEquals(LibvirtVMDef.GuestDef.BootType.BIOS, parser.getBootType()); + assertEquals(LibvirtVMDef.GuestDef.BootMode.LEGACY, parser.getBootMode()); + } + + @Test + public void extractBootDefHandlesEmptyOSSection() { + String xml = "" + + "" + + ""; + + LibvirtDomainXMLParser parser = parseElementFromXML(xml); + + assertEquals(LibvirtVMDef.GuestDef.BootType.BIOS, parser.getBootType()); + assertEquals(LibvirtVMDef.GuestDef.BootMode.LEGACY, parser.getBootMode()); + } + + @Test + public void extractBootDefHandlesMissingOSSection() { + String xml = ""; + + LibvirtDomainXMLParser parser = parseElementFromXML(xml); + + assertEquals(LibvirtVMDef.GuestDef.BootType.BIOS, parser.getBootType()); + assertEquals(LibvirtVMDef.GuestDef.BootMode.LEGACY, parser.getBootMode()); + } } diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index 8d8785de3ba7..865884c25387 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -1140,9 +1140,6 @@ private UserVm importVirtualMachineInternal(final UnmanagedInstanceTO unmanagedI allDetails.put(VmDetailConstants.CPU_SPEED, String.valueOf(validatedServiceOffering.getSpeed())); } } - if (!template.isDeployAsIs() && unmanagedInstance.getBootType() != null) { - allDetails.put(unmanagedInstance.getBootType(), unmanagedInstance.getBootMode()); - } if (!migrateAllowed && host != null && !hostSupportsServiceOfferingAndTemplate(host, validatedServiceOffering, template)) { throw new InvalidParameterValueException(String.format("Service offering: %s or template: %s is not compatible with host: %s of unmanaged VM: %s", serviceOffering.getUuid(), template.getUuid(), host.getUuid(), instanceName));