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

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(a)redhat.com&gt; > > --- > > 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.