[PATCH for 8.9.0] Document caveats of 'VIR_DOMAIN_STATS_VM' group of statistics

[Jiri Denemark]

On Tue, Nov 01, 2022 at 11:28:11 +0100, Peter Krempa wrote:
> The original patches adding the functionality neglected to add any form
> of documentation for the stats fields returned for this group.
> 
> The stats are directly converted from qemu's 'query-stats(-schema)' QMP
> command without any further interpretation. The 'query-stats-schema' has
> the following disclaimer:
> 
>  Note: runtime-collected statistics and their names fall outside QEMU's usual
>        deprecation policies.  QEMU will try to keep the set of available data
>        stable, together with their names, but will not guarantee stability
>        at all costs; the same is true of providers that source statistics
>        externally, e.g. from Linux.  For example, if the same value is being
>        tracked with different names on different architectures or by different
>        providers, one of them might be renamed.  A statistic might go away if
>        an algorithm is changed or some code is removed; changing a default
>        might cause previously useful statistics to always report 0.  Such
>        changes, however, are expected to be rare.
> 
> Since libvirt is not doing any form of conversion of the stats we can't
> meaningfully document any of the returned fields. At the same time we
> can't even meaningfully provide any form of API stability for the filed
> names.
> 
> Modify the documentation for the 'VIR_DOMAIN_STATS_VM' group both in the
> API docs and in the virsh man page to reflect that and disclaim any form
> of stability guarantees we provide normally.
> 
> Fixes: 8c9e3dae142
> Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> ---
>  docs/manpages/virsh.rst      | 36 ++++++++++++++++++++++++++++++++++--
>  src/libvirt-domain.c         | 26 +++++++++++++++++++++++++-
>  tools/virsh-domain-monitor.c |  2 +-
>  3 files changed, 60 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> index 61fcb2e9ca..cb2dbaec18 100644
> --- a/docs/manpages/virsh.rst
> +++ b/docs/manpages/virsh.rst
> @@ -2297,7 +2297,7 @@ domstats
> 
>     domstats [--raw] [--enforce] [--backing] [--nowait] [--state]
>        [--cpu-total] [--balloon] [--vcpu] [--interface]
> -      [--block] [--perf] [--iothread] [--memory] [--dirtyrate]
> +      [--block] [--perf] [--iothread] [--memory] [--dirtyrate] [--vm]
>        [[--list-active] [--list-inactive]
>         [--list-persistent] [--list-transient] [--list-running]y
>         [--list-paused] [--list-shutoff] [--list-other]] | [domain ...]
> @@ -2317,7 +2317,7 @@ The individual statistics groups are selectable via specific flags. By
>  default all supported statistics groups are returned. Supported
>  statistics groups flags are: *--state*, *--cpu-total*, *--balloon*,
>  *--vcpu*, *--interface*, *--block*, *--perf*, *--iothread*, *--memory*,
> -*--dirtyrate*.
> +*--dirtyrate*, *--vm*.
> 
>  Note that - depending on the hypervisor type and version or the domain state
>  - not all of the following statistics may be returned.
> @@ -2533,6 +2533,38 @@ not available for statistical purposes.
>  * ``dirtyrate.vcpu.<num>.megabytes_per_second`` - the calculated memory dirty
>    rate for a virtual cpu in MiB/s
> 
> +*--vm* returns:
> +
> +The *--vm* option enables reporting of hypervisor-specific statistics. Naming
> +and meaning of the fields is entirely hypervisor dependant.

s/dependant/dependent/ :-) twice, as the second copy in libvirt_domain.c
contains the same typo

> +
> +The statistics in this group have following naming scheme:

s/following/the following/ (twice)

> +
> + ``vm.$NAME.$TYPE``
> +
> + ``$NAME``
> +   name of the statistics field provided by the hypervisor
> +
> + ``$TYPE``
> +   Type of the value. Following types are returned:

s/Following/The following/ (twice)

> +
> +   ``cur``
> +     current instant value
> +   ``sum``
> +     aggregate value
> +   ``max``
> +     peak value
> +
> + The returned value may be either an unsigned long long or a boolean.
> +
> + **WARNING**: The stats reported in this group are runtime-collected and
> + hypervisor originated, thus fall outside of the usual stable API
> + policies of libvirt.
> +
> + Libvirt can't guarantee that the statistics reported from the outside
> + source will be present in further versions of the hypervisor, or that
> + naming or meaning will stay consistent. Changes to existing fields,
> + however, are expected to be rare.
> 
>  Selecting a specific statistics groups doesn't guarantee that the
>  daemon supports the selected group of stats. Flag *--enforce*
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 52fa136186..4728ddd6ff 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c

The three corrections mentioned above need to be applied here as well.

Reviewed-by: Jiri Denemark <jdenemar at redhat.com>