From 091ed9a119548989100929ce491cee04035ed867 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 3 Aug 2023 16:21:34 +0530 Subject: [PATCH 01/15] Revert "KVM: fix SSVM starting when overprovisioning memory (#7663)" This reverts commit d127d7939de92ecf3514128207baab36364e51d1. --- .../resource/LibvirtComputingResource.java | 9 +---- .../hypervisor/kvm/resource/LibvirtVMDef.java | 4 +- .../LibvirtComputingResourceTest.java | 2 +- .../kvm/resource/LibvirtVMDefTest.java | 38 +++++++++---------- .../java/com/cloud/hypervisor/KVMGuru.java | 2 +- 5 files changed, 25 insertions(+), 30 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 39f30b6e9349..3f711fa9b809 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/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(); 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 720c48096ef5..b739c0ee0ab6 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 @@ -282,7 +282,7 @@ public void setMemBalloning(boolean memoryBalloning) { @Override public String toString() { StringBuilder response = new StringBuilder(); - response.append(String.format("%s\n", this.memory)); + response.append(String.format("%s\n", this.currentMemory)); response.append(String.format("%s\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("\n"); + memBalloonBuilder.append("\n"); if (StringUtils.isNotBlank(memBalloonStatsPeriod)) { memBalloonBuilder.append("\n"); } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java index 58682b2663cc..ffe20b4ad305 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java +++ b/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); } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java index 4379ede8293d..ec59265c8325 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java +++ b/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 = "\n\n" + "\n\n\n"; - assertEquals(expectedXml, resultingXml); + assertEquals(xmlDef, expectedXml); } @Test @@ -236,7 +236,7 @@ public void testDiskDefWithEncryption() { "\n" + "\n" + "\n"; - assertEquals(expectedXML, disk.toString()); + assertEquals(disk.toString(), expectedXML); } @Test @@ -346,7 +346,7 @@ public void testDiskDefWithBurst() { LibvirtVMDef.setGlobalQemuVersion(2006000L); LibvirtVMDef.setGlobalLibvirtVersion(9008L); - String resultingXml = disk.toString(); + String xmlDef = disk.toString(); String expectedXml = "\n\n" + "\n\n" + "\n"+bytesReadRate+"\n"+bytesWriteRate+"\n" + @@ -356,29 +356,29 @@ public void testDiskDefWithBurst() { ""+bytesReadRateMaxLength+"\n"+bytesWriteRateMaxLength+"\n" + ""+iopsReadRateMaxLength+"\n"+iopsWriteRateMaxLength+"\n\n\n"; - assertEquals(expectedXml, resultingXml); + assertEquals(xmlDef, expectedXml); } @Test public void memBalloonDefTestNone() { - String expectedXml = "\n"; + String expectedXml = "\n"; MemBalloonDef memBalloonDef = new MemBalloonDef(); memBalloonDef.defNoneMemBalloon(); - String resultingXml = memBalloonDef.toString(); + String xmlDef = memBalloonDef.toString(); - assertEquals(expectedXml, resultingXml); + assertEquals(xmlDef, expectedXml); } @Test public void memBalloonDefTestVirtio() { - String expectedXml = "\n\n"; + String expectedXml = "\n\n"; MemBalloonDef memBalloonDef = new MemBalloonDef(); memBalloonDef.defVirtioMemBalloon("60"); - String resultingXml = memBalloonDef.toString(); + String xmlDef = memBalloonDef.toString(); - assertEquals(expectedXml, resultingXml); + assertEquals(xmlDef, expectedXml); } @Test @@ -413,11 +413,11 @@ public void testRngDef() { int bytes = 2048; LibvirtVMDef.RngDef def = new LibvirtVMDef.RngDef(path, backendModel, bytes, period); - assertEquals(path, def.getPath()); - assertEquals(backendModel, def.getRngBackendModel()); - assertEquals(LibvirtVMDef.RngDef.RngModel.VIRTIO, def.getRngModel()); - assertEquals(bytes, def.getRngRateBytes()); - assertEquals(period, def.getRngRatePeriod()); + assertEquals(def.getPath(), path); + assertEquals(def.getRngBackendModel(), backendModel); + assertEquals(def.getRngModel(), LibvirtVMDef.RngDef.RngModel.VIRTIO); + assertEquals(def.getRngRateBytes(), bytes); + assertEquals(def.getRngRatePeriod(), period); } @Test @@ -441,8 +441,8 @@ public void testWatchDogDef() { LibvirtVMDef.WatchDogDef.WatchDogAction action = LibvirtVMDef.WatchDogDef.WatchDogAction.RESET; LibvirtVMDef.WatchDogDef def = new LibvirtVMDef.WatchDogDef(action, model); - assertEquals(model, def.getModel()); - assertEquals(action, def.getAction()); + assertEquals(def.getModel(), model); + assertEquals(def.getAction(), action); } @Test @@ -453,6 +453,6 @@ public void testSCSIDef() { "
\n" + "\n" + "\n"; - assertEquals(expected, str); + assertEquals(str, expected); } } diff --git a/server/src/main/java/com/cloud/hypervisor/KVMGuru.java b/server/src/main/java/com/cloud/hypervisor/KVMGuru.java index 7c02d95f3eb9..f0ccffbc719b 100644 --- a/server/src/main/java/com/cloud/hypervisor/KVMGuru.java +++ b/server/src/main/java/com/cloud/hypervisor/KVMGuru.java @@ -213,7 +213,7 @@ protected void configureVmMemoryAndCpuCores(VirtualMachineTO virtualMachineTo, H Integer maxHostCpuCore = max.second(); long minMemory = virtualMachineTo.getMinRam(); - Long maxMemory = virtualMachineTo.getMaxRam(); + Long maxMemory = minMemory; int minCpuCores = virtualMachineTo.getCpus(); Integer maxCpuCores = minCpuCores; From 2dcc4413049256c387f8616f185c0a6e8b750a22 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 3 Aug 2023 09:50:01 +0530 Subject: [PATCH 02/15] server: KVM guru should pass max memory as per VM transfer object This makes the KVM guru pass the max memory to KVM agent by picking the max. memory from the VM transfer object. This lets the cluster/agent decide if VM should start with the min or max memory depending on whether vm.memballoon.disable is false (default) or true. When `vm.memballoon.disable` is true, VMs start with max. memory. Right now the max memory isn't sent to the KVM agent causing the regression. Also fixes #7430 Signed-off-by: Rohit Yadav --- server/src/main/java/com/cloud/hypervisor/KVMGuru.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/hypervisor/KVMGuru.java b/server/src/main/java/com/cloud/hypervisor/KVMGuru.java index f0ccffbc719b..7c02d95f3eb9 100644 --- a/server/src/main/java/com/cloud/hypervisor/KVMGuru.java +++ b/server/src/main/java/com/cloud/hypervisor/KVMGuru.java @@ -213,7 +213,7 @@ protected void configureVmMemoryAndCpuCores(VirtualMachineTO virtualMachineTo, H Integer maxHostCpuCore = max.second(); long minMemory = virtualMachineTo.getMinRam(); - Long maxMemory = minMemory; + Long maxMemory = virtualMachineTo.getMaxRam(); int minCpuCores = virtualMachineTo.getCpus(); Integer maxCpuCores = minCpuCores; From a5cac4c9c6b623ae23893a2829d8b0aba0b4f5db Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 3 Aug 2023 17:29:20 +0530 Subject: [PATCH 03/15] disable vm ballooning feature by default Signed-off-by: Rohit Yadav --- agent/conf/agent.properties | 5 +++-- .../java/com/cloud/agent/properties/AgentProperties.java | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/agent/conf/agent.properties b/agent/conf/agent.properties index 27c0e387fa3c..327b883904bd 100644 --- a/agent/conf/agent.properties +++ b/agent/conf/agent.properties @@ -218,8 +218,9 @@ hypervisor.type=kvm #guest.cpu.features= # Disables memory ballooning on VM guests for overcommit. -# By default overcommit feature enables balloon and sets currentMemory to a minimum value. -#vm.memballoon.disable=false +# By default this feature is disabled, when enabled by setting the value to false +# the overcommit feature enables balloon and sets currentMemory to a minimum value. +#vm.memballoon.disable=true # The time interval (in seconds) at which the balloon driver will get memory stats updates. # This is equivalent to Libvirt's --period parameter when using the dommemstat command. diff --git a/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java b/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java index 5c7f4ed4b235..b2b4b7fa0967 100644 --- a/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java +++ b/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java @@ -395,11 +395,11 @@ public class AgentProperties{ /** * Disables memory ballooning on VM guests for overcommit.
- * By default overcommit feature enables balloon and sets currentMemory to a minimum value.
+ * By default the setting is disabled; when it's enabled overcommit feature enables balloon and sets currentMemory to a minimum value.
* Data type: Boolean.
- * Default value: false + * Default value: true */ - public static final Property VM_MEMBALLOON_DISABLE = new Property<>("vm.memballoon.disable", false); + public static final Property VM_MEMBALLOON_DISABLE = new Property<>("vm.memballoon.disable", true); /** * Set to true to check disk activity on VM's disks before starting a VM.
From 4f82fa6244ddc0e1f070dec96d7c7b8e72ffa894 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 3 Aug 2023 18:18:52 +0530 Subject: [PATCH 04/15] Update plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java Co-authored-by: dahn --- .../com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java index ec59265c8325..4ef1ec98891b 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java @@ -218,7 +218,7 @@ public void testDiskDef() { String expectedXml = "\n\n" + "\n\n\n"; - assertEquals(xmlDef, expectedXml); + assertEquals(expectedXml, xmlDef); } @Test From 757adfcf8920e57a9090204caaa73c24d16a5816 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 3 Aug 2023 18:19:03 +0530 Subject: [PATCH 05/15] Update plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java Co-authored-by: dahn --- .../com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java index 4ef1ec98891b..2549431ddb76 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java @@ -236,7 +236,7 @@ public void testDiskDefWithEncryption() { "\n" + "\n" + "\n"; - assertEquals(disk.toString(), expectedXML); + assertEquals(expectedXML, disk.toString()); } @Test From b31605926b481aa45df9da1ee5f9c380b9db49af Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 3 Aug 2023 18:19:10 +0530 Subject: [PATCH 06/15] Update plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java Co-authored-by: dahn --- .../com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java index 2549431ddb76..9129b0baff6e 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java @@ -367,7 +367,7 @@ public void memBalloonDefTestNone() { String xmlDef = memBalloonDef.toString(); - assertEquals(xmlDef, expectedXml); + assertEquals(expectedXml, xmlDef); } @Test From 90f9f5596ec506f8fa76c5f7e2435b32cf2ae73d Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 3 Aug 2023 18:19:23 +0530 Subject: [PATCH 07/15] Update plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java Co-authored-by: dahn --- .../com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java index 9129b0baff6e..4f880a8a584e 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java @@ -356,7 +356,7 @@ public void testDiskDefWithBurst() { ""+bytesReadRateMaxLength+"\n"+bytesWriteRateMaxLength+"\n" + ""+iopsReadRateMaxLength+"\n"+iopsWriteRateMaxLength+"\n\n\n"; - assertEquals(xmlDef, expectedXml); + assertEquals(expectedXml, xmlDef); } @Test From 6a14afd06f25621606e3a9d4fea6cb5bf8fd90d2 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 3 Aug 2023 18:19:35 +0530 Subject: [PATCH 08/15] Update plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java Co-authored-by: dahn --- .../com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java index 4f880a8a584e..c804e33632e8 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java @@ -378,7 +378,7 @@ public void memBalloonDefTestVirtio() { String xmlDef = memBalloonDef.toString(); - assertEquals(xmlDef, expectedXml); + assertEquals(expectedXml, xmlDef); } @Test From 63f3f5664b0db4c4f4d038d59c8cf9a1a02bb93f Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 3 Aug 2023 18:19:46 +0530 Subject: [PATCH 09/15] Update plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java Co-authored-by: dahn --- .../hypervisor/kvm/resource/LibvirtVMDefTest.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java index c804e33632e8..e41a343a5481 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java @@ -413,11 +413,11 @@ public void testRngDef() { int bytes = 2048; LibvirtVMDef.RngDef def = new LibvirtVMDef.RngDef(path, backendModel, bytes, period); - assertEquals(def.getPath(), path); - assertEquals(def.getRngBackendModel(), backendModel); - assertEquals(def.getRngModel(), LibvirtVMDef.RngDef.RngModel.VIRTIO); - assertEquals(def.getRngRateBytes(), bytes); - assertEquals(def.getRngRatePeriod(), period); + assertEquals(path, def.getPath()); + assertEquals(backendModel, def.getRngBackendModel()); + assertEquals(LibvirtVMDef.RngDef.RngModel.VIRTIO, def.getRngModel()); + assertEquals(bytes, def.getRngRateBytes()); + assertEquals(period, def.getRngRatePeriod()); } @Test From d6d25e95d8e747d242606998e83a130ff47d68e1 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 3 Aug 2023 18:19:52 +0530 Subject: [PATCH 10/15] Update plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java Co-authored-by: dahn --- .../com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java index e41a343a5481..f76ccc6dfb5d 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java @@ -453,6 +453,6 @@ public void testSCSIDef() { "
\n" + "\n" + "\n"; - assertEquals(str, expected); + assertEquals(expected, str); } } From 56fe36a3b698745c524e0bfcb9164eb549adcd50 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 3 Aug 2023 18:20:02 +0530 Subject: [PATCH 11/15] Update plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java Co-authored-by: dahn --- .../com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java index f76ccc6dfb5d..741f0a8c23dc 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java @@ -441,8 +441,8 @@ public void testWatchDogDef() { LibvirtVMDef.WatchDogDef.WatchDogAction action = LibvirtVMDef.WatchDogDef.WatchDogAction.RESET; LibvirtVMDef.WatchDogDef def = new LibvirtVMDef.WatchDogDef(action, model); - assertEquals(def.getModel(), model); - assertEquals(def.getAction(), action); + assertEquals(model, def.getModel()); + assertEquals(action, def.getAction()); } @Test From 037bcbe4c7f4fa0f05cf0d8a91ca075652ab555c Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 3 Aug 2023 18:21:46 +0530 Subject: [PATCH 12/15] Revert "disable vm ballooning feature by default" This reverts commit a5cac4c9c6b623ae23893a2829d8b0aba0b4f5db. --- agent/conf/agent.properties | 5 ++--- .../java/com/cloud/agent/properties/AgentProperties.java | 6 +++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/agent/conf/agent.properties b/agent/conf/agent.properties index 327b883904bd..27c0e387fa3c 100644 --- a/agent/conf/agent.properties +++ b/agent/conf/agent.properties @@ -218,9 +218,8 @@ hypervisor.type=kvm #guest.cpu.features= # Disables memory ballooning on VM guests for overcommit. -# By default this feature is disabled, when enabled by setting the value to false -# the overcommit feature enables balloon and sets currentMemory to a minimum value. -#vm.memballoon.disable=true +# By default overcommit feature enables balloon and sets currentMemory to a minimum value. +#vm.memballoon.disable=false # The time interval (in seconds) at which the balloon driver will get memory stats updates. # This is equivalent to Libvirt's --period parameter when using the dommemstat command. diff --git a/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java b/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java index b2b4b7fa0967..5c7f4ed4b235 100644 --- a/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java +++ b/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java @@ -395,11 +395,11 @@ public class AgentProperties{ /** * Disables memory ballooning on VM guests for overcommit.
- * By default the setting is disabled; when it's enabled overcommit feature enables balloon and sets currentMemory to a minimum value.
+ * By default overcommit feature enables balloon and sets currentMemory to a minimum value.
* Data type: Boolean.
- * Default value: true + * Default value: false */ - public static final Property VM_MEMBALLOON_DISABLE = new Property<>("vm.memballoon.disable", true); + public static final Property VM_MEMBALLOON_DISABLE = new Property<>("vm.memballoon.disable", false); /** * Set to true to check disk activity on VM's disks before starting a VM.
From 63cd85ff07292766b5e30846fbc20fbcc240ab0d Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 3 Aug 2023 21:58:54 +0530 Subject: [PATCH 13/15] add logging around vm memory ballooning; don't use min memory for system VMs Signed-off-by: Rohit Yadav --- .../kvm/resource/LibvirtComputingResource.java | 12 +++++++++--- .../cloud/hypervisor/kvm/resource/LibvirtVMDef.java | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 3f711fa9b809..194ea154944b 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -2795,11 +2795,17 @@ protected GuestResourceDef createGuestResourceDef(VirtualMachineTO vmTO){ } protected long getCurrentMemAccordingToMemBallooning(VirtualMachineTO vmTO, long 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)); + if (_noMemBalloon || (vmTO != null && vmTO.getType() != VirtualMachine.Type.User)) { + 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)); + } else { + s_logger.warn(String.format("Setting System VM's [%s] current memory as max memory [%s].", vmTO.toString(), maxRam)); + } return 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)); + return minRam; } } 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 b739c0ee0ab6..857559fe4009 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 @@ -282,7 +282,7 @@ public void setMemBalloning(boolean memoryBalloning) { @Override public String toString() { StringBuilder response = new StringBuilder(); - response.append(String.format("%s\n", this.currentMemory)); + response.append(String.format("%s\n", this.memory)); response.append(String.format("%s\n", this.currentMemory)); if (this.memory > this.currentMemory) { From 123b1e2b6def4e9c125bdcb5fc82829a2b9dd012 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 3 Aug 2023 22:55:28 +0530 Subject: [PATCH 14/15] fix unit tests and xml Signed-off-by: Rohit Yadav --- .../java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java | 2 +- .../hypervisor/kvm/resource/LibvirtComputingResourceTest.java | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) 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 857559fe4009..b739c0ee0ab6 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 @@ -282,7 +282,7 @@ public void setMemBalloning(boolean memoryBalloning) { @Override public String toString() { StringBuilder response = new StringBuilder(); - response.append(String.format("%s\n", this.memory)); + response.append(String.format("%s\n", this.currentMemory)); response.append(String.format("%s\n", this.currentMemory)); if (this.memory > this.currentMemory) { diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java index ffe20b4ad305..cc658ff8455c 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java @@ -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); From 14c83abca41651100683cd2e9ec9ba0483502a9f Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Fri, 4 Aug 2023 15:30:47 +0200 Subject: [PATCH 15/15] method cleanup --- .../kvm/resource/LibvirtComputingResource.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 194ea154944b..6fcb5469b6e9 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -2795,18 +2795,17 @@ protected GuestResourceDef createGuestResourceDef(VirtualMachineTO vmTO){ } protected long getCurrentMemAccordingToMemBallooning(VirtualMachineTO vmTO, long maxRam) { - if (_noMemBalloon || (vmTO != null && vmTO.getType() != VirtualMachine.Type.User)) { - 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)); - } else { - s_logger.warn(String.format("Setting System VM's [%s] current memory as max memory [%s].", vmTO.toString(), maxRam)); - } - return 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)); + } 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 { 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)); - return minRam; + retVal = minRam; } + return retVal; } /**