On Tue, Mar 04, 2025 at 14:04:06 +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 | 123 +++++++++++++++++++++++++++++++
> src/libvirt-domain.c | 44 +++--------
> src/qemu/qemu_driver.c | 44 +++++++----
> 3 files changed, 166 insertions(+), 45 deletions(-)
>
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 562bc6e17e..3f84cbce65 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -3066,6 +3066,129 @@ struct _virDomainStatsRecord {
> */
> #define VIR_DOMAIN_STATS_BALLOON_HUGETLB_PGFAIL "balloon.hugetlb_pgfail"
>
> +
> +/**
> + * VIR_DOMAIN_STATS_VCPU_CURRENT:
> + *
> + * Current number of online virtual CPUs as unsigned int.
> + *
> + * Since: 11.2.0
> + */
> +#define VIR_DOMAIN_STATS_VCPU_CURRENT "vcpu.current"
> +
> +/**
> + * VIR_DOMAIN_STATS_VCPU_MAXIMUM:
> + *
> + * maximum number of online virtual CPUs as unsigned int.
Maximum (as the sentence ends with a full stop)
> + *
> + * Since: 11.2.0
> + */
> +#define VIR_DOMAIN_STATS_VCPU_MAXIMUM "vcpu.maximum"
> +
> +/**
> + * VIR_DOMAIN_STATS_VCPU_PREFIX:
> + *
> + * The parameter name prefix to access each vCPU entry. Concatenate the
> + * prefix, the entry number formatted as an unsigned integer and one of the
> + * vCPU suffix parameters to form a complete parameter name.
> + *
> + * Due to VCPU hotplug, the array could be sparse. The actual number of
> + * entries present corresponds to VIR_DOMAIN_STATS_VCPU_CURRENT, while the
> + * array size will never exceed VIR_DOMAIN_STATS_VCPU_MAXIMUM.
> + *
> + * Since: 11.2.0
> + */
> +#define VIR_DOMAIN_STATS_VCPU_PREFIX "vcpu."
> +
> +/**
> + * VIR_DOMAIN_STATS_VCPU_SUFFIX_STATE:
> + *
> + * State of the virtual CPU <num>, as int from virVcpuState enum.
> + *
> + * Since: 11.2.0
> + */
> +#define VIR_DOMAIN_STATS_VCPU_SUFFIX_STATE ".state"
> +
> +/**
> + * VIR_DOMAIN_STATS_VCPU_SUFFIX_TIME:
> + *
> + * Virtual cpu time spent by virtual CPU as unsigned long long.
> + *
> + * Since: 11.2.0
> + */
> +#define VIR_DOMAIN_STATS_VCPU_SUFFIX_TIME ".time"
> +
> +/**
> + * VIR_DOMAIN_STATS_VCPU_SUFFIX_WAIT:
> + *
> + * Time the vCPU wants to run, but the host scheduler has something else
> + * running ahead of it as unsigned long long.
> + *
> + * Since: 11.2.0
> + */
> +#define VIR_DOMAIN_STATS_VCPU_SUFFIX_WAIT ".wait"
> +
> +/**
> + * VIR_DOMAIN_STATS_VCPU_SUFFIX_HALTED:
> + *
> + * Virtual CPU is halted as a boolean, may indicate the processor is idle or
> + * even disabled, depending on the architecture.
> + *
> + * Since: 11.2.0
> + */
> +#define VIR_DOMAIN_STATS_VCPU_SUFFIX_HALTED ".halted"
> +
> +/**
> + * VIR_DOMAIN_STATS_VCPU_SUFFIX_DELAY:
> + *
> + * Time the vCPU thread was enqueued by the host scheduler, but was waiting in
> + * the queue instead of running. Exposed to the VM as a steal time. In
> + * nanoseconds as unsigned long long.
> + *
> + * Since: 11.2.0
> + */
> +#define VIR_DOMAIN_STATS_VCPU_SUFFIX_DELAY ".delay"
> +
> +
> +/**
> + * VIR_DOMAIN_STATS_CUSTOM_SUFFIX_TYPE_CUR:
> + *
> + * Hypervisor specific custom data type for current instant value
> + *
> + * The complete parameter name is formed by concatenating the field prefix,
> + * the array index formatted as an unsigned integer, a hypervisor specific
> + * parameter name, and this data type suffix.
> + *
> + * Since: 11.2.0
> + */
> +#define VIR_DOMAIN_STATS_CUSTOM_SUFFIX_TYPE_CUR ".cur"
> +
> +/**
> + * VIR_DOMAIN_STATS_CUSTOM_SUFFIX_TYPE_SUM:
> + *
> + * Hypervisor specific custom data type for current instant value
aggregate value
> + *
> + * The complete parameter name is formed by concatenating the field prefix,
> + * the array index formatted as an unsigned integer, a hypervisor specific
> + * parameter name, and this data type suffix.
> + *
> + * Since: 11.2.0
> + */
> +#define VIR_DOMAIN_STATS_CUSTOM_SUFFIX_TYPE_SUM ".sum"
> +
> +/**
> + * VIR_DOMAIN_STATS_CUSTOM_SUFFIX_TYPE_MAX:
> + *
> + * Hypervisor specific custom data type for peak value.
> + *
> + * The complete parameter name is formed by concatenating the field prefix,
> + * the array index formatted as an unsigned integer, a hypervisor specific
> + * parameter name, and this data type suffix.
> + *
> + * Since: 11.2.0
> + */
> +#define VIR_DOMAIN_STATS_CUSTOM_SUFFIX_TYPE_MAX ".max"
> +
> /**
> * virDomainStatsTypes:
> *
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 6c86ad566f..78bc053151 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -12253,43 +12253,23 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
> *
> * VIR_DOMAIN_STATS_VCPU:
> * Return virtual CPU statistics.
> - * Due to VCPU hotplug, the vcpu.<num>.* array could be sparse.
[...]
> - * This group of statistics also reports additional hypervisor-originating
> - * per-vCPU stats. The hypervisor-specific statistics in this group have the
> - * following naming scheme:
> + * This group of statistics also reports additional hypervisor-originating
> + * per-vCPU stats. The hypervisor-specific statistics in this group have the
> + * following naming scheme:
The alignment of this hunk is off by 1.
> *
> * "vcpu.<num>.$NAME.$TYPE"
> *
> - * $NAME - name of the statistics field provided by the hypervisor
> + * Where $NAME is an arbitrary choice of the hypervisor driver, for
> + * which no API constants are defined.
> + * The $TYPE values are defined by VIR_DOMAIN_STATS_CUSTOM_TYPE_*
> + * constants.
> *
> - * $TYPE - Type of the value. The following types are returned:
> - * 'cur' - current instant value
> - * 'sum' - aggregate value
^^^ see in new docs
> - * 'max' - peak value
[...]
> @@ -17147,17 +17150,30 @@ qemuDomainGetStatsVcpu(virQEMUDriver *driver G_GNUC_UNUSED,
> for (i = 0; i < virDomainDefGetVcpus(dom->def); i++) {
> virJSONValue *stat_obj = NULL;
> g_autoptr(GHashTable) stats = NULL;
> - g_autofree char *prefix = g_strdup_printf("vcpu.%u", cpuinfo[i].number);
> + g_autofree char *prefix = g_strdup_printf(
> + VIR_DOMAIN_STATS_VCPU_PREFIX "%u", cpuinfo[i].number);
formatting
>
Reviewed-by: Peter Krempa <pkrempa@xxxxxxxxxx>
