Allow KVM overcommit to work without reducing minimum VM memory when vm ballooning is disabled (#7810)

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
Co-authored-by: dahn <daan.hoogland@gmail.com>
Co-authored-by: Daan Hoogland <daan@onecht.net>

Author: yadvr

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java

@@ -2780,15 +2780,10 @@ protected GuestResourceDef createGuestResourceDef(VirtualMachineTO vmTO){
 
         grd.setMemBalloning(!_noMemBalloon);
 
-        long maxRam = ByteScaleUtils.bytesToKibibytes(vmTO.getMaxRam());
-        long currRam = vmTO.getType() == VirtualMachine.Type.User ? getCurrentMemAccordingToMemBallooning(vmTO, maxRam) : maxRam;
-
-        if (s_logger.isTraceEnabled()) {
-            s_logger.trace(String.format("memory values for VM %s are %d/%d",vmTO.getName(),maxRam, currRam));
-        }
+        Long maxRam = ByteScaleUtils.bytesToKibibytes(vmTO.getMaxRam());
 
         grd.setMemorySize(maxRam);
-        grd.setCurrentMem(currRam);
+        grd.setCurrentMem(getCurrentMemAccordingToMemBallooning(vmTO, maxRam));
 
         int vcpus = vmTO.getCpus();
         Integer maxVcpus = vmTO.getVcpuMaxLimit();
@@ -2800,12 +2795,17 @@ protected GuestResourceDef createGuestResourceDef(VirtualMachineTO vmTO){
     }
 
     protected long getCurrentMemAccordingToMemBallooning(VirtualMachineTO vmTO, long maxRam) {
+        long retVal = maxRam;
         if (_noMemBalloon) {
             s_logger.warn(String.format("Setting VM's [%s] current memory as max memory [%s] due to memory ballooning is disabled. If you are using a custom service offering, verify if memory ballooning really should be disabled.", vmTO.toString(), maxRam));
-            return maxRam;
+        } else if (vmTO != null && vmTO.getType() != VirtualMachine.Type.User) {
+            s_logger.warn(String.format("Setting System VM's [%s] current memory as max memory [%s].", vmTO.toString(), maxRam));
         } else {
-            return ByteScaleUtils.bytesToKibibytes(vmTO.getMinRam());
+            long minRam = ByteScaleUtils.bytesToKibibytes(vmTO.getMinRam());
+            s_logger.debug(String.format("Setting VM's [%s] current memory as min memory [%s] due to memory ballooning is enabled.", vmTO.toString(), minRam));
+            retVal = minRam;
         }
+        return retVal;
     }
 
     /**

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java

@@ -282,7 +282,7 @@ public void setMemBalloning(boolean memoryBalloning) {
         @Override
         public String toString() {
             StringBuilder response = new StringBuilder();
-            response.append(String.format("<memory>%s</memory>\n", this.memory));
+            response.append(String.format("<memory>%s</memory>\n", this.currentMemory));
             response.append(String.format("<currentMemory>%s</currentMemory>\n", this.currentMemory));
 
             if (this.memory > this.currentMemory) {
@@ -1238,7 +1238,7 @@ public String getMemBalloonStatsPeriod() {
         @Override
         public String toString() {
             StringBuilder memBalloonBuilder = new StringBuilder();
-            memBalloonBuilder.append("<memballoon model='" + memBalloonModel + "' autodeflate='on'>\n");
+            memBalloonBuilder.append("<memballoon model='" + memBalloonModel + "'>\n");
             if (StringUtils.isNotBlank(memBalloonStatsPeriod)) {
                 memBalloonBuilder.append("<stats period='" + memBalloonStatsPeriod +"'/>\n");
             }

plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java

@@ -760,7 +760,7 @@ private void verifyVcpu(VirtualMachineTO to, Document domainDoc) {
 
     private void verifyMemory(VirtualMachineTO to, Document domainDoc, String minRam) {
         assertXpath(domainDoc, "/domain/maxMemory/text()", String.valueOf( to.getMaxRam() / 1024 ));
-        assertXpath(domainDoc, "/domain/currentMemory/text()",minRam);
+        assertXpath(domainDoc, "/domain/memory/text()",minRam);
         assertXpath(domainDoc, "/domain/cpu/numa/cell/@memory", minRam);
         assertXpath(domainDoc, "/domain/currentMemory/text()", minRam);
     }
@@ -5762,6 +5762,7 @@ public void testAddExtraConfigComponentNotEmptyExtraConfig() {
 
     public void validateGetCurrentMemAccordingToMemBallooningWithoutMemBalooning(){
         VirtualMachineTO vmTo = Mockito.mock(VirtualMachineTO.class);
+        Mockito.when(vmTo.getType()).thenReturn(Type.User);
         LibvirtComputingResource libvirtComputingResource = new LibvirtComputingResource();
         libvirtComputingResource._noMemBalloon = true;
         long maxMemory = 2048;
@@ -5780,6 +5781,7 @@ public void validateGetCurrentMemAccordingToMemBallooningWithtMemBalooning(){
         long minMemory = ByteScaleUtils.mebibytesToBytes(64);
 
         VirtualMachineTO vmTo = Mockito.mock(VirtualMachineTO.class);
+        Mockito.when(vmTo.getType()).thenReturn(Type.User);
         Mockito.when(vmTo.getMinRam()).thenReturn(minMemory);
 
         long currentMemory = libvirtComputingResource.getCurrentMemAccordingToMemBallooning(vmTo, maxMemory);

plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java

@@ -214,11 +214,11 @@ public void testDiskDef() {
         assertEquals(bus, disk.getBusType());
         assertEquals(DiskDef.DeviceType.DISK, disk.getDeviceType());
 
-        String resultingXml = disk.toString();
+        String xmlDef = disk.toString();
         String expectedXml = "<disk  device='disk' type='file'>\n<driver name='qemu' type='" + type.toString() + "' cache='" + cacheMode.toString() + "' />\n" +
                              "<source file='" + filePath + "'/>\n<target dev='" + diskLabel + "' bus='" + bus.toString() + "'/>\n</disk>\n";
 
-        assertEquals(expectedXml, resultingXml);
+        assertEquals(expectedXml, xmlDef);
     }
 
     @Test
@@ -346,7 +346,7 @@ public void testDiskDefWithBurst() {
         LibvirtVMDef.setGlobalQemuVersion(2006000L);
         LibvirtVMDef.setGlobalLibvirtVersion(9008L);
 
-        String resultingXml = disk.toString();
+        String xmlDef = disk.toString();
         String expectedXml = "<disk  device='disk' type='file'>\n<driver name='qemu' type='" + type.toString() + "' cache='none' />\n" +
                 "<source file='" + filePath + "'/>\n<target dev='" + diskLabel + "' bus='" + bus.toString() + "'/>\n" +
                 "<iotune>\n<read_bytes_sec>"+bytesReadRate+"</read_bytes_sec>\n<write_bytes_sec>"+bytesWriteRate+"</write_bytes_sec>\n" +
@@ -356,29 +356,29 @@ public void testDiskDefWithBurst() {
                 "<read_bytes_sec_max_length>"+bytesReadRateMaxLength+"</read_bytes_sec_max_length>\n<write_bytes_sec_max_length>"+bytesWriteRateMaxLength+"</write_bytes_sec_max_length>\n" +
                 "<read_iops_sec_max_length>"+iopsReadRateMaxLength+"</read_iops_sec_max_length>\n<write_iops_sec_max_length>"+iopsWriteRateMaxLength+"</write_iops_sec_max_length>\n</iotune>\n</disk>\n";
 
-                assertEquals(expectedXml, resultingXml);
+                assertEquals(expectedXml, xmlDef);
     }
 
     @Test
     public void memBalloonDefTestNone() {
-        String expectedXml = "<memballoon model='none' autodeflate='on'>\n</memballoon>";
+        String expectedXml = "<memballoon model='none'>\n</memballoon>";
         MemBalloonDef memBalloonDef = new MemBalloonDef();
         memBalloonDef.defNoneMemBalloon();
 
-        String resultingXml = memBalloonDef.toString();
+        String xmlDef = memBalloonDef.toString();
 
-        assertEquals(expectedXml, resultingXml);
+        assertEquals(expectedXml, xmlDef);
     }
 
     @Test
     public void memBalloonDefTestVirtio() {
-        String expectedXml = "<memballoon model='virtio' autodeflate='on'>\n<stats period='60'/>\n</memballoon>";
+        String expectedXml = "<memballoon model='virtio'>\n<stats period='60'/>\n</memballoon>";
         MemBalloonDef memBalloonDef = new MemBalloonDef();
         memBalloonDef.defVirtioMemBalloon("60");
 
-        String resultingXml = memBalloonDef.toString();
+        String xmlDef = memBalloonDef.toString();
 
-        assertEquals(expectedXml, resultingXml);
+        assertEquals(expectedXml, xmlDef);
     }
 
     @Test