From f23ecbca502ed290aae0cb856a6c1061a244732e Mon Sep 17 00:00:00 2001 From: Bryan Lima Date: Tue, 22 Feb 2022 13:25:30 -0300 Subject: [PATCH 1/4] Update VM priority (cpu_chares) when live scaling it --- .../resource/LibvirtComputingResource.java | 37 ++++++++++++- .../wrapper/LibvirtScaleVmCommandWrapper.java | 22 +++++++- .../LibvirtComputingResourceTest.java | 54 +++++++++++++++++++ .../LibvirtScaleVmCommandWrapperTest.java | 39 +++++++++++++- 4 files changed, 149 insertions(+), 3 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 f1c0ce8153c9..b2ba16b6ea30 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 @@ -77,6 +77,9 @@ import org.libvirt.LibvirtException; import org.libvirt.MemoryStatistic; import org.libvirt.Network; +import org.libvirt.SchedParameter; +import org.libvirt.SchedUlongParameter; +import org.libvirt.VcpuInfo; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.Node; @@ -186,7 +189,6 @@ import com.cloud.vm.VmDetailConstants; import org.apache.commons.lang3.StringUtils; import org.apache.cloudstack.utils.bytescale.ByteScaleUtils; -import org.libvirt.VcpuInfo; /** * LibvirtComputingResource execute requests on the computing/routing host using @@ -4621,4 +4623,37 @@ public static long countDomainRunningVcpus(Domain dm) throws LibvirtException { VcpuInfo vcpus[] = dm.getVcpusInfo(); return Arrays.stream(vcpus).filter(vcpu -> vcpu.state.equals(VcpuInfo.VcpuState.VIR_VCPU_RUNNING)).count(); } + + /** + * Retrieves the cpu_shares (priority) of the running VM
+ * @param dm domain of the VM. + * @return the value of cpu_shares of the running VM. + * @throws org.libvirt.LibvirtException + **/ + public static Integer getCpuShares(Domain dm) throws LibvirtException { + int cpu_shares = 0; + for (SchedParameter c : dm.getSchedulerParameters()) { + if (c.field.equals("cpu_shares")) { + cpu_shares = Integer.parseInt(c.getValueAsString()); + return cpu_shares; + } + } + s_logger.warn(String.format("Could not get cpu_shares of domain: [%s]. Returning default value of 0. ", dm.getName())); + return cpu_shares; + } + + /** + * Set the cpu_shares (priority) of the running VM
+ * @param dm domain of the VM. + * @param cpuShares new priority of the running VM. + * @throws org.libvirt.LibvirtException + **/ + public static void setCpuShares(Domain dm, Integer cpuShares) throws LibvirtException { + SchedUlongParameter[] params = new SchedUlongParameter[1]; + params[0] = new SchedUlongParameter(); + params[0].field = "cpu_shares"; + params[0].value = cpuShares; + + dm.setSchedulerParameters(params); + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapper.java index 384d5cc8b151..6723f06a5f05 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapper.java @@ -39,8 +39,10 @@ public Answer execute(ScaleVmCommand command, LibvirtComputingResource libvirtCo long newMemory = ByteScaleUtils.bytesToKib(vmSpec.getMaxRam()); int newVcpus = vmSpec.getCpus(); + int newCpuSpeed = vmSpec.getMinSpeed() != null ? vmSpec.getMinSpeed() : vmSpec.getSpeed(); + int newCpuShares = newVcpus * newCpuSpeed; String vmDefinition = vmSpec.toString(); - String scalingDetails = String.format("%s memory to [%s KiB] and CPU cores to [%s]", vmDefinition, newMemory, newVcpus); + String scalingDetails = String.format("%s memory to [%s KiB], CPU cores to [%s] and cpu_shares to [%s]", vmDefinition, newMemory, newVcpus, newCpuShares); try { LibvirtUtilitiesHelper libvirtUtilitiesHelper = libvirtComputingResource.getLibvirtUtilitiesHelper(); @@ -51,6 +53,7 @@ public Answer execute(ScaleVmCommand command, LibvirtComputingResource libvirtCo logger.debug(String.format("Scaling %s.", scalingDetails)); scaleMemory(dm, newMemory, vmDefinition); scaleVcpus(dm, newVcpus, vmDefinition); + updateCpuShares(dm, newCpuShares); return new ScaleVmAnswer(command, true, String.format("Successfully scaled %s.", scalingDetails)); } catch (LibvirtException | CloudRuntimeException e) { @@ -68,6 +71,23 @@ public Answer execute(ScaleVmCommand command, LibvirtComputingResource libvirtCo } } + /** + * Set the cpu_shares (priority) of the running VM. This is necessary because the priority is only calculated when deploying the VM. + * When the number of cores and/or speed of the CPU is changed the cpu_shares is only updated if the VM is restarted or manually, using the command schedinfo. + * To correct this behaviour when live scaling, this command changes the cpu_shares of a running VM. + * @param dm domain of the VM. + * @param newCpuShares new priority of the running VM. + * @throws org.libvirt.LibvirtException + **/ + protected void updateCpuShares(Domain dm, int newCpuShares) throws LibvirtException { + int oldCpuShares = LibvirtComputingResource.getCpuShares(dm); + + if (oldCpuShares < newCpuShares) { + LibvirtComputingResource.setCpuShares(dm, newCpuShares); + logger.info(String.format("Successfully increased cpu_shares of VM [%s] from [%s] to [%s].", dm.getName(), oldCpuShares, newCpuShares)); + } + } + protected void scaleVcpus(Domain dm, int newVcpus, String vmDefinition) throws LibvirtException { long runningVcpus = LibvirtComputingResource.countDomainRunningVcpus(dm); 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 1f6545881795..72485d384cbb 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 @@ -74,6 +74,7 @@ import org.libvirt.LibvirtException; import org.libvirt.MemoryStatistic; import org.libvirt.NodeInfo; +import org.libvirt.SchedUlongParameter; import org.libvirt.StorageVol; import org.libvirt.jna.virDomainMemoryStats; import org.mockito.BDDMockito; @@ -5784,4 +5785,57 @@ private DiskDef configureAndTestSetDiskIoDriverTest(long hypervisorLibvirtVersio libvirtComputingResourceSpy.setDiskIoDriver(diskDef); return diskDef; } + + private SchedUlongParameter[] createSchedParametersWithCpuSharesOf2000 () { + SchedUlongParameter[] params = new SchedUlongParameter[1]; + params[0] = new SchedUlongParameter(); + params[0].field = "cpu_shares"; + params[0].value = 2000; + + return params; + } + + private SchedUlongParameter[] createSchedParametersWithoutCpuShares () { + SchedUlongParameter[] params = new SchedUlongParameter[1]; + params[0] = new SchedUlongParameter(); + params[0].field = "weight"; + params[0].value = 200; + + return params; + } + + @Test + public void getCpuSharesTestReturnCpuSharesIfFound() throws LibvirtException { + SchedUlongParameter[] cpuSharesOf2000 = createSchedParametersWithCpuSharesOf2000(); + + Mockito.when(domainMock.getSchedulerParameters()).thenReturn(cpuSharesOf2000); + int cpuShares = LibvirtComputingResource.getCpuShares(domainMock); + + Assert.assertEquals(2000, cpuShares); + } + + @Test + public void getCpuSharesTestReturnZeroIfCpuSharesNotFound() throws LibvirtException { + SchedUlongParameter[] withoutCpuShares = createSchedParametersWithoutCpuShares(); + + Mockito.when(domainMock.getSchedulerParameters()).thenReturn(withoutCpuShares); + int actualValue = LibvirtComputingResource.getCpuShares(domainMock); + + Assert.assertEquals(0, actualValue); + } + + @Test + public void setCpuSharesTestSuccessfullySetCpuShares() throws LibvirtException { + LibvirtComputingResource.setCpuShares(domainMock, 2000); + Mockito.verify(domainMock, times(1)).setSchedulerParameters(Mockito.argThat(schedParameters -> { + if (schedParameters == null || schedParameters.length > 1 || !(schedParameters[0] instanceof SchedUlongParameter)) { + return false; + } + SchedUlongParameter param = (SchedUlongParameter) schedParameters[0]; + if (param.field != "cpu_shares" || param.value != 2000) { + return false; + } + return true; + })); + } } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapperTest.java index a0851e747f12..6079a70ddd14 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapperTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapperTest.java @@ -78,7 +78,8 @@ public void init() { long memory = ByteScaleUtils.bytesToKib(vmTo.getMaxRam()); int vcpus = vmTo.getCpus(); - scalingDetails = String.format("%s memory to [%s KiB] and CPU cores to [%s]", vmTo.toString(), memory, vcpus); + int cpuShares = vcpus * vmTo.getSpeed(); + scalingDetails = String.format("%s memory to [%s KiB], CPU cores to [%s] and cpu_shares to [%s]", vmTo.toString(), memory, vcpus, cpuShares); PowerMockito.mockStatic(LibvirtComputingResource.class); } @@ -241,4 +242,40 @@ public void validateExecuteThrowAnyOtherException() { libvirtScaleVmCommandWrapperSpy.execute(scaleVmCommandMock, libvirtComputingResourceMock); } + + @Test + public void updateCpuSharesTestOldSharesLessThanNewSharesUpdateShares() throws LibvirtException { + int oldShares = 2000; + int newShares = 3000; + + PowerMockito.when(LibvirtComputingResource.getCpuShares(Mockito.any())).thenReturn(oldShares); + libvirtScaleVmCommandWrapperSpy.updateCpuShares(domainMock, newShares); + + PowerMockito.verifyStatic(LibvirtComputingResource.class, Mockito.times(1)); + libvirtComputingResourceMock.setCpuShares(domainMock, newShares); + } + + @Test + public void updateCpuSharesTestOldSharesHigherThanNewSharesDoNothing() throws LibvirtException { + int oldShares = 3000; + int newShares = 2000; + + PowerMockito.when(LibvirtComputingResource.getCpuShares(Mockito.any())).thenReturn(oldShares); + libvirtScaleVmCommandWrapperSpy.updateCpuShares(domainMock, newShares); + + PowerMockito.verifyStatic(LibvirtComputingResource.class, Mockito.times(0)); + libvirtComputingResourceMock.setCpuShares(domainMock, newShares); + } + + @Test + public void updateCpuSharesTestOldSharesEqualsNewSharesDoNothing() throws LibvirtException { + int oldShares = 2000; + int newShares = 2000; + + PowerMockito.when(LibvirtComputingResource.getCpuShares(Mockito.any())).thenReturn(oldShares); + libvirtScaleVmCommandWrapperSpy.updateCpuShares(domainMock, newShares); + + PowerMockito.verifyStatic(LibvirtComputingResource.class, Mockito.times(0)); + libvirtComputingResourceMock.setCpuShares(domainMock, newShares); + } } From a5f7a5f093411529e18923fc128a97bfad30b8ea Mon Sep 17 00:00:00 2001 From: Bryan Lima <42067040+BryanMLima@users.noreply.github.com> Date: Wed, 23 Feb 2022 11:50:15 -0300 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: dahn Co-authored-by: Daniel Augusto Veronezi Salvador <38945620+GutoVeronezi@users.noreply.github.com> --- .../hypervisor/kvm/resource/LibvirtComputingResource.java | 8 +++----- .../resource/wrapper/LibvirtScaleVmCommandWrapper.java | 2 +- 2 files changed, 4 insertions(+), 6 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 b2ba16b6ea30..6ce23f1d7489 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 @@ -4631,19 +4631,17 @@ public static long countDomainRunningVcpus(Domain dm) throws LibvirtException { * @throws org.libvirt.LibvirtException **/ public static Integer getCpuShares(Domain dm) throws LibvirtException { - int cpu_shares = 0; for (SchedParameter c : dm.getSchedulerParameters()) { if (c.field.equals("cpu_shares")) { - cpu_shares = Integer.parseInt(c.getValueAsString()); - return cpu_shares; + return Integer.parseInt(c.getValueAsString()); } } s_logger.warn(String.format("Could not get cpu_shares of domain: [%s]. Returning default value of 0. ", dm.getName())); - return cpu_shares; + return o; } /** - * Set the cpu_shares (priority) of the running VM
+ * Sets the cpu_shares (priority) of the running VM
* @param dm domain of the VM. * @param cpuShares new priority of the running VM. * @throws org.libvirt.LibvirtException diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapper.java index 6723f06a5f05..e0e4f88bc55c 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapper.java @@ -72,7 +72,7 @@ public Answer execute(ScaleVmCommand command, LibvirtComputingResource libvirtCo } /** - * Set the cpu_shares (priority) of the running VM. This is necessary because the priority is only calculated when deploying the VM. + * Sets the cpu_shares (priority) of the running VM. This is necessary because the priority is only calculated when deploying the VM. * When the number of cores and/or speed of the CPU is changed the cpu_shares is only updated if the VM is restarted or manually, using the command schedinfo. * To correct this behaviour when live scaling, this command changes the cpu_shares of a running VM. * @param dm domain of the VM. From 11213491825a5b3a59991f762ddfedacdd9ad227 Mon Sep 17 00:00:00 2001 From: Bryan Lima Date: Wed, 23 Feb 2022 12:22:58 -0300 Subject: [PATCH 3/4] Addressing javadoc review --- .../kvm/resource/wrapper/LibvirtScaleVmCommandWrapper.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapper.java index e0e4f88bc55c..96c3e844abfe 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapper.java @@ -73,8 +73,7 @@ public Answer execute(ScaleVmCommand command, LibvirtComputingResource libvirtCo /** * Sets the cpu_shares (priority) of the running VM. This is necessary because the priority is only calculated when deploying the VM. - * When the number of cores and/or speed of the CPU is changed the cpu_shares is only updated if the VM is restarted or manually, using the command schedinfo. - * To correct this behaviour when live scaling, this command changes the cpu_shares of a running VM. + * To prevent the cpu_shares to be manually updated by using the command virsh schedinfo or restarting the VM. This method updates the cpu_shares of a running VM on the fly. * @param dm domain of the VM. * @param newCpuShares new priority of the running VM. * @throws org.libvirt.LibvirtException From bc0b698fa1ee1c52bece171fe05ca2736763620c Mon Sep 17 00:00:00 2001 From: Bryan Lima Date: Wed, 23 Feb 2022 16:06:48 -0300 Subject: [PATCH 4/4] Addressing review typo --- .../cloud/hypervisor/kvm/resource/LibvirtComputingResource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 6ce23f1d7489..3706fb37719a 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 @@ -4637,7 +4637,7 @@ public static Integer getCpuShares(Domain dm) throws LibvirtException { } } s_logger.warn(String.format("Could not get cpu_shares of domain: [%s]. Returning default value of 0. ", dm.getName())); - return o; + return 0; } /**