phsm commented on code in PR #10221:
URL: https://github.com/apache/cloudstack/pull/10221#discussion_r1926963411
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -2796,12 +2800,23 @@ public int calculateCpuShares(VirtualMachineTO vmTO) {
if (hostCpuMaxCapacity > 0) {
int updatedCpuShares = (int) Math.ceil((requestedCpuShares *
CGROUP_V2_UPPER_LIMIT) / (double) hostCpuMaxCapacity);
- LOGGER.debug(String.format("This host utilizes cgroupv2 (as the
max shares value is [%s]), thus, the VM requested shares of [%s] will be
converted to " +
- "consider the host limits; the new CPU shares value is
[%s].", hostCpuMaxCapacity, requestedCpuShares, updatedCpuShares));
- return updatedCpuShares;
- }
- LOGGER.debug(String.format("This host does not have a maximum CPU
shares set; therefore, this host utilizes cgroupv1 and the VM requested CPU
shares [%s] will not be " +
- "converted.", requestedCpuShares));
+
+ LOGGER.debug("This host utilizes cgroupv2 (as the max shares value
is [{}]), thus, the VM requested shares of [{}] will be converted to " +
+ "consider the host limits; the new CPU shares value is
[{}].", hostCpuMaxCapacity, requestedCpuShares, updatedCpuShares);
+ requestedCpuShares = updatedCpuShares;
+ } else {
+ LOGGER.debug("This host does not have a maximum CPU shares set;
therefore, this host utilizes cgroupv1 and the VM requested CPU shares [{}]
will not be " +
+ "converted.", requestedCpuShares);
+ }
+
+ /**
+ * Libvirt < 9.1.0 enforces the same value range to both cgroupv1 and
cgroupv2.
+ * Therefore, if the shares value is determined to be outside of
boundaries,
+ * then bring it to the minimum or maximum allowed value.
+ * See:
https://github.com/libvirt/libvirt/commit/38af6497610075e5fe386734b87186731d4c17ac
+ */
+ if (requestedCpuShares < LIBVIRT_CGROUPV1_CPU_SHARES_MIN)
requestedCpuShares = LIBVIRT_CGROUPV1_CPU_SHARES_MIN;
Review Comment:
Well, not quite.
The root cause of the bug is that the older libvirt versions apply cgroupv1
limits checks to cgroupv2 systems. Libvirt still uses cgroupv2 for the VMs in
this case.
So if we apply correctness checks "by the book", then the this will happen
in the following situation:
1. The CPU overprovisioning factor is set to some very high value
2. The VM CPU service offering is set to something small.
3. Then Cloudstack will check that the target system uses cgroupv2, which is
correct.
4. The CPU shares formula will produce the shares value 1. Which is also an
acceptable value according to cgroupv2 allowed range.
5. Libvirt will erroneously check the value against the cgroupv1 range,
which is 2-262144.
6. Libvirt will return an error.
So I guess the proper compromise could be this:
```
if cgroupv2 {
<adjust the value to range 2-10000>
} else {
<adjust the value to 2-262144>
}
```
Basically, we need to exclude the value 1 from cgroupv2 to always conform
that buggy libvirt check
--
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]