From 27ebff9a9c42d95bb65fdfecdb0776215cf7ed45 Mon Sep 17 00:00:00 2001 From: HawkForge <75696486+HawkForge@users.noreply.github.com> Date: Thu, 12 Nov 2026 16:27:07 +0600 Subject: [PATCH] KVM: fix NPE on live migration when the domain XML has no CPU shares updateVmSharesIfNeeded assumed is always present, but CpuTuneDef omits it when the calculated shares is 0. Add the element (and if needed) when the destination requires shares, and leave the XML alone when it does not. --- .../wrapper/LibvirtMigrateCommandWrapper.java | 19 +++++++ .../LibvirtMigrateCommandWrapperTest.java | 56 +++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java index fe18a88fe1f5..0da00251d3ea 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java @@ -456,6 +456,25 @@ protected String updateVmSharesIfNeeded(MigrateCommand migrateCommand, String xm Element root = document.getDocumentElement(); Node sharesNode = root.getElementsByTagName("shares").item(0); + + if (sharesNode == null) { + // CpuTuneDef omits when the calculated value is 0, so the running domain may not have it. + if (newVmCpuShares <= 0) { + return xmlDesc; + } + logger.info(String.format("VM [%s] has no CPU shares defined; setting them to [%s] as part of migration because the cgroups version differs between hosts.", + migrateCommand.getVmName(), newVmCpuShares)); + Node cputuneNode = root.getElementsByTagName("cputune").item(0); + if (cputuneNode == null) { + cputuneNode = document.createElement("cputune"); + root.appendChild(cputuneNode); + } + sharesNode = document.createElement("shares"); + cputuneNode.appendChild(sharesNode); + sharesNode.setTextContent(String.valueOf(newVmCpuShares)); + return getXml(document); + } + String currentShares = sharesNode.getTextContent(); logger.info(String.format("VM [%s] will have CPU shares altered from [%s] to [%s] as part of migration because the cgroups version differs between hosts.", diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapperTest.java index 05d89cc3d972..8e3379f69c8f 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapperTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapperTest.java @@ -950,6 +950,62 @@ public void updateVmSharesIfNeededTestNewCpuSharesLowerThanCurrentSharesShouldUp Assert.assertEquals(updateShares, newVmCpuShares); } + private Element parseRoot(String xml) throws ParserConfigurationException, IOException, SAXException { + InputStream inputStream = IOUtils.toInputStream(xml, StandardCharsets.UTF_8); + DocumentBuilder docBuilder = ParserUtils.getSaferDocumentBuilderFactory().newDocumentBuilder(); + return docBuilder.parse(inputStream).getDocumentElement(); + } + + private void mockSharesMigration(int newVmCpuShares, int currentVmCpuShares) { + Mockito.doReturn(newVmCpuShares).when(migrateCommandMock).getNewVmCpuShares(); + Mockito.doReturn(virtualMachineTOMock).when(migrateCommandMock).getVirtualMachine(); + Mockito.doReturn(currentVmCpuShares).when(libvirtComputingResourceMock).calculateCpuShares(virtualMachineTOMock); + } + + @Test + public void updateVmSharesIfNeededTestNoSharesElementShouldAddItToExistingCputune() throws ParserConfigurationException, IOException, TransformerException, + SAXException { + mockSharesMigration(1, 100); + String xmlWithoutShares = fullfile.replace(" 100\n", ""); + Assert.assertFalse(xmlWithoutShares.contains("")); + + String finalXml = libvirtMigrateCmdWrapper.updateVmSharesIfNeeded(migrateCommandMock, xmlWithoutShares, libvirtComputingResourceMock); + + Element root = parseRoot(finalXml); + Assert.assertEquals(1, root.getElementsByTagName("cputune").getLength()); + Node sharesNode = root.getElementsByTagName("shares").item(0); + Assert.assertNotNull(sharesNode); + Assert.assertEquals("cputune", sharesNode.getParentNode().getNodeName()); + Assert.assertEquals("1", sharesNode.getTextContent()); + } + + @Test + public void updateVmSharesIfNeededTestNoCputuneElementShouldCreateCputuneAndShares() throws ParserConfigurationException, IOException, TransformerException, + SAXException { + mockSharesMigration(1, 100); + String xmlWithoutCputune = fullfile.replace(" \n 100\n \n", ""); + Assert.assertFalse(xmlWithoutCputune.contains("")); + + String finalXml = libvirtMigrateCmdWrapper.updateVmSharesIfNeeded(migrateCommandMock, xmlWithoutCputune, libvirtComputingResourceMock); + + Element root = parseRoot(finalXml); + Node sharesNode = root.getElementsByTagName("shares").item(0); + Assert.assertNotNull(sharesNode); + Assert.assertEquals("cputune", sharesNode.getParentNode().getNodeName()); + Assert.assertEquals("1", sharesNode.getTextContent()); + } + + @Test + public void updateVmSharesIfNeededTestNoSharesElementAndNoNewSharesShouldNotChangeXml() throws ParserConfigurationException, IOException, TransformerException, + SAXException { + mockSharesMigration(0, 100); + String xmlWithoutShares = fullfile.replace(" 100\n", ""); + + String finalXml = libvirtMigrateCmdWrapper.updateVmSharesIfNeeded(migrateCommandMock, xmlWithoutShares, libvirtComputingResourceMock); + + Assert.assertEquals(xmlWithoutShares, finalXml); + } + @Test public void getMigrateStorageDeviceLabelsTestNoDiskDefinitions() { Map mapMigrateStorage = createMapMigrateStorage("sourceTest", "/mnt/812ea6a3-7ad0-30f4-9cab-01e3f2985b98/4650a2f7-fce5-48e2-beaa-bcdf063194e6");