Update VM priority (cpu_shares) when live scaling it (#6031)

* Update VM priority (cpu_chares) when live scaling it

* Apply suggestions from code review

Co-authored-by: dahn <[email protected]>
Co-authored-by: Daniel Augusto Veronezi Salvador <[email protected]>

* Addressing javadoc review

* Addressing review typo

Co-authored-by: dahn <[email protected]>
Co-authored-by: Daniel Augusto Veronezi Salvador <[email protected]>

Author: 3 people

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,35 @@ 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 <br/>
+     * @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 {
+        for (SchedParameter c : dm.getSchedulerParameters()) {
+            if (c.field.equals("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 0;
+    }
+
+    /**
+     * Sets the cpu_shares (priority) of the running VM <br/>
+     * @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);
+    }
 }

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,22 @@ 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.
+     * 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
+     **/
+    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);
 

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;
@@ -5787,4 +5788,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;
+        }));
+    }
 }

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);
+    }
 }