DaanHoogland commented on a change in pull request #6031:
URL: https://github.com/apache/cloudstack/pull/6031#discussion_r812620610
##########
File path:
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapper.java
##########
@@ -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.
Review comment:
I don't understand this sentence; is it updated manually or on restart?
Restart seems clear, but can the update be forced manually or is the restart
manually?
Also, the sentence below sugests that this is no longer true. I think this
needs a re-formulation. Something like "To prevent that the cpu_shares value is
only updated in circumstances A and B, this method ....."
##########
File path:
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapper.java
##########
@@ -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) {
Review comment:
only up, never down? is this like `nice` for users or can the root user
force something?
##########
File path:
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -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 <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 {
+ 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;
Review comment:
not vital but as we are not aggregating no need for a var:
```suggestion
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 o;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
