[PATCH] docs: fix 'virsh domstats --vcpu' measure units and descriptions

Fabricio Duarte posted 1 patch 1 year, 1 month ago
Failed in applying to current master (apply log)
docs/manpages/virsh.rst | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
[PATCH] docs: fix 'virsh domstats --vcpu' measure units and descriptions
Posted by Fabricio Duarte 1 year, 1 month ago
The fields are in nanoseconds, not microseconds. Also fixes the description of `vcpu.<num>.wait`, as it does not actually represent the time waiting on I/O.

Signed-off-by: Fabricio Duarte <fabricio.duarte.jr@gmail.com>
---
 docs/manpages/virsh.rst | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 2e525d3fac..b0a21e019a 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -2428,14 +2428,14 @@ When selecting the *--state* group the following fields are returned:
 * ``vcpu.<num>.state`` - state of the virtual CPU <num>, as
   number from virVcpuState enum
 * ``vcpu.<num>.time`` - virtual cpu time spent by virtual
-  CPU <num> (in microseconds)
-* ``vcpu.<num>.wait`` - virtual cpu time spent by virtual
-  CPU <num> waiting on I/O (in microseconds)
+  CPU <num> (in nanoseconds)
+* ``vcpu.<num>.wait`` - time the vCPU <num> wants to run, but the host
+  scheduler has something else running ahead of it (in nanoseconds)
 * ``vcpu.<num>.halted`` - virtual CPU <num> is halted: yes or
   no (may indicate the processor is idle or even disabled,
   depending on the architecture)
 * ``vcpu.<num>.delay`` - time the vCPU <num> thread was enqueued by the
-  host scheduler, but was waiting in the queue instead of running.
+  host scheduler, but was waiting in the queue instead of running (in nanoseconds).
   Exposed to the VM as a steal time.
 
 This group of statistics also reports additional hypervisor-originating per-vCPU
-- 
2.39.2
Re: [PATCH] docs: fix 'virsh domstats --vcpu' measure units and descriptions
Posted by Martin Kletzander 1 year ago
On Mon, Dec 16, 2024 at 07:02:44PM -0300, Fabricio Duarte wrote:
>The fields are in nanoseconds, not microseconds. Also fixes the description of `vcpu.<num>.wait`, as it does not actually represent the time waiting on I/O.
>

Sorry for responding so late to your patch and thank you for the
contribution.  Just a few small nitpicks that I'll fix before pushing:

1) It's better to wrap the commit message so that it does not have such
    long lines, and

>Signed-off-by: Fabricio Duarte <fabricio.duarte.jr@gmail.com>
>---
> docs/manpages/virsh.rst | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
>index 2e525d3fac..b0a21e019a 100644
>--- a/docs/manpages/virsh.rst
>+++ b/docs/manpages/virsh.rst
>@@ -2428,14 +2428,14 @@ When selecting the *--state* group the following fields are returned:
> * ``vcpu.<num>.state`` - state of the virtual CPU <num>, as
>   number from virVcpuState enum
> * ``vcpu.<num>.time`` - virtual cpu time spent by virtual
>-  CPU <num> (in microseconds)
>-* ``vcpu.<num>.wait`` - virtual cpu time spent by virtual
>-  CPU <num> waiting on I/O (in microseconds)
>+  CPU <num> (in nanoseconds)
>+* ``vcpu.<num>.wait`` - time the vCPU <num> wants to run, but the host
>+  scheduler has something else running ahead of it (in nanoseconds)

2) I would rather use the short description about waiting in runqueue.
    Having said that...

> * ``vcpu.<num>.halted`` - virtual CPU <num> is halted: yes or
>   no (may indicate the processor is idle or even disabled,
>   depending on the architecture)
> * ``vcpu.<num>.delay`` - time the vCPU <num> thread was enqueued by the
>-  host scheduler, but was waiting in the queue instead of running.
>+  host scheduler, but was waiting in the queue instead of running (in nanoseconds).
>   Exposed to the VM as a steal time.

that makes it the same message as this, even though we gather the info
from different places.  I went through a really deep dive into the
differences just to review this and I can't think of any other
difference than the wait being calculated maybe more precisely, but only
if CONFIG_SCHED_INFO is enabled.

Anyway, I'll fix up these two things and push the patch with my

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

Feel free to send any amendments if I made a mistake anywhere.

Thanks again for your contribution and have a nice day!

>
> This group of statistics also reports additional hypervisor-originating per-vCPU
>-- 
>2.39.2
>