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@xxxxxxxxxx> > --- > 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@xxxxxxxxxx>
