You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "winterhazel (via GitHub)" <gi...@apache.org> on 2026/10/11 17:23:47 UTC
[PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
winterhazel opened a new pull request, #14383:
URL: https://github.com/apache/cloudstack/pull/14383
### Description
This PR is a minimal follow-up to #12190. A few refactorings and adjustments will be done in a separate PR targeting `main`.
#12190 changed the order in which the KVM Agent retrieves the CPU speed from `lscpu`, so that `CPU max MHz` is preferred over the frequency advertised in the model name. However, the host CPU speed is not only informative. It is used to:
- calculate the CPU capacity of the host (`cores * speed`);
- calculate the CPU quota of VMs that use a compute offering with CPU cap enabled (`VM speed / host speed`).
`CPU max MHz` may include the turbo frequency, which the CPU is not able to sustain; thus, it is not a suitable value for these calculations. Besides that, its meaning depends on the cpufreq driver in use: with `intel_pstate`/`amd-pstate` it is the turbo frequency, with `acpi-cpufreq` it is the base frequency plus 1 MHz, and without a cpufreq driver it is not reported at all.
As a consequence, the change impacts existing VMs with CPU cap enabled. Considering a host with a base frequency of 2.9 GHz and a maximum frequency of 4.8 GHz, a VM using a CPU cap enabled offering with 2.9 GHz does not have its CPU time limited. After #12190, once the Agent reconnects and the VM is started or migrated, it is able to use only ~60% of the CPU time, making users feel that the performance of their VMs degraded out of nowhere.
Therefore, this PR:
- restores the previous order of the `lscpu` checks to the same behavior that existed before #12190;
- exposes the maximum frequency of the CPU as a new host detail (`host.cpu.max.speed.mhz`), which is returned in the `details` of the `listHosts` API and shown in the details of the host in the UI as `CPU maximum frequency (turbo)`. This value is only informative; it is not used in capacity or CPU quota calculations.
Operators that want a host to be registered with a specific CPU speed, such as the turbo frequency, can still do so through the `host.cpu.manual.speed.mhz` property in `agent.properties`.
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
- [ ] Build/CI
- [ ] Test (unit or integration test code)
### Feature/Enhancement Scale or Bug Severity
#### Bug Severity
- [X] BLOCKER
- [ ] Critical
- [ ] Major
- [ ] Minor
- [ ] Trivial
### Screenshots (if appropriate):
### How Has This Been Tested?
Tests are still pending.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #14383:
URL: https://github.com/apache/cloudstack/pull/14383#issuecomment-6353671004
@winterhazel a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
Posted by "Damans227 (via GitHub)" <gi...@apache.org>.
Damans227 commented on code in PR #14383:
URL: https://github.com/apache/cloudstack/pull/14383#discussion_r4259070914
##########
plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/linux/KVMHostInfo.java:
##########
@@ -136,25 +146,30 @@ protected static long getCpuSpeed(final String cpabilities, final NodeInfo nodeI
return speed;
}
- private static long getCpuSpeedFromCommandLscpu() {
+ /**
+ * Retrieves the CPU speed, in MHz, from the "lscpu" command. The base frequency advertised in the model name is preferred because the CPU speed is used to
+ * calculate the capacity of the host and the CPU quota of the VMs with CPU cap enabled; thus, it must be a frequency that the CPU is able to sustain.
+ * "CPU max MHz" may include the turbo frequency; therefore, it is only used when the model name does not advertise a frequency.
+ */
+ protected static long getCpuSpeedFromCommandLscpu() {
long speed = 0L;
LOGGER.info("Fetching CPU speed from command \"lscpu\".");
try {
- String command = "lscpu | grep -i 'CPU max MHz' | head -n 1 | sed 's/^.*: //' | xargs";
+ String command = "lscpu | grep -i 'Model name' | head -n 1 | egrep -o '[[:digit:]].[[:digit:]]+GHz' | sed 's/GHz//g'";
+ if(isHostS390x()) {
+ command = "lscpu | grep 'CPU dynamic MHz' | cut -d ':' -f 2 | tr -d ' ' | awk '{printf \"%.1f\\n\", $1 / 1000}'";
+ }
String result = Script.runSimpleBashScript(command);
- speed = (long) (Float.parseFloat(result));
+ speed = (long) (Float.parseFloat(result) * 1000);
LOGGER.info(String.format("Command [%s] resulted in the value [%s] for CPU speed.", command, speed));
return speed;
} catch (NullPointerException | NumberFormatException e) {
LOGGER.error(String.format("Unable to retrieve the CPU speed from lscpu."), e);
}
try {
- String command = "lscpu | grep -i 'Model name' | head -n 1 | egrep -o '[[:digit:]].[[:digit:]]+GHz' | sed 's/GHz//g'";
- if(isHostS390x()) {
- command = "lscpu | grep 'CPU dynamic MHz' | cut -d ':' -f 2 | tr -d ' ' | awk '{printf \"%.1f\\n\", $1 / 1000}'";
- }
+ String command = LSCPU_CPU_MAX_MHZ_COMMAND;
Review Comment:
amd model names have no GHz in them (e.g. "AMD EPYC 7763 64-Core Processor"), so amd hosts still land here and get the boost clock as host speed. same as before #12190, so fine for this pr, but is it on the list for the main follow-up?
##########
ui/public/locales/en.json:
##########
@@ -1215,6 +1215,7 @@
"label.home": "Home",
"label.host": "IP address",
"label.host.alerts": "Hosts in alert state",
+"label.host.cpu.max.speed": "CPU maximum frequency (turbo)",
Review Comment:
with acpi-cpufreq this is base + 1 MHz, as the description says, so the ui would show the base clock labelled as turbo. maybe drop "(turbo)" and just say "CPU maximum frequency"?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez merged PR #14383:
URL: https://github.com/apache/cloudstack/pull/14383
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
Posted by "Damans227 (via GitHub)" <gi...@apache.org>.
Damans227 commented on code in PR #14383:
URL: https://github.com/apache/cloudstack/pull/14383#discussion_r4258826043
##########
plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/linux/KVMHostInfo.java:
##########
@@ -163,6 +178,16 @@ private static long getCpuSpeedFromCommandLscpu() {
return speed;
}
+ protected static long getCpuMaxSpeedFromCommandLscpu() {
Review Comment:
Script.runSimpleBashScript may return an empty string if lscpu has no CPU max MHz line. Float.parseFloat("") throws NumberFormatException, which is caught — but the debug log then prints the empty value. Is there value in logging this at all if it is only informational? An explicit empty check before parsing would also avoid the exception.
##########
plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/linux/KVMHostInfo.java:
##########
@@ -163,6 +178,16 @@ private static long getCpuSpeedFromCommandLscpu() {
return speed;
}
+ protected static long getCpuMaxSpeedFromCommandLscpu() {
+ String result = Script.runSimpleBashScript(LSCPU_CPU_MAX_MHZ_COMMAND);
+ try {
+ return (long) Float.parseFloat(result);
+ } catch (NullPointerException | NumberFormatException e) {
+ LOGGER.debug(String.format("Unable to retrieve the CPU max speed from lscpu; command [%s] resulted in the value [%s].", LSCPU_CPU_MAX_MHZ_COMMAND, result));
+ return 0L;
+ }
+ }
+
private static long getCpuSpeedFromFile() {
for (final String cpuInfoFreqFileName: cpuInfoFreqFileNames) {
Review Comment:
The catch block logs at DEBUG level while getCpuSpeedFromCommandLscpu() logs failures at ERROR. If a host genuinely cannot report CPU max MHz, this will be completely silent in production logs. Is that the right tradeoff — or should at least the empty-result case (no lscpu line at all) log at INFO to distinguish a missing lscpu output from a formatting issue?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on PR #14383:
URL: https://github.com/apache/cloudstack/pull/14383#issuecomment-6354960142
@blueorangutan test
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
Posted by "winterhazel (via GitHub)" <gi...@apache.org>.
winterhazel commented on PR #14383:
URL: https://github.com/apache/cloudstack/pull/14383#issuecomment-6111671547
@blueorangutan package
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
Posted by "winterhazel (via GitHub)" <gi...@apache.org>.
winterhazel commented on code in PR #14383:
URL: https://github.com/apache/cloudstack/pull/14383#discussion_r4354599366
##########
plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/linux/KVMHostInfo.java:
##########
@@ -136,25 +146,30 @@ protected static long getCpuSpeed(final String cpabilities, final NodeInfo nodeI
return speed;
}
- private static long getCpuSpeedFromCommandLscpu() {
+ /**
+ * Retrieves the CPU speed, in MHz, from the "lscpu" command. The base frequency advertised in the model name is preferred because the CPU speed is used to
+ * calculate the capacity of the host and the CPU quota of the VMs with CPU cap enabled; thus, it must be a frequency that the CPU is able to sustain.
+ * "CPU max MHz" may include the turbo frequency; therefore, it is only used when the model name does not advertise a frequency.
+ */
+ protected static long getCpuSpeedFromCommandLscpu() {
long speed = 0L;
LOGGER.info("Fetching CPU speed from command \"lscpu\".");
try {
- String command = "lscpu | grep -i 'CPU max MHz' | head -n 1 | sed 's/^.*: //' | xargs";
+ String command = "lscpu | grep -i 'Model name' | head -n 1 | egrep -o '[[:digit:]].[[:digit:]]+GHz' | sed 's/GHz//g'";
+ if(isHostS390x()) {
+ command = "lscpu | grep 'CPU dynamic MHz' | cut -d ':' -f 2 | tr -d ' ' | awk '{printf \"%.1f\\n\", $1 / 1000}'";
+ }
String result = Script.runSimpleBashScript(command);
- speed = (long) (Float.parseFloat(result));
+ speed = (long) (Float.parseFloat(result) * 1000);
LOGGER.info(String.format("Command [%s] resulted in the value [%s] for CPU speed.", command, speed));
return speed;
} catch (NullPointerException | NumberFormatException e) {
LOGGER.error(String.format("Unable to retrieve the CPU speed from lscpu."), e);
}
try {
- String command = "lscpu | grep -i 'Model name' | head -n 1 | egrep -o '[[:digit:]].[[:digit:]]+GHz' | sed 's/GHz//g'";
- if(isHostS390x()) {
- command = "lscpu | grep 'CPU dynamic MHz' | cut -d ':' -f 2 | tr -d ' ' | awk '{printf \"%.1f\\n\", $1 / 1000}'";
- }
+ String command = LSCPU_CPU_MAX_MHZ_COMMAND;
Review Comment:
Yes, see #14384.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #14383:
URL: https://github.com/apache/cloudstack/pull/14383#issuecomment-6112227267
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19514
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #14383:
URL: https://github.com/apache/cloudstack/pull/14383#issuecomment-6111686011
@winterhazel a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #14383:
URL: https://github.com/apache/cloudstack/pull/14383#issuecomment-6354973897
@nvazquez a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #14383:
URL: https://github.com/apache/cloudstack/pull/14383#discussion_r4359206136
##########
plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/linux/KVMHostInfo.java:
##########
@@ -136,25 +146,30 @@ protected static long getCpuSpeed(final String cpabilities, final NodeInfo nodeI
return speed;
}
- private static long getCpuSpeedFromCommandLscpu() {
+ /**
+ * Retrieves the CPU speed, in MHz, from the "lscpu" command. The base frequency advertised in the model name is preferred because the CPU speed is used to
+ * calculate the capacity of the host and the CPU quota of the VMs with CPU cap enabled; thus, it must be a frequency that the CPU is able to sustain.
+ * "CPU max MHz" may include the turbo frequency; therefore, it is only used when the model name does not advertise a frequency.
+ */
+ protected static long getCpuSpeedFromCommandLscpu() {
long speed = 0L;
LOGGER.info("Fetching CPU speed from command \"lscpu\".");
try {
- String command = "lscpu | grep -i 'CPU max MHz' | head -n 1 | sed 's/^.*: //' | xargs";
+ String command = "lscpu | grep -i 'Model name' | head -n 1 | egrep -o '[[:digit:]].[[:digit:]]+GHz' | sed 's/GHz//g'";
+ if(isHostS390x()) {
Review Comment:
```suggestion
if (isHostS390x()) {
```
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #14383:
URL: https://github.com/apache/cloudstack/pull/14383#issuecomment-6367232340
@vladimirpetrov a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #14383:
URL: https://github.com/apache/cloudstack/pull/14383#issuecomment-6111746697
## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/14383?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 4.04%. Comparing base ([`437adc8`](https://app.codecov.io/gh/apache/cloudstack/commit/437adc81dec08b2fbb6a57461126db56d1a20059?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`1f3f14c`](https://app.codecov.io/gh/apache/cloudstack/commit/1f3f14cdea8f2073100b6a74fbfd20b862ac0a5e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on 4.22.
> :exclamation: There is a different number of reports uploaded between BASE (437adc8) and HEAD (1f3f14c). Click for more details.
>
> <details><summary>HEAD has 1 upload less than BASE</summary>
>
>| Flag | BASE (437adc8) | HEAD (1f3f14c) |
>|------|------|------|
>|unittests|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 4.22 #14383 +/- ##
=============================================
- Coverage 18.02% 4.04% -13.99%
=============================================
Files 5936 449 -5487
Lines 535823 38240 -497583
Branches 65612 7082 -58530
=============================================
- Hits 96572 1546 -95026
+ Misses 428252 36482 -391770
+ Partials 10999 212 -10787
```
| [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/14383/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/14383/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.04% <ø> (ø)` | |
| [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/14383/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/cloudstack/pull/14383?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #14383:
URL: https://github.com/apache/cloudstack/pull/14383#issuecomment-6354497631
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19693
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #14383:
URL: https://github.com/apache/cloudstack/pull/14383#issuecomment-6377609958
<b>[SF] Trillian test result (tid-17221)</b>
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 63560 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr14383-t17221-kvm-ol8.zip
Smoke tests completed. 149 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
Posted by "winterhazel (via GitHub)" <gi...@apache.org>.
winterhazel commented on PR #14383:
URL: https://github.com/apache/cloudstack/pull/14383#issuecomment-6353662357
@blueorangutan package
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
Posted by "vladimirpetrov (via GitHub)" <gi...@apache.org>.
vladimirpetrov commented on PR #14383:
URL: https://github.com/apache/cloudstack/pull/14383#issuecomment-6367204164
@blueorangutan package
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on PR #14383:
URL: https://github.com/apache/cloudstack/pull/14383#issuecomment-6387727175
Merging based on approvals and test results, thanks for the fix @winterhazel
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
Posted by "Damans227 (via GitHub)" <gi...@apache.org>.
Damans227 commented on code in PR #14383:
URL: https://github.com/apache/cloudstack/pull/14383#discussion_r4258826043
##########
plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/linux/KVMHostInfo.java:
##########
@@ -163,6 +178,16 @@ private static long getCpuSpeedFromCommandLscpu() {
return speed;
}
+ protected static long getCpuMaxSpeedFromCommandLscpu() {
Review Comment:
Script.runSimpleBashScript may return an empty string if lscpu has no CPU max MHz line. Float.parseFloat("") throws NumberFormatException, which is caught — but the debug log then prints the empty value. Is there value in logging this at all if it is only informational? An explicit empty check before parsing would also avoid the exception.
##########
plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/linux/KVMHostInfo.java:
##########
@@ -163,6 +178,16 @@ private static long getCpuSpeedFromCommandLscpu() {
return speed;
}
+ protected static long getCpuMaxSpeedFromCommandLscpu() {
+ String result = Script.runSimpleBashScript(LSCPU_CPU_MAX_MHZ_COMMAND);
+ try {
+ return (long) Float.parseFloat(result);
+ } catch (NullPointerException | NumberFormatException e) {
+ LOGGER.debug(String.format("Unable to retrieve the CPU max speed from lscpu; command [%s] resulted in the value [%s].", LSCPU_CPU_MAX_MHZ_COMMAND, result));
+ return 0L;
+ }
+ }
+
private static long getCpuSpeedFromFile() {
for (final String cpuInfoFreqFileName: cpuInfoFreqFileNames) {
Review Comment:
The catch block logs at DEBUG level while getCpuSpeedFromCommandLscpu() logs failures at ERROR. If a host genuinely cannot report CPU max MHz, this will be completely silent in production logs. Is that the right tradeoff — or should at least the empty-result case (no lscpu line at all) log at INFO to distinguish a missing lscpu output from a formatting issue?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
Posted by "Damans227 (via GitHub)" <gi...@apache.org>.
Damans227 commented on code in PR #14383:
URL: https://github.com/apache/cloudstack/pull/14383#discussion_r4258827954
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -4262,6 +4263,9 @@ public StartupCommand[] initialize() {
new StartupRoutingCommand(info.getAllocatableCpus(), info.getCpuSpeed(), info.getTotalMemory(), info.getReservedMemory(), capabilities, hypervisorType,
RouterPrivateIpStrategy.HostLocal);
cmd.setCpuSockets(info.getCpuSockets());
+ if (info.getCpuMaxSpeed() > 0) {
+ cmd.getHostDetails().put(HOST_CPU_MAX_SPEED_MHZ, String.valueOf(info.getCpuMaxSpeed()));
Review Comment:
This is set in initialize() — runs once on agent connection. If a VM on this host has CPU cap enabled, the host cpuSpeed is used to calculate the quota. Since getCpuSpeed() also only refreshes on reconnect, both values share the same one-shot behavior. Is there any case where CPU max speed could diverge from cpuSpeed in a way that makes the detail misleading? E.g. a host with no turbo boost reported.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
Posted by "Damans227 (via GitHub)" <gi...@apache.org>.
Damans227 commented on code in PR #14383:
URL: https://github.com/apache/cloudstack/pull/14383#discussion_r4258827954
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -4262,6 +4263,9 @@ public StartupCommand[] initialize() {
new StartupRoutingCommand(info.getAllocatableCpus(), info.getCpuSpeed(), info.getTotalMemory(), info.getReservedMemory(), capabilities, hypervisorType,
RouterPrivateIpStrategy.HostLocal);
cmd.setCpuSockets(info.getCpuSockets());
+ if (info.getCpuMaxSpeed() > 0) {
+ cmd.getHostDetails().put(HOST_CPU_MAX_SPEED_MHZ, String.valueOf(info.getCpuMaxSpeed()));
Review Comment:
This is set in initialize() — runs once on agent connection. If a VM on this host has CPU cap enabled, the host cpuSpeed is used to calculate the quota. Since getCpuSpeed() also only refreshes on reconnect, both values share the same one-shot behavior. Is there any case where CPU max speed could diverge from cpuSpeed in a way that makes the detail misleading? E.g. a host with no turbo boost reported.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] kvm: use the base CPU frequency as the host speed and expose the maximum frequency as a host detail [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #14383:
URL: https://github.com/apache/cloudstack/pull/14383#issuecomment-6368589744
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19706
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org