Re: [PATCH 09/19] src: add constants for domain stats 'cpu.' parameters

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 04, 2025 at 15:43:10 +0000, Daniel P. Berrangé wrote:
> On Tue, Mar 04, 2025 at 04:02:28PM +0100, Peter Krempa wrote:
> > On Tue, Mar 04, 2025 at 14:04:04 +0000, Daniel P. Berrangé wrote:
> > > Contrary to most APIs returning typed parameters, there are no constants
> > > defined for the domain stats data keys. This is was because many of the
> > > keys needs to be dynamically constructed using one or more array index
> > > values.
> > > 
> > > It is possible to define constants while still supporting dynamic
> > > array indexes by simply defining the prefixes and suffixes as constants.
> > > The consuming code can then combine the constants with array index
> > > value.
> > > 
> > > With this approach, it is practical to add constants for the domain stats
> > > API keys.
> > > 
> > > Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> > > ---
> > >  include/libvirt/libvirt-domain.h | 128 +++++++++++++++++++++++++++++++
> > >  src/libvirt-domain.c             |  30 +-------
> > >  src/qemu/qemu_driver.c           |  44 +++++++----
> > >  3 files changed, 162 insertions(+), 40 deletions(-)

[...]

> > > +#define VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_PREFIX ".bank."
> > > +
> > > +/**
> > > + * VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_SUFFIX_ID:
> > > + *
> > > + * Host allocated cache id for the bank as an unsigned int.
> > > + *
> > > + * Since: 11.2.0
> > > + */
> > > +#define VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_SUFFIX_ID ".id"
> > 
> > 
> > I really consider this to be much less understandable if you are
> > attempting to figure out what this API returns. 
> > 
> > At least for the final suffix you should add the example as it was
> > formatted in the docs you're replacing. Alternatively with actual
> > numbers instead of the substitution tokens such as <num>.
> 
> Perhaps the example usage from my cover letter should be added in the API
> docs for the guest info / stats APIs.

Yeah that is a good idea. Maybe just a high-level concatenation example
rather than language specific ones but in general that explains it
pretty good.

As said I'm also okay with documenting that the 'Since' tag is for the
constants and not when the field was added so that should take care of
both of the problems I had.




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux