KVM: fix NPE on live migration when the domain XML has no CPU shares

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 <shares> 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.",

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("    <shares>100</shares>\n", "");
+        Assert.assertFalse(xmlWithoutShares.contains("<shares>"));
+
+        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("  <cputune>\n    <shares>100</shares>\n  </cputune>\n", "");
+        Assert.assertFalse(xmlWithoutCputune.contains("<cputune>"));
+
+        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("    <shares>100</shares>\n", "");
+
+        String finalXml = libvirtMigrateCmdWrapper.updateVmSharesIfNeeded(migrateCommandMock, xmlWithoutShares, libvirtComputingResourceMock);
+
+        Assert.assertEquals(xmlWithoutShares, finalXml);
+    }
+
     @Test
     public void getMigrateStorageDeviceLabelsTestNoDiskDefinitions() {
         Map<String, MigrateDiskInfo> mapMigrateStorage = createMapMigrateStorage("sourceTest", "/mnt/812ea6a3-7ad0-30f4-9cab-01e3f2985b98/4650a2f7-fce5-48e2-beaa-bcdf063194e6");