Contrary to most APIs returning typed parameters, there are no constants defined for the guest info 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 guest info API keys. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- include/libvirt/libvirt-domain.h | 49 ++++++++++++++++++++++++++++++++ src/libvirt-domain.c | 14 +++------ src/qemu/qemu_agent.c | 12 +++++--- 3 files changed, 61 insertions(+), 14 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index f5420bca6e..cef8bd4dbe 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -6442,6 +6442,55 @@ int virDomainSetLaunchSecurityState(virDomainPtr domain, int nparams, unsigned int flags); +/** + * VIR_DOMAIN_GUEST_INFO_USER_COUNT: + * + * The number of active users on this domain as an unsigned int. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_USER_COUNT "user.count" + +/** + * VIR_DOMAIN_GUEST_INFO_USER_PREFIX: + * + * The parameter name prefix to access each user entry. Concatenate the + * prefix, the entry number formatted as an unsigned integer and one of the + * user suffix parameters to form a complete parameter name. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_USER_PREFIX "user." + +/** + * VIR_DOMAIN_GUEST_INFO_USER_SUFFIX_NAME: + * + * Username of the user as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_USER_SUFFIX_NAME ".name" + +/** + * VIR_DOMAIN_GUEST_INFO_USER_SUFFIX_DOMAIN: + * + * Domain of the user as a string (may only be present on certain guest + * types). + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_USER_SUFFIX_DOMAIN ".domain" + +/** + * VIR_DOMAIN_GUEST_INFO_USER_SUFFIX_LOGIN_TIME: + * + * The login time of a user in milliseconds since the epoch as unsigned long + * long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_USER_SUFFIX_LOGIN_TIME ".login-time" + /** * virDomainGuestInfoTypes: * diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 072cc32255..8485dad668 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -13199,16 +13199,10 @@ virDomainSetVcpu(virDomainPtr domain, * (although not necessarily implemented for each hypervisor): * * VIR_DOMAIN_GUEST_INFO_USERS: - * returns information about users that are currently logged in within the - * guest domain. The typed parameter keys are in this format: - * - * "user.count" - the number of active users on this domain as an - * unsigned int - * "user.<num>.name" - username of the user as a string - * "user.<num>.domain" - domain of the user as a string (may only be - * present on certain guest types) - * "user.<num>.login-time" - the login time of a user in milliseconds - * since the epoch as unsigned long long + * Return information about users that are currently logged in within the + * guest domain. + * The VIR_DOMAIN_GUEST_INFO_USER_* constants define the known typed parameter + * keys. * * VIR_DOMAIN_GUEST_INFO_OS: * Return information about the operating system running within the guest. The diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 43fca86f10..cb9cce7f6a 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2200,7 +2200,7 @@ qemuAgentGetUsers(qemuAgent *agent, ndata = virJSONValueArraySize(data); if (virTypedParamsAddUInt(params, nparams, maxparams, - "user.count", ndata) < 0) + VIR_DOMAIN_GUEST_INFO_USER_COUNT, ndata) < 0) return -1; for (i = 0; i < ndata; i++) { @@ -2221,7 +2221,9 @@ qemuAgentGetUsers(qemuAgent *agent, return -1; } - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, "user.%zu.name", i); + g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + VIR_DOMAIN_GUEST_INFO_USER_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_USER_SUFFIX_NAME, i); if (virTypedParamsAddString(params, nparams, maxparams, param_name, strvalue) < 0) return -1; @@ -2229,7 +2231,8 @@ qemuAgentGetUsers(qemuAgent *agent, /* 'domain' is only present for windows guests */ if ((strvalue = virJSONValueObjectGetString(entry, "domain"))) { g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "user.%zu.domain", i); + VIR_DOMAIN_GUEST_INFO_USER_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_USER_SUFFIX_DOMAIN, i); if (virTypedParamsAddString(params, nparams, maxparams, param_name, strvalue) < 0) return -1; @@ -2241,7 +2244,8 @@ qemuAgentGetUsers(qemuAgent *agent, return -1; } g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "user.%zu.login-time", i); + VIR_DOMAIN_GUEST_INFO_USER_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_USER_SUFFIX_LOGIN_TIME, i); if (virTypedParamsAddULLong(params, nparams, maxparams, param_name, logintime * 1000) < 0) return -1; -- 2.48.1

On Tue, Mar 04, 2025 at 03:20:56PM +0100, Peter Krempa wrote: Yeah, its a double edged sword, as I really didn't want to duplicate the information, nor encourage users to use the hardcoded strings instead of the constants. I guess we're effectively dumbing down guest info/stats to the same level of docs "quality" as all the other typed parameter APIs. Doh, I missed that those changes merged now. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

