[PATCH] util: virHostCPUGetStatsLinux: add guest_nice to VIR_NODE_CPU_STATS_GUEST
Fix VIR_NODE_CPU_STATS_GUEST to also account for guest nice time, similarly to how VIR_NODE_CPU_STATS_USER also accounts for nice time. VIR_NODE_CPU_STATS_GUEST was introduced in: b5878727c714 ("util: virHostCPUGetStatsLinux: support VIR_NODE_CPU_STATS_GUEST") Extend the tests artificially to capture this specific case. Fixes: b5878727c714d813c820ad4a1b695fbbb5ffc84e Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/util/virhostcpu.c | 2 +- tests/virhostcpudata/linux-cpustat-24cpu.out | 8 ++++---- tests/virhostcpudata/linux-cpustat-24cpu.stat | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 1db096ba24..f2b25e940b 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -882,7 +882,7 @@ virHostCPUGetStatsLinux(FILE *procstat, return -1; if (virHostCPUStatsAssign(¶ms[4], VIR_NODE_CPU_STATS_GUEST, - guest * TICK_TO_NSEC) < 0) + (guest + guest_nice) * TICK_TO_NSEC) < 0) return -1; return 0; } diff --git a/tests/virhostcpudata/linux-cpustat-24cpu.out b/tests/virhostcpudata/linux-cpustat-24cpu.out index f3183f5c41..a975702a06 100644 --- a/tests/virhostcpudata/linux-cpustat-24cpu.out +++ b/tests/virhostcpudata/linux-cpustat-24cpu.out @@ -1,16 +1,16 @@ cpu: kernel: 8751170 user: 14128079 -idle: 1816344522 +idle: 1816344521 iowait: 81323 -guest: 5880634 +guest: 5880635 cpu0: kernel: 447603 user: 749021 -idle: 75399242 +idle: 75399241 iowait: 5295 -guest: 331814 +guest: 331815 cpu1: kernel: 167215 diff --git a/tests/virhostcpudata/linux-cpustat-24cpu.stat b/tests/virhostcpudata/linux-cpustat-24cpu.stat index bc9d449e1c..e13035b5f0 100644 --- a/tests/virhostcpudata/linux-cpustat-24cpu.stat +++ b/tests/virhostcpudata/linux-cpustat-24cpu.stat @@ -1,5 +1,5 @@ -cpu 14126233 1846 7764352 1816344522 81323 395581 591237 0 5880634 0 -cpu0 748997 24 320851 75399242 5295 22050 104702 0 331814 0 +cpu 14126233 1846 7764352 1816344521 81323 395581 591237 0 5880634 1 +cpu0 748997 24 320851 75399241 5295 22050 104702 0 331814 1 cpu1 337325 1 140909 76178612 1121 8962 17344 0 166726 0 cpu2 666860 29 269302 75649696 4298 18473 21155 0 272094 0 cpu3 328387 77 211400 76131634 1219 9701 6573 0 115551 0 -- 2.35.3
On Mon, Jun 15, 2026 at 10:34:54 +0200, Claudio Fontana wrote:
Fix VIR_NODE_CPU_STATS_GUEST to also account for guest nice time, similarly to how VIR_NODE_CPU_STATS_USER also accounts for nice time.
Can you elaborate a bit what the problem is ... ideally in the commit message.
VIR_NODE_CPU_STATS_GUEST was introduced in: b5878727c714 ("util: virHostCPUGetStatsLinux: support VIR_NODE_CPU_STATS_GUEST")
Extend the tests artificially to capture this specific case.
Fixes: b5878727c714d813c820ad4a1b695fbbb5ffc84e
v11.7.0-2-gb5878727c7
Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/util/virhostcpu.c | 2 +- tests/virhostcpudata/linux-cpustat-24cpu.out | 8 ++++---- tests/virhostcpudata/linux-cpustat-24cpu.stat | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 1db096ba24..f2b25e940b 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -882,7 +882,7 @@ virHostCPUGetStatsLinux(FILE *procstat, return -1;
if (virHostCPUStatsAssign(¶ms[4], VIR_NODE_CPU_STATS_GUEST, - guest * TICK_TO_NSEC) < 0) + (guest + guest_nice) * TICK_TO_NSEC) < 0) return -1; return 0; }
Shouldn't we rather introduce a new field rather than modify what the old one does?
diff --git a/tests/virhostcpudata/linux-cpustat-24cpu.out b/tests/virhostcpudata/linux-cpustat-24cpu.out index f3183f5c41..a975702a06 100644 --- a/tests/virhostcpudata/linux-cpustat-24cpu.out +++ b/tests/virhostcpudata/linux-cpustat-24cpu.out @@ -1,16 +1,16 @@ cpu: kernel: 8751170 user: 14128079 -idle: 1816344522 +idle: 1816344521 iowait: 81323 -guest: 5880634 +guest: 5880635
cpu0: kernel: 447603 user: 749021 -idle: 75399242 +idle: 75399241 iowait: 5295 -guest: 331814 +guest: 331815
cpu1: kernel: 167215 diff --git a/tests/virhostcpudata/linux-cpustat-24cpu.stat b/tests/virhostcpudata/linux-cpustat-24cpu.stat index bc9d449e1c..e13035b5f0 100644 --- a/tests/virhostcpudata/linux-cpustat-24cpu.stat +++ b/tests/virhostcpudata/linux-cpustat-24cpu.stat @@ -1,5 +1,5 @@ -cpu 14126233 1846 7764352 1816344522 81323 395581 591237 0 5880634 0 -cpu0 748997 24 320851 75399242 5295 22050 104702 0 331814 0 +cpu 14126233 1846 7764352 1816344521 81323 395581 591237 0 5880634 1 +cpu0 748997 24 320851 75399241 5295 22050 104702 0 331814 1 cpu1 337325 1 140909 76178612 1121 8962 17344 0 166726 0 cpu2 666860 29 269302 75649696 4298 18473 21155 0 272094 0 cpu3 328387 77 211400 76131634 1219 9701 6573 0 115551 0 -- 2.35.3
Hello Peter, On 6/15/26 10:47, Peter Krempa wrote:
On Mon, Jun 15, 2026 at 10:34:54 +0200, Claudio Fontana wrote:
Fix VIR_NODE_CPU_STATS_GUEST to also account for guest nice time, similarly to how VIR_NODE_CPU_STATS_USER also accounts for nice time.
Can you elaborate a bit what the problem is ... ideally in the commit message.
Well it is there, the problem is that as I introduced this last year I did not account for guest nice time. Hopefully we can still fix it as maybe no-one is relying on this yet..
VIR_NODE_CPU_STATS_GUEST was introduced in: b5878727c714 ("util: virHostCPUGetStatsLinux: support VIR_NODE_CPU_STATS_GUEST")
Extend the tests artificially to capture this specific case.
The trap was that the system on which the tests were likely generated did not include any guest_nice time, so tests seemed to pass. But now as I revisited the issue I noticed the missing guest_nice.
Fixes: b5878727c714d813c820ad4a1b695fbbb5ffc84e
v11.7.0-2-gb5878727c7
Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/util/virhostcpu.c | 2 +- tests/virhostcpudata/linux-cpustat-24cpu.out | 8 ++++---- tests/virhostcpudata/linux-cpustat-24cpu.stat | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 1db096ba24..f2b25e940b 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -882,7 +882,7 @@ virHostCPUGetStatsLinux(FILE *procstat, return -1;
if (virHostCPUStatsAssign(¶ms[4], VIR_NODE_CPU_STATS_GUEST, - guest * TICK_TO_NSEC) < 0) + (guest + guest_nice) * TICK_TO_NSEC) < 0) return -1; return 0; }
Shouldn't we rather introduce a new field rather than modify what the old one does?
I hope it's early enough that not many users are relying on VIR_NODE_CPU_STATS_GUEST , and accounting it together would be in line with what is done for the other stats. Thanks, Claudio
diff --git a/tests/virhostcpudata/linux-cpustat-24cpu.out b/tests/virhostcpudata/linux-cpustat-24cpu.out index f3183f5c41..a975702a06 100644 --- a/tests/virhostcpudata/linux-cpustat-24cpu.out +++ b/tests/virhostcpudata/linux-cpustat-24cpu.out @@ -1,16 +1,16 @@ cpu: kernel: 8751170 user: 14128079 -idle: 1816344522 +idle: 1816344521 iowait: 81323 -guest: 5880634 +guest: 5880635
cpu0: kernel: 447603 user: 749021 -idle: 75399242 +idle: 75399241 iowait: 5295 -guest: 331814 +guest: 331815
cpu1: kernel: 167215 diff --git a/tests/virhostcpudata/linux-cpustat-24cpu.stat b/tests/virhostcpudata/linux-cpustat-24cpu.stat index bc9d449e1c..e13035b5f0 100644 --- a/tests/virhostcpudata/linux-cpustat-24cpu.stat +++ b/tests/virhostcpudata/linux-cpustat-24cpu.stat @@ -1,5 +1,5 @@ -cpu 14126233 1846 7764352 1816344522 81323 395581 591237 0 5880634 0 -cpu0 748997 24 320851 75399242 5295 22050 104702 0 331814 0 +cpu 14126233 1846 7764352 1816344521 81323 395581 591237 0 5880634 1 +cpu0 748997 24 320851 75399241 5295 22050 104702 0 331814 1 cpu1 337325 1 140909 76178612 1121 8962 17344 0 166726 0 cpu2 666860 29 269302 75649696 4298 18473 21155 0 272094 0 cpu3 328387 77 211400 76131634 1219 9701 6573 0 115551 0 -- 2.35.3
On Mon, Jun 15, 2026 at 11:01:21AM +0200, Claudio Fontana wrote:
Hello Peter,
On 6/15/26 10:47, Peter Krempa wrote:
On Mon, Jun 15, 2026 at 10:34:54 +0200, Claudio Fontana wrote:
Fix VIR_NODE_CPU_STATS_GUEST to also account for guest nice time, similarly to how VIR_NODE_CPU_STATS_USER also accounts for nice time.
Can you elaborate a bit what the problem is ... ideally in the commit message.
Well it is there, the problem is that as I introduced this last year I did not account for guest nice time.
Hopefully we can still fix it as maybe no-one is relying on this yet..
VIR_NODE_CPU_STATS_GUEST was introduced in: b5878727c714 ("util: virHostCPUGetStatsLinux: support VIR_NODE_CPU_STATS_GUEST")
Extend the tests artificially to capture this specific case.
The trap was that the system on which the tests were likely generated did not include any guest_nice time, so tests seemed to pass. But now as I revisited the issue I noticed the missing guest_nice.
Fixes: b5878727c714d813c820ad4a1b695fbbb5ffc84e
v11.7.0-2-gb5878727c7
Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/util/virhostcpu.c | 2 +- tests/virhostcpudata/linux-cpustat-24cpu.out | 8 ++++---- tests/virhostcpudata/linux-cpustat-24cpu.stat | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 1db096ba24..f2b25e940b 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -882,7 +882,7 @@ virHostCPUGetStatsLinux(FILE *procstat, return -1;
if (virHostCPUStatsAssign(¶ms[4], VIR_NODE_CPU_STATS_GUEST, - guest * TICK_TO_NSEC) < 0) + (guest + guest_nice) * TICK_TO_NSEC) < 0) return -1; return 0; }
Shouldn't we rather introduce a new field rather than modify what the old one does?
I hope it's early enough that not many users are relying on VIR_NODE_CPU_STATS_GUEST , and accounting it together would be in line with what is done for the other stats.
Yes, this feels like simply a bug fix to me, rather than an entirely new category of data to report. With regards, Daniel -- |: https://berrange.com ~~ https://hachyderm.io/@berrange :| |: https://libvirt.org ~~ https://entangle-photo.org :| |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
participants (3)
-
Claudio Fontana -
Daniel P. Berrangé -
Peter Krempa