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");