Contrary to most APIs returning typed parameters, there are no constants defined for the guest info 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 guest info API keys. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- include/libvirt/libvirt-domain.h | 19 +++++++++++++++++++ src/libvirt-domain.c | 8 +++----- src/qemu/qemu_agent.c | 4 ++-- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index cbf1f6304c..29c4d9c9e0 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -6588,6 +6588,25 @@ int virDomainSetLaunchSecurityState(virDomainPtr domain, */ #define VIR_DOMAIN_GUEST_INFO_OS_VARIANT_ID "os.variant-id" + +/** + * VIR_DOMAIN_GUEST_INFO_TIMEZONE_NAME: + * + * The name of the timezone as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_TIMEZONE_NAME "timezone.name" + +/** + * VIR_DOMAIN_GUEST_INFO_TIMEZONE_OFFSET: + * + * The offset to UTC in seconds as an int. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_TIMEZONE_OFFSET "timezone.offset" + /** * virDomainGuestInfoTypes: * diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 77f1865c21..b2dac9864d 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -13210,11 +13210,9 @@ virDomainSetVcpu(virDomainPtr domain, * keys. * * VIR_DOMAIN_GUEST_INFO_TIMEZONE: - * Returns information about the timezone within the domain. The typed - * parameter keys are in this format: - * - * "timezone.name" - the name of the timezone as a string - * "timezone.offset" - the offset to UTC in seconds as an int + * Returns information about the timezone within the domain. + * The VIR_DOMAIN_GUEST_INFO_TIMEZONE_* constants define the known typed parameter + * keys. * * VIR_DOMAIN_GUEST_INFO_FILESYSTEM: * Returns information about the filesystems within the domain. The typed diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 88b293b29e..99e41d8b74 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2342,7 +2342,7 @@ qemuAgentGetTimezone(qemuAgent *agent, if ((name = virJSONValueObjectGetString(data, "zone")) && virTypedParamsAddString(params, nparams, maxparams, - "timezone.name", name) < 0) + VIR_DOMAIN_GUEST_INFO_TIMEZONE_NAME, name) < 0) return -1; if ((virJSONValueObjectGetNumberInt(data, "offset", &offset)) < 0) { @@ -2352,7 +2352,7 @@ qemuAgentGetTimezone(qemuAgent *agent, } if (virTypedParamsAddInt(params, nparams, maxparams, - "timezone.offset", offset) < 0) + VIR_DOMAIN_GUEST_INFO_TIMEZONE_OFFSET, offset) < 0) return -1; return 0; -- 2.48.1

Contrary to most APIs returning typed parameters, there are no constants defined for the guest info 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 guest info API keys. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- include/libvirt/libvirt-domain.h | 115 +++++++++++++++++++++++++++++++ src/libvirt-domain.c | 17 +---- src/qemu/qemu_driver.c | 32 ++++++--- 3 files changed, 140 insertions(+), 24 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 78ff82384c..34c931e260 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -6617,6 +6617,121 @@ int virDomainSetLaunchSecurityState(virDomainPtr domain, */ #define VIR_DOMAIN_GUEST_INFO_HOSTNAME_HOSTNAME "hostname" + +/** + * VIR_DOMAIN_GUEST_INFO_FS_COUNT: + * + * The number of filesystems defined on this domain as an unsigned int. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_FS_COUNT "fs.count" + +/** + * VIR_DOMAIN_GUEST_INFO_FS_PREFIX: + * + * The parameter name prefix to access each filesystem entry. Concatenate the + * prefix, the entry number formatted as an unsigned integer and one of the + * filesystem suffix parameters to form a complete parameter name. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_FS_PREFIX "fs." + +/** + * VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_MOUNTPOINT: + * + * The path to the mount point for the filesystem as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_MOUNTPOINT ".mountpoint" + +/** + * VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_NAME: + * + * Device name in the guest (e.g. "sda1") as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_NAME ".name" + +/** + * VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_FSTYPE: + * + * The type of filesystem as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_FSTYPE ".fstype" + +/** + * VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_TOTAL_BYTES: + * + * The total size of the filesystem as an unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_TOTAL_BYTES ".total-bytes" + +/** + * VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_USED_BYTES: + * + * The number of bytes used in the filesystem as an unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_USED_BYTES ".used-bytes" + +/** + * VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_DISK_COUNT: + * + * The number of disks targeted by this filesystem as an int. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_DISK_COUNT ".disk.count" + +/** + * VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_DISK_PREFIX: + * + * The parameter name prefix to access each disk entry. Concatenate the + * filesystem prefix, the filesystem entry number formatted as an unsigned + * integer, the disk prefix, the disk entry number formatted as an unsigned + * integer and one of the disk suffix parameters to form a complete parameter + * name. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_DISK_PREFIX ".disk." + +/** + * VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_DISK_SUFFIX_ALIAS: + * + * The device alias of the disk (e.g. sda) as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_DISK_SUFFIX_ALIAS ".alias" + +/** + * VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_DISK_SUFFIX_SERIAL: + * + * The serial number of the disk as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_DISK_SUFFIX_SERIAL ".serial" + +/** + * VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_DISK_SUFFIX_DEVICE: + * + * The device node of the disk as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_DISK_SUFFIX_DEVICE ".device" + /** * virDomainGuestInfoTypes: * diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index bd4ea7c729..324797337c 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -13220,20 +13220,9 @@ virDomainSetVcpu(virDomainPtr domain, * keys. * * VIR_DOMAIN_GUEST_INFO_FILESYSTEM: - * Returns information about the filesystems within the domain. The typed - * parameter keys are in this format: - * - * "fs.count" - the number of filesystems defined on this domain - * as an unsigned int - * "fs.<num>.mountpoint" - the path to the mount point for the filesystem - * "fs.<num>.name" - device name in the guest (e.g. "sda1") - * "fs.<num>.fstype" - the type of filesystem - * "fs.<num>.total-bytes" - the total size of the filesystem - * "fs.<num>.used-bytes" - the number of bytes used in the filesystem - * "fs.<num>.disk.count" - the number of disks targeted by this filesystem - * "fs.<num>.disk.<num>.alias" - the device alias of the disk (e.g. sda) - * "fs.<num>.disk.<num>.serial" - the serial number of the disk - * "fs.<num>.disk.<num>.device" - the device node of the disk + * Returns information about the filesystems within the domain. + * The VIR_DOMAIN_GUEST_INFO_FS_* constants define the known typed parameter + * keys. * * VIR_DOMAIN_GUEST_INFO_DISKS: * Returns information about the disks within the domain. The typed diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bfb4ef9170..0df1732a56 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19302,23 +19302,26 @@ qemuAgentFSInfoFormatParams(qemuAgentFSInfo **fsinfo, /* FIXME: get disk target */ if (virTypedParamsAddUInt(params, nparams, maxparams, - "fs.count", nfs) < 0) + VIR_DOMAIN_GUEST_INFO_FS_COUNT, nfs) < 0) return; for (i = 0; i < nfs; i++) { char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "fs.%zu.name", i); + VIR_DOMAIN_GUEST_INFO_FS_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_NAME, i); if (virTypedParamsAddString(params, nparams, maxparams, param_name, fsinfo[i]->name) < 0) return; g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "fs.%zu.mountpoint", i); + VIR_DOMAIN_GUEST_INFO_FS_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_MOUNTPOINT, i); if (virTypedParamsAddString(params, nparams, maxparams, param_name, fsinfo[i]->mountpoint) < 0) return; g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "fs.%zu.fstype", i); + VIR_DOMAIN_GUEST_INFO_FS_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_FSTYPE, i); if (virTypedParamsAddString(params, nparams, maxparams, param_name, fsinfo[i]->fstype) < 0) return; @@ -19326,21 +19329,24 @@ qemuAgentFSInfoFormatParams(qemuAgentFSInfo **fsinfo, /* disk usage values are not returned by older guest agents, so * only add the params if the value is set */ g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "fs.%zu.total-bytes", i); + VIR_DOMAIN_GUEST_INFO_FS_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_TOTAL_BYTES, i); if (fsinfo[i]->total_bytes != -1 && virTypedParamsAddULLong(params, nparams, maxparams, param_name, fsinfo[i]->total_bytes) < 0) return; g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "fs.%zu.used-bytes", i); + VIR_DOMAIN_GUEST_INFO_FS_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_USED_BYTES, i); if (fsinfo[i]->used_bytes != -1 && virTypedParamsAddULLong(params, nparams, maxparams, param_name, fsinfo[i]->used_bytes) < 0) return; g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "fs.%zu.disk.count", i); + VIR_DOMAIN_GUEST_INFO_FS_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_DISK_COUNT, i); if (virTypedParamsAddUInt(params, nparams, maxparams, param_name, fsinfo[i]->ndisks) < 0) return; @@ -19356,7 +19362,9 @@ qemuAgentFSInfoFormatParams(qemuAgentFSInfo **fsinfo, d->unit); if (diskdef) { g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "fs.%zu.disk.%zu.alias", i, j); + VIR_DOMAIN_GUEST_INFO_FS_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_DISK_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_DISK_SUFFIX_ALIAS, i, j); if (diskdef->dst && virTypedParamsAddString(params, nparams, maxparams, param_name, diskdef->dst) < 0) @@ -19364,14 +19372,18 @@ qemuAgentFSInfoFormatParams(qemuAgentFSInfo **fsinfo, } g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "fs.%zu.disk.%zu.serial", i, j); + VIR_DOMAIN_GUEST_INFO_FS_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_DISK_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_DISK_SUFFIX_SERIAL, i, j); if (d->serial && virTypedParamsAddString(params, nparams, maxparams, param_name, d->serial) < 0) return; g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "fs.%zu.disk.%zu.device", i, j); + VIR_DOMAIN_GUEST_INFO_FS_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_DISK_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_DISK_SUFFIX_DEVICE, i, j); if (d->devnode && virTypedParamsAddString(params, nparams, maxparams, param_name, d->devnode) < 0) -- 2.48.1

Contrary to most APIs returning typed parameters, there are no constants defined for the guest info 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 guest info API keys. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- include/libvirt/libvirt-domain.h | 88 ++++++++++++++++++++++++++++++++ src/libvirt-domain.c | 13 ++--- src/qemu/qemu_driver.c | 24 ++++++--- 3 files changed, 108 insertions(+), 17 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 002996b18c..1d988daf96 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -6833,6 +6833,94 @@ int virDomainSetLaunchSecurityState(virDomainPtr domain, */ #define VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_GUEST_ALIAS ".guest_alias" + +/** + * VIR_DOMAIN_GUEST_INFO_IF_COUNT: + * + * The number of interfaces defined on this domain as an unsigned int. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_IF_COUNT "if.count" + +/** + * VIR_DOMAIN_GUEST_INFO_IF_PREFIX: + * + * The parameter name prefix to access each interface entry. Concatenate the + * prefix, the entry number formatted as an unsigned integer and one of the + * interface suffix parameters to form a complete parameter name. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_IF_PREFIX "if." + +/** + * VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_NAME: + * + * Name in the guest (e.g. ``eth0``) for the interface as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_NAME ".name" + +/** + * VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_HWADDR: + * + * Hardware address in the guest of the interface as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_HWADDR ".hwaddr" + +/** + * VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_ADDR_COUNT: + * + * The number of IP addresses of interface as an unsigned int. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_ADDR_COUNT ".addr.count" + +/** + * VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_ADDR_PREFIX: + * + * The parameter name prefix to access each address entry. Concatenate the + * interface prefix, the interface entry number formatted as an unsigned + * integer, the address prefix, the address entry number formatted as an + * unsigned integer and one of the address suffix parameters to form a + * complete parameter name. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_ADDR_PREFIX ".addr." + +/** + * VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_ADDR_SUFFIX_TYPE: + * + * The IP address type (e.g. ipv4) as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_ADDR_SUFFIX_TYPE ".type" + +/** + * VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_ADDR_SUFFIX_ADDR: + * + * The IP address as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_ADDR_SUFFIX_ADDR ".addr" + +/** + * VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_ADDR_SUFFIX_PREFIX: + * + * The prefix of IP address as an unsigned int. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_ADDR_SUFFIX_PREFIX ".prefix" + /** * virDomainGuestInfoTypes: * diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 097d628c7b..b945f22efe 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -13230,16 +13230,9 @@ virDomainSetVcpu(virDomainPtr domain, * keys. * * VIR_DOMAIN_GUEST_INFO_INTERFACES: - * Returns information about the interfaces within the domain. The typed - * parameter keys are in this format: - * - * "if.count" - the number of interfaces defined on this domain - * "if.<num>.name" - name in the guest (e.g. ``eth0``) for interface <num> - * "if.<num>.hwaddr" - hardware address in the guest for interface <num> - * "if.<num>.addr.count - the number of IP addresses of interface <num> - * "if.<num>.addr.<num1>.type" - the IP address type of addr <num1> (e.g. ipv4) - * "if.<num>.addr.<num1>.addr" - the IP address of addr <num1> - * "if.<num>.addr.<num1>.prefix" - the prefix of IP address of addr <num1> + * Returns information about the interfaces within the domain. + * The VIR_DOMAIN_GUEST_INFO_IF_* constants define the known typed parameter + * keys. * * Using 0 for @types returns all information groups supported by the given * hypervisor. diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a637c36170..b091e3f850 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19411,26 +19411,30 @@ virDomainInterfaceFormatParams(virDomainInterfacePtr *ifaces, size_t j; if (virTypedParamsAddUInt(params, nparams, maxparams, - "if.count", nifaces) < 0) + VIR_DOMAIN_GUEST_INFO_IF_COUNT, + nifaces) < 0) return; for (i = 0; i < nifaces; i++) { char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "if.%zu.name", i); + VIR_DOMAIN_GUEST_INFO_IF_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_NAME, i); if (virTypedParamsAddString(params, nparams, maxparams, param_name, ifaces[i]->name) < 0) return; g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "if.%zu.hwaddr", i); + VIR_DOMAIN_GUEST_INFO_IF_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_HWADDR, i); if (virTypedParamsAddString(params, nparams, maxparams, param_name, ifaces[i]->hwaddr) < 0) return; g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "if.%zu.addr.count", i); + VIR_DOMAIN_GUEST_INFO_IF_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_ADDR_COUNT, i); if (virTypedParamsAddUInt(params, nparams, maxparams, param_name, ifaces[i]->naddrs) < 0) return; @@ -19448,19 +19452,25 @@ virDomainInterfaceFormatParams(virDomainInterfacePtr *ifaces, } g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "if.%zu.addr.%zu.type", i, j); + VIR_DOMAIN_GUEST_INFO_IF_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_ADDR_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_ADDR_SUFFIX_TYPE, i, j); if (virTypedParamsAddString(params, nparams, maxparams, param_name, type) < 0) return; g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "if.%zu.addr.%zu.addr", i, j); + VIR_DOMAIN_GUEST_INFO_IF_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_ADDR_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_ADDR_SUFFIX_ADDR, i, j); if (virTypedParamsAddString(params, nparams, maxparams, param_name, ifaces[i]->addrs[j].addr) < 0) return; g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "if.%zu.addr.%zu.prefix", i, j); + VIR_DOMAIN_GUEST_INFO_IF_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_ADDR_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_IF_SUFFIX_ADDR_SUFFIX_PREFIX, i, j); if (virTypedParamsAddUInt(params, nparams, maxparams, param_name, ifaces[i]->addrs[j].prefix) < 0) return; -- 2.48.1

On Tue, Mar 04, 2025 at 14:04:03 +0000, Daniel P. Berrangé wrote: As noted in my reply to 1/19, I think we need some wording saying that for the legacy constants that are being added the 'since' field applies only on when the constant was added but not since when the data is available. Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt;

On Tue, Mar 04, 2025 at 14:51:55 +0000, Daniel P. Berrangé wrote: Given that it wasn't versioned before we could possibly also document e.g. at the function docs block that 'anything 11.2.0' is when the constant was introduced; but doesn't necessarily mean that it's when the data is present. Both APIs explicitly allow individual fields to be missing based on a variety of factors so it's not like the user can rely on the data in any way.

On Tue, Mar 04, 2025 at 14:04:04 +0000, Daniel P. Berrangé wrote: 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>. Broken alignment. I'd prefer if you not do this style of alignment. We mention in the coding style docs that doing weird alignment is unwanted if done purely to stick to "80 columns". here too. Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt;

On Tue, Mar 04, 2025 at 14:04:05 +0000, Daniel P. Berrangé wrote: Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt;

On Tue, Mar 04, 2025 at 14:04:06 +0000, Daniel P. Berrangé wrote: Maximum (as the sentence ends with a full stop) aggregate value [...] The alignment of this hunk is off by 1. ^^^ see in new docs [...] formatting Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt;

On Tue, Mar 11, 2025 at 14:01:38 +0000, Daniel P. Berrangé wrote: Ah I see. I was comparing it to the hunk directly above (right under VIR_DOMAIN_STATS_VCPU:) which is actually correct; so this is actually fixing the alignment for that paragraph and not breaking it. It seemed to me as if it was breaking the alignment instead. Sorry for the noise

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 | 172 +++++++++++++++++++++++++++++++ src/libvirt-domain.c | 39 +------ src/qemu/qemu_driver.c | 79 ++++++++++---- 3 files changed, 233 insertions(+), 57 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 33d73da501..0360e163ca 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3291,6 +3291,178 @@ struct _virDomainStatsRecord { */ #define VIR_DOMAIN_STATS_NET_SUFFIX_TX_DROP ".tx.drop" + +/** + * VIR_DOMAIN_STATS_BLOCK_COUNT: + * + * Number of block devices in the subsequent list, as unsigned int. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BLOCK_COUNT "block.count" + +/** + * VIR_DOMAIN_STATS_BLOCK_PREFIX: + * + * The parameter name prefix to access each disk entry. Concatenate the + * prefix, the entry number formatted as an unsigned integer and one of the + * block suffix parameters to form a complete parameter name. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BLOCK_PREFIX "block." + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_NAME: + * + * Name of the block device as string. Matches the target name (vda/sda/hda) + * of the block device. If the backing chain is listed, this name is the same + * for all host resources tied to the same guest device. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BLOCK_SUFFIX_NAME ".name" + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_BACKINGINDEX: + * + * The <backingStore> index as unsigned int, only used when backing images are + * listed. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BLOCK_SUFFIX_BACKINGINDEX ".backingIndex" + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_PATH: + * + * Source of the block device as a string, if it is a file or block device + * (omitted for network sources and drives with no media inserted). + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BLOCK_SUFFIX_PATH ".path" + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_RD_REQS: + * + * Number of read requests as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BLOCK_SUFFIX_RD_REQS ".rd.reqs" + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_RD_BYTES: + * + * Number of read bytes as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BLOCK_SUFFIX_RD_BYTES ".rd.bytes" + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_RD_TIMES: + * + * Total time (ns) spent on reads as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BLOCK_SUFFIX_RD_TIMES ".rd.times" + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_WR_REQS: + * + * Number of write requests as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BLOCK_SUFFIX_WR_REQS ".wr.reqs" + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_WR_BYTES: + * + * Number of written bytes as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BLOCK_SUFFIX_WR_BYTES ".wr.bytes" + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_WR_TIMES: + * + * Total time (ns) spent on writes as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BLOCK_SUFFIX_WR_TIMES ".wr.times" + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_FL_REQS: + * + * Total flush requests as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BLOCK_SUFFIX_FL_REQS ".fl.reqs" + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_FL_TIMES: + * + * Total time (ns) spent on cache flushing as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BLOCK_SUFFIX_FL_TIMES ".fl.times" + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_ERRORS: + * + * Xen only: the 'oo_req' value as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BLOCK_SUFFIX_ERRORS ".errors" + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_ALLOCATION: + * + * Offset of the highest written sector as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BLOCK_SUFFIX_ALLOCATION ".allocation" + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_CAPACITY: + * + * Logical size in bytes of the block device backing image as unsigned long + * long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BLOCK_SUFFIX_CAPACITY ".capacity" + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_PHYSICAL: + * + * Physical size in bytes of the container of the backing image as unsigned + * long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BLOCK_SUFFIX_PHYSICAL ".physical" + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_THRESHOLD: + * + * Current threshold for delivering the VIR_DOMAIN_EVENT_ID_BLOCK_THRESHOLD + * event in bytes as unsigned long long. See virDomainSetBlockThreshold. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BLOCK_SUFFIX_THRESHOLD ".threshold" + /** * virDomainStatsTypes: * diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index cf5d718a0c..90eecea641 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12283,43 +12283,8 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING to @flags will expand the * array to cover backing chains (block.count corresponds to the number * of host resources used together to provide the guest disks). - * The typed parameter keys are in this format: - * - * "block.count" - number of block devices in the subsequent list, - * as unsigned int. - * "block.<num>.name" - name of the block device <num> as string. - * matches the target name (vda/sda/hda) of the - * block device. If the backing chain is listed, - * this name is the same for all host resources tied - * to the same guest device. - * "block.<num>.backingIndex" - unsigned int giving the <backingStore> - * index, only used when backing images - * are listed. - * "block.<num>.path" - string describing the source of block device <num>, - * if it is a file or block device (omitted for network - * sources and drives with no media inserted). - * "block.<num>.rd.reqs" - number of read requests as unsigned long long. - * "block.<num>.rd.bytes" - number of read bytes as unsigned long long. - * "block.<num>.rd.times" - total time (ns) spent on reads as - * unsigned long long. - * "block.<num>.wr.reqs" - number of write requests as unsigned long long. - * "block.<num>.wr.bytes" - number of written bytes as unsigned long long. - * "block.<num>.wr.times" - total time (ns) spent on writes as - * unsigned long long. - * "block.<num>.fl.reqs" - total flush requests as unsigned long long. - * "block.<num>.fl.times" - total time (ns) spent on cache flushing as - * unsigned long long. - * "block.<num>.errors" - Xen only: the 'oo_req' value as - * unsigned long long. - * "block.<num>.allocation" - offset of the highest written sector - * as unsigned long long. - * "block.<num>.capacity" - logical size in bytes of the block device - * backing image as unsigned long long. - * "block.<num>.physical" - physical size in bytes of the container of the - * backing image as unsigned long long. - * "block.<num>.threshold" - current threshold for delivering the - * VIR_DOMAIN_EVENT_ID_BLOCK_THRESHOLD - * event in bytes. See virDomainSetBlockThreshold. + * The VIR_DOMAIN_STATS_BLOCK_* constants define the known typed + * parameter keys. * * VIR_DOMAIN_STATS_PERF: * Return perf event statistics. diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1f1aa57b75..6f9e0270dc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17277,13 +17277,19 @@ qemuDomainGetStatsOneBlockFallback(virQEMUDriverConfig *cfg, } if (src->allocation) - virTypedParamListAddULLong(params, src->allocation, "block.%zu.allocation", block_idx); + virTypedParamListAddULLong(params, src->allocation, + VIR_DOMAIN_STATS_BLOCK_PREFIX "%zu" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_ALLOCATION, block_idx); if (src->capacity) - virTypedParamListAddULLong(params, src->capacity, "block.%zu.capacity", block_idx); + virTypedParamListAddULLong(params, src->capacity, + VIR_DOMAIN_STATS_BLOCK_PREFIX "%zu" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_CAPACITY, block_idx); if (src->physical) - virTypedParamListAddULLong(params, src->physical, "block.%zu.physical", block_idx); + virTypedParamListAddULLong(params, src->physical, + VIR_DOMAIN_STATS_BLOCK_PREFIX "%zu" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_PHYSICAL, block_idx); } @@ -17311,16 +17317,24 @@ qemuDomainGetStatsOneBlock(virQEMUDriverConfig *cfg, if (!stats || !entryname || !(entry = virHashLookup(stats, entryname))) return; - virTypedParamListAddULLong(params, entry->wr_highest_offset, "block.%zu.allocation", block_idx); + virTypedParamListAddULLong(params, entry->wr_highest_offset, + VIR_DOMAIN_STATS_BLOCK_PREFIX "%zu" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_ALLOCATION, block_idx); if (entry->capacity) - virTypedParamListAddULLong(params, entry->capacity, "block.%zu.capacity", block_idx); + virTypedParamListAddULLong(params, entry->capacity, + VIR_DOMAIN_STATS_BLOCK_PREFIX "%zu" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_CAPACITY, block_idx); if (entry->physical) { - virTypedParamListAddULLong(params, entry->physical, "block.%zu.physical", block_idx); + virTypedParamListAddULLong(params, entry->physical, + VIR_DOMAIN_STATS_BLOCK_PREFIX "%zu" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_PHYSICAL, block_idx); } else { if (qemuDomainStorageUpdatePhysical(cfg, dom, src) == 0) { - virTypedParamListAddULLong(params, src->physical, "block.%zu.physical", block_idx); + virTypedParamListAddULLong(params, src->physical, + VIR_DOMAIN_STATS_BLOCK_PREFIX "%zu" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_PHYSICAL, block_idx); } } } @@ -17338,7 +17352,9 @@ qemuDomainGetStatsBlockExportBackendStorage(const char *entryname, return; if (entry->write_threshold) - virTypedParamListAddULLong(params, entry->write_threshold, "block.%zu.threshold", recordnr); + virTypedParamListAddULLong(params, entry->write_threshold, + VIR_DOMAIN_STATS_BLOCK_PREFIX "%zu" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_THRESHOLD, recordnr); } @@ -17356,14 +17372,30 @@ qemuDomainGetStatsBlockExportFrontend(const char *frontendname, if (!stats || !frontendname || !(en = virHashLookup(stats, frontendname))) return; - virTypedParamListAddULLong(par, en->rd_req, "block.%zu.rd.reqs", idx); - virTypedParamListAddULLong(par, en->rd_bytes, "block.%zu.rd.bytes", idx); - virTypedParamListAddULLong(par, en->rd_total_times, "block.%zu.rd.times", idx); - virTypedParamListAddULLong(par, en->wr_req, "block.%zu.wr.reqs", idx); - virTypedParamListAddULLong(par, en->wr_bytes, "block.%zu.wr.bytes", idx); - virTypedParamListAddULLong(par, en->wr_total_times, "block.%zu.wr.times", idx); - virTypedParamListAddULLong(par, en->flush_req, "block.%zu.fl.reqs", idx); - virTypedParamListAddULLong(par, en->flush_total_times, "block.%zu.fl.times", idx); + virTypedParamListAddULLong(par, en->rd_req, + VIR_DOMAIN_STATS_BLOCK_PREFIX "%zu" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_RD_REQS, idx); + virTypedParamListAddULLong(par, en->rd_bytes, + VIR_DOMAIN_STATS_BLOCK_PREFIX "%zu" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_RD_BYTES, idx); + virTypedParamListAddULLong(par, en->rd_total_times, + VIR_DOMAIN_STATS_BLOCK_PREFIX "%zu" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_RD_TIMES, idx); + virTypedParamListAddULLong(par, en->wr_req, + VIR_DOMAIN_STATS_BLOCK_PREFIX "%zu" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_WR_REQS, idx); + virTypedParamListAddULLong(par, en->wr_bytes, + VIR_DOMAIN_STATS_BLOCK_PREFIX "%zu" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_WR_BYTES, idx); + virTypedParamListAddULLong(par, en->wr_total_times, + VIR_DOMAIN_STATS_BLOCK_PREFIX "%zu" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_WR_TIMES, idx); + virTypedParamListAddULLong(par, en->flush_req, + VIR_DOMAIN_STATS_BLOCK_PREFIX "%zu" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_FL_REQS, idx); + virTypedParamListAddULLong(par, en->flush_total_times, + VIR_DOMAIN_STATS_BLOCK_PREFIX "%zu" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_FL_TIMES, idx); } @@ -17373,13 +17405,19 @@ qemuDomainGetStatsBlockExportHeader(virDomainDiskDef *disk, size_t recordnr, virTypedParamList *params) { - virTypedParamListAddString(params, disk->dst, "block.%zu.name", recordnr); + virTypedParamListAddString(params, disk->dst, + VIR_DOMAIN_STATS_BLOCK_PREFIX "%zu" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_NAME, recordnr); if (virStorageSourceIsLocalStorage(src) && src->path) - virTypedParamListAddString(params, src->path, "block.%zu.path", recordnr); + virTypedParamListAddString(params, src->path, + VIR_DOMAIN_STATS_BLOCK_PREFIX "%zu" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_PATH, recordnr); if (src->id) - virTypedParamListAddUInt(params, src->id, "block.%zu.backingIndex", recordnr); + virTypedParamListAddUInt(params, src->id, + VIR_DOMAIN_STATS_BLOCK_PREFIX "%zu" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_BACKINGINDEX, recordnr); } @@ -17550,7 +17588,8 @@ qemuDomainGetStatsBlock(virQEMUDriver *driver, &visited, visitBacking, cfg, dom); } - virTypedParamListAddUInt(params, visited, "block.count"); + virTypedParamListAddUInt(params, visited, + VIR_DOMAIN_STATS_BLOCK_COUNT); virTypedParamListConcat(params, &blockparams); } -- 2.48.1

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 | 224 +++++++++++++++++++++++++++++++ src/libvirt-domain.c | 68 +--------- src/qemu/qemu_driver.c | 35 ++++- 3 files changed, 260 insertions(+), 67 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 0360e163ca..abdefe6aec 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3463,6 +3463,230 @@ struct _virDomainStatsRecord { */ #define VIR_DOMAIN_STATS_BLOCK_SUFFIX_THRESHOLD ".threshold" + +/** + * VIR_DOMAIN_STATS_PERF_CMT: + * + * The usage of l3 cache (bytes) by applications running on the platform as + * unsigned long long. It is produced by the cmt perf event. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_PERF_CMT "perf.cmt" + +/** + * VIR_DOMAIN_STATS_PERF_MBMT: + * + * The total system bandwidth (bytes/s) from one level of cache to another as + * unsigned long long. It is produced by the mbmt perf event. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_PERF_MBMT "perf.mbmt" + +/** + * VIR_DOMAIN_STATS_PERF_MBML: + * + * The amount of data (bytes/s) sent through the memory controller on the + * socket as unsigned long long. It is produced by the mbml perf event. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_PERF_MBML "perf.mbml" + +/** + * VIR_DOMAIN_STATS_PERF_CACHE_MISSES: + * + * The count of cache misses as unsigned long long. It is produced by the + * cache_misses perf event. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_PERF_CACHE_MISSES "perf.cache_misses" + +/** + * VIR_DOMAIN_STATS_PERF_CACHE_REFERENCES: + * + * The count of cache hits as unsigned long long. It is produced by the + * cache_references perf event. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_PERF_CACHE_REFERENCES "perf.cache_references" + +/** + * VIR_DOMAIN_STATS_PERF_INSTRUCTIONS: + * + * The count of instructions as unsigned long long. It is produced by the + * instructions perf event. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_PERF_INSTRUCTIONS "perf.instructions" + +/** + * VIR_DOMAIN_STATS_PERF_CPU_CYCLES: + * + * The count of cpu cycles (total/elapsed) as an unsigned long long. It is + * produced by the cpu_cycles perf event. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_PERF_CPU_CYCLES "perf.cpu_cycles" + +/** + * VIR_DOMAIN_STATS_PERF_BRANCH_INSTRUCTIONS: + * + * The count of branch instructions as unsigned long long. It is produced by + * the branch_instructions perf event. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_PERF_BRANCH_INSTRUCTIONS "perf.branch_instructions" + +/** + * VIR_DOMAIN_STATS_PERF_BRANCH_MISSES: + * + * The count of branch misses as unsigned long long. It is produced by the + * branch_misses perf event. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_PERF_BRANCH_MISSES "perf.branch_misses" + +/** + * VIR_DOMAIN_STATS_PERF_BUS_CYCLES: + * + * The count of bus cycles as unsigned long long. It is produced by the + * bus_cycles perf event. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_PERF_BUS_CYCLES "perf.bus_cycles" + +/** + * VIR_DOMAIN_STATS_PERF_STALLED_CYCLES_FRONTEND: + * + * The count of stalled cpu cycles in the frontend of the instruction processor + * pipeline as unsigned long long. It is produced by the + * stalled_cycles_frontend perf event. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_PERF_STALLED_CYCLES_FRONTEND "perf.stalled_cycles_frontend" + +/** + * VIR_DOMAIN_STATS_PERF_STALLED_CYCLES_BACKEND: + * + * The count of stalled cpu cycles in the backend of the instruction processor + * pipeline as unsigned long long. It is produced by the stalled_cycles_backend + * perf event. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_PERF_STALLED_CYCLES_BACKEND "perf.stalled_cycles_backend" + +/** + * VIR_DOMAIN_STATS_PERF_REF_CPU_CYCLES: + * + * The count of total cpu cycles not affected by CPU frequency scaling by + * applications running as unsigned long long. It is produced by the + * ref_cpu_cycles perf event. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_PERF_REF_CPU_CYCLES "perf.ref_cpu_cycles" + +/** + * VIR_DOMAIN_STATS_PERF_CPU_CLOCK: + * + * The count of cpu clock time as unsigned long long. It is produced by the + * cpu_clock perf event. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_PERF_CPU_CLOCK "perf.cpu_clock" + +/** + * VIR_DOMAIN_STATS_PERF_TASK_CLOCK: + * + * The count of task clock time as unsigned long long. It is produced by the + * task_clock perf event. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_PERF_TASK_CLOCK "perf.task_clock" + +/** + * VIR_DOMAIN_STATS_PERF_PAGE_FAULTS: + * + * The count of page faults as unsigned long long. It is produced by the + * page_faults perf event + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_PERF_PAGE_FAULTS "perf.page_faults" + +/** + * VIR_DOMAIN_STATS_PERF_CONTEXT_SWITCHES: + * + * The count of context switches as unsigned long long. It is produced by the + * context_switches perf event. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_PERF_CONTEXT_SWITCHES "perf.context_switches" + +/** + * VIR_DOMAIN_STATS_PERF_CPU_MIGRATIONS: + * + * The count of cpu migrations, from one logical processor to another, as + * unsigned long long. It is produced by the cpu_migrations perf event. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_PERF_CPU_MIGRATIONS "perf.cpu_migrations" + +/** + * VIR_DOMAIN_STATS_PERF_PAGE_FAULTS_MIN: + * + * The count of minor page faults as unsigned long long. It is produced by the + * page_faults_min perf event. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_PERF_PAGE_FAULTS_MIN "perf.page_faults_min" + +/** + * VIR_DOMAIN_STATS_PERF_PAGE_FAULTS_MAJ: + * + * The count of major page faults as unsigned long long. It is produced by the + * page_faults_maj perf event. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_PERF_PAGE_FAULTS_MAJ "perf.page_faults_maj" + +/** + * VIR_DOMAIN_STATS_PERF_ALIGNMENT_FAULTS: + * + * The count of alignment faults as unsigned long long. It is produced by the + * alignment_faults perf event + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_PERF_ALIGNMENT_FAULTS "perf.alignment_faults" + +/** + * VIR_DOMAIN_STATS_PERF_EMULATION_FAULTS: + * + * The count of emulation faults as unsigned long long. It is produced by the + * emulation_faults perf event + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_PERF_EMULATION_FAULTS "perf.emulation_faults" + /** * virDomainStatsTypes: * diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 90eecea641..516957a106 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12288,72 +12288,8 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * * VIR_DOMAIN_STATS_PERF: * Return perf event statistics. - * The typed parameter keys are in this format: - * - * "perf.cmt" - the usage of l3 cache (bytes) by applications running on - * the platform as unsigned long long. It is produced by cmt - * perf event. - * "perf.mbmt" - the total system bandwidth (bytes/s) from one level of - * cache to another as unsigned long long. It is produced - * by mbmt perf event. - * "perf.mbml" - the amount of data (bytes/s) sent through the memory - * controller on the socket as unsigned long long. It is - * produced by mbml perf event. - * "perf.cache_misses" - the count of cache misses as unsigned long long. - * It is produced by cache_misses perf event. - * "perf.cache_references" - the count of cache hits as unsigned long long. - * It is produced by cache_references perf event. - * "perf.instructions" - The count of instructions as unsigned long long. - * It is produced by instructions perf event. - * "perf.cpu_cycles" - The count of cpu cycles (total/elapsed) as an - * unsigned long long. It is produced by cpu_cycles - * perf event. - * "perf.branch_instructions" - The count of branch instructions as - * unsigned long long. It is produced by - * branch_instructions perf event. - * "perf.branch_misses" - The count of branch misses as unsigned long - * long. It is produced by branch_misses perf event. - * "perf.bus_cycles" - The count of bus cycles as unsigned long - * long. It is produced by bus_cycles perf event. - * "perf.stalled_cycles_frontend" - The count of stalled cpu cycles in the - * frontend of the instruction processor - * pipeline as unsigned long long. It is - * produced by stalled_cycles_frontend - * perf event. - * "perf.stalled_cycles_backend" - The count of stalled cpu cycles in the - * backend of the instruction processor - * pipeline as unsigned long long. It is - * produced by stalled_cycles_backend - * perf event. - * "perf.ref_cpu_cycles" - The count of total cpu cycles not affected by - * CPU frequency scaling by applications running - * as unsigned long long. It is produced by the - * ref_cpu_cycles perf event. - * "perf.cpu_clock" - The count of cpu clock time as unsigned long long. - * It is produced by the cpu_clock perf event. - * "perf.task_clock" - The count of task clock time as unsigned long long. - * It is produced by the task_clock perf event. - * "perf.page_faults" - The count of page faults as unsigned long long. - * It is produced by the page_faults perf event - * "perf.context_switches" - The count of context switches as unsigned long - * long. It is produced by the context_switches - * perf event. - * "perf.cpu_migrations" - The count of cpu migrations, from one logical - * processor to another, as unsigned long - * long. It is produced by the cpu_migrations - * perf event. - * "perf.page_faults_min" - The count of minor page faults as unsigned - * long long. It is produced by the - * page_faults_min perf event. - * "perf.page_faults_maj" - The count of major page faults as unsigned - * long long. It is produced by the - * page_faults_maj perf event. - * "perf.alignment_faults" - The count of alignment faults as unsigned - * long long. It is produced by the - * alignment_faults perf event - * "perf.emulation_faults" - The count of emulation faults as unsigned - * long long. It is produced by the - * emulation_faults perf event + * The VIR_DOMAIN_STATS_PERF_* constants define the known typed + * parameter keys. * * VIR_DOMAIN_STATS_IOTHREAD: * Return IOThread statistics if available. IOThread polling is a diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6f9e0270dc..d964a67574 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17644,11 +17644,44 @@ qemuDomainGetStatsPerfOneEvent(virPerf *perf, virTypedParamList *params) { uint64_t value = 0; +#define MAP(NAME) \ + [VIR_PERF_EVENT_ ## NAME] = VIR_DOMAIN_STATS_PERF_ ## NAME + + static const char *keys[] = { + MAP(CMT), + MAP(MBMT), + MAP(MBML), + MAP(CPU_CYCLES), + MAP(INSTRUCTIONS), + + MAP(CACHE_REFERENCES), + MAP(CACHE_MISSES), + MAP(BRANCH_INSTRUCTIONS), + MAP(BRANCH_MISSES), + MAP(BUS_CYCLES), + + MAP(STALLED_CYCLES_FRONTEND), + MAP(STALLED_CYCLES_BACKEND), + MAP(REF_CPU_CYCLES), + MAP(CPU_CLOCK), + MAP(TASK_CLOCK), + + MAP(PAGE_FAULTS), + MAP(CONTEXT_SWITCHES), + MAP(CPU_MIGRATIONS), + MAP(PAGE_FAULTS_MIN), + MAP(PAGE_FAULTS_MAJ), + + MAP(ALIGNMENT_FAULTS), + MAP(EMULATION_FAULTS), + }; +#undef MAP + G_STATIC_ASSERT(G_N_ELEMENTS(keys) == VIR_PERF_EVENT_LAST); if (virPerfReadEvent(perf, type, &value) < 0) return; - virTypedParamListAddULLong(params, value, "perf.%s", virPerfEventTypeToString(type)); + virTypedParamListAddULLong(params, value, "%s", keys[type]); } static void -- 2.48.1

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 | 56 ++++++++++++++++++++++++++++++++ src/libvirt-domain.c | 24 ++------------ src/qemu/qemu_driver.c | 12 ++++--- 3 files changed, 66 insertions(+), 26 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index abdefe6aec..fe2bbc48cb 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3687,6 +3687,62 @@ struct _virDomainStatsRecord { */ #define VIR_DOMAIN_STATS_PERF_EMULATION_FAULTS "perf.emulation_faults" + +/** + * VIR_DOMAIN_STATS_IOTHREAD_COUNT: + * + * Maximum number of IOThreads in the subsequent list as unsigned int. Each + * IOThread in the list will will use it's iothread_id value as the array + * index. There may be fewer array entries than the iothread.count value if + * the polling values are not supported. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_IOTHREAD_COUNT "iothread.count" + +/** + * VIR_DOMAIN_STATS_IOTHREAD_PREFIX: + * + * The parameter name prefix to access each iothread entry. Concatenate the + * prefix, the entry number formatted as an unsigned integer and one of the + * iothread suffix parameters to form a complete parameter name. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_IOTHREAD_PREFIX "iothread." + +/** + * VIR_DOMAIN_STATS_IOTHREAD_SUFFIX_POLL_MAX_NS: + * + * Maximum polling time in ns as an unsigned long long. A 0 (zero) means + * polling is disabled. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_IOTHREAD_SUFFIX_POLL_MAX_NS ".poll-max-ns" + +/** + * VIR_DOMAIN_STATS_IOTHREAD_SUFFIX_POLL_GROW: + * + * Polling time factor as an unsigned int or unsigned long long if exceeding + * range of unsigned int. A 0 (zero) indicates to allow the underlying + * hypervisor to choose how to grow the polling time. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_IOTHREAD_SUFFIX_POLL_GROW ".poll-grow" + +/** + * VIR_DOMAIN_STATS_IOTHREAD_SUFFIX_POLL_SHRINK: + * + * Polling time divisor as an unsigned int or unsigned long long if exceeding + * range of unsigned int. A 0 (zero) indicates to allow the underlying + * hypervisor to choose how to shrink the polling time. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_IOTHREAD_SUFFIX_POLL_SHRINK ".poll-shrink" + /** * virDomainStatsTypes: * diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 516957a106..e643fec02c 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12310,28 +12310,8 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * which could quickly exceed poll-max-ns; however, a poll-shrink of * 10 would cut that polling time more gradually. * - * The typed parameter keys are in this format: - * - * "iothread.count" - maximum number of IOThreads in the subsequent list - * as unsigned int. Each IOThread in the list will - * will use it's iothread_id value as the <id>. There - * may be fewer <id> entries than the iothread.count - * value if the polling values are not supported. - * "iothread.<id>.poll-max-ns" - maximum polling time in ns as an unsigned - * long long. A 0 (zero) means polling is - * disabled. - * "iothread.<id>.poll-grow" - polling time factor as an unsigned int or - * unsigned long long if exceeding range of - * unsigned int. - * A 0 (zero) indicates to allow the underlying - * hypervisor to choose how to grow the - * polling time. - * "iothread.<id>.poll-shrink" - polling time divisor as an unsigned int or - * unsigned long long if exceeding range of - * unsigned int. - * A 0 (zero) indicates to allow the underlying - * hypervisor to choose how to shrink the - * polling time. + * The VIR_DOMAIN_STATS_IOTHREAD_* constants define the known typed + * parameter keys. * * VIR_DOMAIN_STATS_MEMORY: * Return memory bandwidth statistics and the usage information. The typed diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d964a67574..76e121144d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17617,18 +17617,22 @@ qemuDomainGetStatsIOThread(virQEMUDriver *driver G_GNUC_UNUSED, if (niothreads == 0) return; - virTypedParamListAddUInt(params, niothreads, "iothread.count"); + virTypedParamListAddUInt(params, niothreads, + VIR_DOMAIN_STATS_IOTHREAD_COUNT); for (i = 0; i < niothreads; i++) { if (iothreads[i]->poll_valid) { virTypedParamListAddULLong(params, iothreads[i]->poll_max_ns, - "iothread.%u.poll-max-ns", + VIR_DOMAIN_STATS_IOTHREAD_PREFIX "%u" + VIR_DOMAIN_STATS_IOTHREAD_SUFFIX_POLL_MAX_NS, iothreads[i]->iothread_id); virTypedParamListAddUnsigned(params, iothreads[i]->poll_grow, - "iothread.%u.poll-grow", + VIR_DOMAIN_STATS_IOTHREAD_PREFIX "%u" + VIR_DOMAIN_STATS_IOTHREAD_SUFFIX_POLL_GROW, iothreads[i]->iothread_id); virTypedParamListAddUnsigned(params, iothreads[i]->poll_shrink, - "iothread.%u.poll-shrink", + VIR_DOMAIN_STATS_IOTHREAD_PREFIX "%u" + VIR_DOMAIN_STATS_IOTHREAD_SUFFIX_POLL_SHRINK, iothreads[i]->iothread_id); } } -- 2.48.1

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 | 92 ++++++++++++++++++++++++++++++++ src/libvirt-domain.c | 19 +------ src/qemu/qemu_driver.c | 37 +++++++++---- 3 files changed, 120 insertions(+), 28 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index fe2bbc48cb..e76d1d9319 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3743,6 +3743,98 @@ struct _virDomainStatsRecord { */ #define VIR_DOMAIN_STATS_IOTHREAD_SUFFIX_POLL_SHRINK ".poll-shrink" + +/** + * VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_COUNT: + * + * The number of memory bandwidth monitors for this domain as an unsigned int. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_COUNT "memory.bandwidth.monitor.count" + +/** + * VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_PREFIX: + * + * The parameter name prefix to access each bandwith monitor entry. + * Concatenate the prefix, the entry number formatted as an unsigned integer + * and one of the memory bandwith suffix parameters to form a complete + * parameter name. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_PREFIX "memory.bandwidth.monitor." + +/** + * VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_NAME: + * + * The name of the bandwidth monitor as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_NAME ".name" + +/** + * VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_VCPUS: + * + * The vcpu list of the bandwidth monitor as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_VCPUS ".vcpus" + +/** + * VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_NODE_COUNT: + * + * The number of memory controllers in the bandwidth monitor. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_NODE_COUNT ".node.count" + +/** + * VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_NODE_PREFIX: + * + * The parameter name prefix to access each controller entry. Concatenate the + * bandwidth monitor prefix, the monitor entry number formatted as an unsigned + * integer, the controller prefix, the controller entry number formatted as an + * unsigned integer and one of the controller suffix parameters to form a + * complete parameter name. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_NODE_PREFIX ".node." + +/** + * VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_NODE_SUFFIX_ID: + * + * Host allocated memory controller id for the controller as an unsigned int. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_NODE_SUFFIX_ID ".id" + +/** + * VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_NODE_SUFFIX_BYTES_LOCAL: + * + * The accumulative bytes consumed by vcpus passing through the memory + * controller in the same processor that the scheduled host CPU belongs to as + * an unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_NODE_SUFFIX_BYTES_LOCAL ".bytes.local" + +/** + * VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_NODE_SUFFIX_BYTES_TOTAL: + * + * The accumulative bytes consumed by vcpus passing through all memory + * controllers, either local or remote, as an unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_NODE_SUFFIX_BYTES_TOTAL ".bytes.total" + /** * virDomainStatsTypes: * diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index e643fec02c..9111184d57 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12316,23 +12316,8 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * VIR_DOMAIN_STATS_MEMORY: * Return memory bandwidth statistics and the usage information. The typed * parameter keys are in this format: - * - * "memory.bandwidth.monitor.count" - the number of memory bandwidth - * monitors for this domain - * "memory.bandwidth.monitor.<num>.name" - the name of monitor <num> - * "memory.bandwidth.monitor.<num>.vcpus" - the vcpu list of monitor <num> - * "memory.bandwidth.monitor.<num>.node.count" - the number of memory - * controller in monitor <num> - * "memory.bandwidth.monitor.<num>.node.<index>.id" - host allocated memory - * controller id for controller - * <index> of monitor <num> - * "memory.bandwidth.monitor.<num>.node.<index>.bytes.local" - the - * accumulative bytes consumed by @vcpus that passing - * through the memory controller in the same processor - * that the scheduled host CPU belongs to. - * "memory.bandwidth.monitor.<num>.node.<index>.bytes.total" - the total - * bytes consumed by @vcpus that passing through all - * memory controllers, either local or remote controller. + * The VIR_DOMAIN_STATS_MEMORY_* constants define the known typed + * parameter keys. * * VIR_DOMAIN_STATS_DIRTYRATE: * Return memory dirty rate information. The typed parameter keys are in diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 76e121144d..d48e79d5c6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16722,32 +16722,47 @@ qemuDomainGetStatsMemoryBandwidth(virQEMUDriver *driver, if (nresdata == 0) return; - virTypedParamListAddUInt(params, nresdata, "memory.bandwidth.monitor.count"); + virTypedParamListAddUInt(params, nresdata, + VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_COUNT); for (i = 0; i < nresdata; i++) { - virTypedParamListAddString(params, resdata[i]->name, "memory.bandwidth.monitor.%zu.name", i); - virTypedParamListAddString(params, resdata[i]->vcpus, "memory.bandwidth.monitor.%zu.vcpus", i); - virTypedParamListAddUInt(params, resdata[i]->nstats, "memory.bandwidth.monitor.%zu.node.count", i); + virTypedParamListAddString(params, resdata[i]->name, + VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_PREFIX "%zu" + VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_NAME, i); + virTypedParamListAddString(params, resdata[i]->vcpus, + VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_PREFIX "%zu" + VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_VCPUS, i); + virTypedParamListAddUInt(params, resdata[i]->nstats, + VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_PREFIX "%zu" + VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_NODE_COUNT, i); for (j = 0; j < resdata[i]->nstats; j++) { - virTypedParamListAddUInt(params, resdata[i]->stats[j]->id, - "memory.bandwidth.monitor.%zu.node.%zu.id", i, j); - + virTypedParamListAddUInt( + params, resdata[i]->stats[j]->id, + VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_PREFIX "%zu" + VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_NODE_PREFIX "%zu" + VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_NODE_SUFFIX_ID, i, j); features = resdata[i]->stats[j]->features; for (k = 0; features[k]; k++) { if (STREQ(features[k], "mbm_local_bytes")) { /* The accumulative data passing through local memory * controller is recorded with 64 bit counter. */ - virTypedParamListAddULLong(params, resdata[i]->stats[j]->vals[k], - "memory.bandwidth.monitor.%zu.node.%zu.bytes.local", i, j); + virTypedParamListAddULLong( + params, resdata[i]->stats[j]->vals[k], + VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_PREFIX "%zu" + VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_NODE_PREFIX "%zu" + VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_NODE_SUFFIX_BYTES_LOCAL, i, j); } if (STREQ(features[k], "mbm_total_bytes")) { /* The accumulative data passing through local and remote * memory controller is recorded with 64 bit counter. */ - virTypedParamListAddULLong(params, resdata[i]->stats[j]->vals[k], - "memory.bandwidth.monitor.%zu.node.%zu.bytes.total", i, j); + virTypedParamListAddULLong( + params, resdata[i]->stats[j]->vals[k], + VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_PREFIX "%zu" + VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_NODE_PREFIX "%zu" + VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_NODE_SUFFIX_BYTES_TOTAL, i, j); } } } -- 2.48.1

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 | 70 ++++++++++++++++++++++++++++++++ src/libvirt-domain.c | 5 ++- src/qemu/qemu_driver.c | 22 ++++++---- 3 files changed, 87 insertions(+), 10 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index e76d1d9319..a412f9ecfd 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3835,6 +3835,76 @@ struct _virDomainStatsRecord { */ #define VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_NODE_SUFFIX_BYTES_TOTAL ".bytes.total" + +/** + * VIR_DOMAIN_STATS_DIRTYRATE_CALC_STATUS: + * + * The status of last memory dirty rate calculation as an int from the + * virDomainDirtyRateStatus enum. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_DIRTYRATE_CALC_STATUS "dirtyrate.calc_status" + +/** + * VIR_DOMAIN_STATS_DIRTYRATE_CALC_START_TIME: + * + * The start time in seconds of the last memory dirty rate calculation as long + * long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_DIRTYRATE_CALC_START_TIME "dirtyrate.calc_start_time" + +/** + * VIR_DOMAIN_STATS_DIRTYRATE_CALC_PERIOD: + * + * The period in seconds of last memory dirty rate calculation as int. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_DIRTYRATE_CALC_PERIOD "dirtyrate.calc_period" + +/** + * VIR_DOMAIN_STATS_DIRTYRATE_MEGABYTES_PER_SECOND: + * + * The calculated memory dirty rate in MiB/s as long long. It is produced only + * if the calc_status is measured. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_DIRTYRATE_MEGABYTES_PER_SECOND "dirtyrate.megabytes_per_second" + +/** + * VIR_DOMAIN_STATS_DIRTYRATE_CALC_MODE: + * + * The calculation mode used last measurement as string, either of these + * 'page-sampling', 'dirty-bitmap' or 'dirty-ring' values returned. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_DIRTYRATE_CALC_MODE "dirtyrate.calc_mode" + +/** + * VIR_DOMAIN_STATS_DIRTYRATE_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. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_DIRTYRATE_VCPU_PREFIX "dirtyrate.vcpu." + +/** + * VIR_DOMAIN_STATS_DIRTYRATE_VCPU_SUFFIX_MEGABYTES_PER_SECOND: + * + * The calculated memory dirty rate for a virtual cpu as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_DIRTYRATE_VCPU_SUFFIX_MEGABYTES_PER_SECOND ".megabytes_per_second" + /** * virDomainStatsTypes: * diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 9111184d57..02fee7b5b9 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12320,8 +12320,9 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * parameter keys. * * VIR_DOMAIN_STATS_DIRTYRATE: - * Return memory dirty rate information. The typed parameter keys are in - * this format: + * Return memory dirty rate information. + * The VIR_DOMAIN_STATS_DIRTYRATE_* constants define the known typed + * parameter keys. * * "dirtyrate.calc_status" - the status of last memory dirty rate calculation, * returned as int from virDomainDirtyRateStatus diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d48e79d5c6..4747308d01 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17743,21 +17743,27 @@ qemuDomainGetStatsDirtyRate(virQEMUDriver *driver G_GNUC_UNUSED, return; } - virTypedParamListAddInt(params, info.status, "dirtyrate.calc_status"); - virTypedParamListAddLLong(params, info.startTime, "dirtyrate.calc_start_time"); - virTypedParamListAddInt(params, info.calcTime, "dirtyrate.calc_period"); + virTypedParamListAddInt(params, info.status, + VIR_DOMAIN_STATS_DIRTYRATE_CALC_STATUS); + virTypedParamListAddLLong(params, info.startTime, + VIR_DOMAIN_STATS_DIRTYRATE_CALC_START_TIME); + virTypedParamListAddInt(params, info.calcTime, + VIR_DOMAIN_STATS_DIRTYRATE_CALC_PERIOD); virTypedParamListAddString(params, qemuMonitorDirtyRateCalcModeTypeToString(info.mode), - "dirtyrate.calc_mode"); + VIR_DOMAIN_STATS_DIRTYRATE_CALC_MODE); if (info.status == VIR_DOMAIN_DIRTYRATE_MEASURED) { - virTypedParamListAddLLong(params, info.dirtyRate, "dirtyrate.megabytes_per_second"); + virTypedParamListAddLLong(params, info.dirtyRate, + VIR_DOMAIN_STATS_DIRTYRATE_MEGABYTES_PER_SECOND); if (info.mode == QEMU_MONITOR_DIRTYRATE_CALC_MODE_DIRTY_RING) { size_t i; for (i = 0; i < info.nvcpus; i++) { - virTypedParamListAddULLong(params, info.rates[i].value, - "dirtyrate.vcpu.%d.megabytes_per_second", - info.rates[i].idx); + virTypedParamListAddULLong( + params, info.rates[i].value, + VIR_DOMAIN_STATS_DIRTYRATE_VCPU_PREFIX "%d" + VIR_DOMAIN_STATS_DIRTYRATE_VCPU_SUFFIX_MEGABYTES_PER_SECOND, + info.rates[i].idx); } } } -- 2.48.1

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; --- src/libvirt-domain.c | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 02fee7b5b9..aaa44ed3ef 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12324,23 +12324,6 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * The VIR_DOMAIN_STATS_DIRTYRATE_* constants define the known typed * parameter keys. * - * "dirtyrate.calc_status" - the status of last memory dirty rate calculation, - * returned as int from virDomainDirtyRateStatus - * enum. - * "dirtyrate.calc_start_time" - the start time of last memory dirty rate - * calculation as long long. - * "dirtyrate.calc_period" - the period of last memory dirty rate calculation - * as int. - * "dirtyrate.megabytes_per_second" - the calculated memory dirty rate in - * MiB/s as long long. It is produced - * only if the calc_status is measured. - * "dirtyrate.calc_mode" - the calculation mode used last measurement, either - * of these 3 'page-sampling,dirty-bitmap,dirty-ring' - * values returned. - * "dirtyrate.vcpu.<num>.megabytes_per_second" - the calculated memory dirty - * rate for a virtual cpu as - * unsigned long long. - * * VIR_DOMAIN_STATS_VM: * Return hypervisor-specific statistics. Note that the naming and meaning * of the fields is entirely hypervisor dependent. -- 2.48.1

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 | 12 ++++++++++++ src/libvirt-domain.c | 12 +++++------- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index a412f9ecfd..3dc65e5389 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3905,6 +3905,18 @@ struct _virDomainStatsRecord { */ #define VIR_DOMAIN_STATS_DIRTYRATE_VCPU_SUFFIX_MEGABYTES_PER_SECOND ".megabytes_per_second" + +/** + * VIR_DOMAIN_STATS_VM_PREFIX: + * + * Concatenate the prefix, a hypervisor specific custom stats name and one + * of the VIR_DOMAIN_STATS_CUSTOM_TYPE_* constants to form a complete + * parameter name + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_VM_PREFIX "vm." + /** * virDomainStatsTypes: * diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index aaa44ed3ef..2015aedf34 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12332,14 +12332,12 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * * "vm.$NAME.$TYPE" * - * $NAME - name of the statistics field provided by the hypervisor - * - * $TYPE - Type of the value. The following types are returned: - * 'cur' - current instant value - * 'sum' - aggregate value - * 'max' - peak value + * 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. * - * The returned value may be either an unsigned long long or a boolean. +* The returned value may be either an unsigned long long or a boolean. * * WARNING: * The stats reported in this group are runtime-collected and -- 2.48.1