[PATCH 00/19] Add typed param constants for guest info and domain stats APIs
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. The pattern for array entries is as follows * VIR_DOMAIN_GUEST_INFO_blah_COUNT - the number of items in the array * VIR_DOMAIN_GUEST_INFO_blah_PREFIX - the key prefix, including the trailing '.' - eg "disk." * VIR_DOMAIN_GUEST_INFO_blah_SUFFIX_blah - the key suffix, including the leading '.' - eg ".name" Where there are nested arrays, this repeats in the natural way * VIR_DOMAIN_GUEST_INFO_blah_SUFFIX_blah_COUNT - the number of items in the array * VIR_DOMAIN_GUEST_INFO_blah_SUFFIX_blah_PREFIX - the key prefix, including the leading and trailing '.' - eg ".addr." * VIR_DOMAIN_GUEST_INFO_blah_SUFFIX_blah_SUFFIX_blah - the key suffix, including the leading '.' - eg ".name" A usage of this would look like, relying on CPP string concatenation asprintf(&str, VIR_DOMAIN_STATS_VCPU_PREFIX "%d" VIR_DOMAIN_STATS_VCPU_SUFFIX_DELAY, idx); Or in language bindings such as Golang fmt.Sprintf(C.VIR_DOMAIN_STATS_VCPU_PREFIX+"%d"+C.VIR_DOMAIN_STATS_VCPU_SUFFIX_HALTED, idx) Adding these constants has a few benefits: * We can ensure that different drivers are all consistent in keys used for typed parameters * Language bindings can query the API XML to identify any gaps in their API coverage A note on "Since" tags. We use the "Since" tags to express when an API was added to libvirt. In this case we're adding constants long /after/ we first introduced these APIs. So the question is whether the Since tags reflect when the typed parameter key was first used or when the API constant was added. For convenience I chose the latter, because the Golang bnidings use the Since tags to auto generate code with #if !LIBVIRT_CHECK_VERSION() around its usage of the constants. This would break if we retroactively dated the Since tags to when the keys were first used. As proof of the value, adding these constants has lead to be find sooooooooooo many mistakes in the Golang bindings where we forgot to add new domain stats/guest info data items, and one place where we typod the name. Daniel P. Berrangé (19): src: add constants for guest info 'user.' parameters src: add constants for guest info 'os.' parameters src: add constants for guest info 'timezone.' parameters src: add constant for the guest info 'hostname' parameter src: add constants for guest info 'fs.' parameters src: add constants for guest info 'disk.' parameters src: add constants for guest info 'if.' parameters src: add constants for domain stats 'state.' parameters src: add constants for domain stats 'cpu.' parameters src: add constants for domain stats 'balloon.' parameters src: add constants for domain stats 'vcpu.' parameters src: add constants for domain stats 'net.' parameters src: add constants for domain stats 'block.' parameters src: add constants for domain stats 'perf.' parameters src: add constants for domain stats 'iothread.' parameters src: add constants for domain stats 'memory.' parameters src: add constants for domain stats 'dirtyrate.' parameters src: add constants for domain stats 'dirtyrate.' parameters src: document that no constants are provided for custom VM stats include/libvirt/libvirt-domain.h | 1614 ++++++++++++++++++++++++++++++ src/libvirt-domain.c | 424 ++------ src/qemu/qemu_agent.c | 36 +- src/qemu/qemu_driver.c | 425 +++++--- 4 files changed, 1996 insertions(+), 503 deletions(-) -- 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@redhat.com> --- 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
Couple notes before doing a proper review: On Tue, Mar 04, 2025 at 14:03:56 +0000, Daniel P. Berrangé wrote:
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@redhat.com> --- 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
This makes it sound as if the field was available since the version which is not exactly true. This is especially true as ...
+ */ +#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
... you delete this ...
+ * 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.
... and replace it by the reference here. I also understand the reason but honestly for human consumption the existing format was more understandable. At least we keep that in virsh.
* * 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)
All of the above code was refactored recently to use 'virTypedParamListAdd*' so all of the patches dealing with guest agent info will need to be rebased.
On Tue, Mar 04, 2025 at 03:20:56PM +0100, Peter Krempa wrote:
Couple notes before doing a proper review:
On Tue, Mar 04, 2025 at 14:03:56 +0000, Daniel P. Berrangé wrote:
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@redhat.com> --- 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
This makes it sound as if the field was available since the version which is not exactly true. This is especially true as ...
+ */ +#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
... you delete this ...
+ * 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.
... and replace it by the reference here.
I also understand the reason but honestly for human consumption the existing format was more understandable. At least we keep that in virsh.
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.
- 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)
All of the above code was refactored recently to use 'virTypedParamListAdd*' so all of the patches dealing with guest agent info will need to be rebased.
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@redhat.com> --- include/libvirt/libvirt-domain.h | 97 ++++++++++++++++++++++++++++++++ src/libvirt-domain.c | 24 +------- src/qemu/qemu_agent.c | 20 +++---- 3 files changed, 110 insertions(+), 31 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index cef8bd4dbe..cbf1f6304c 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -6491,6 +6491,103 @@ int virDomainSetLaunchSecurityState(virDomainPtr domain, */ #define VIR_DOMAIN_GUEST_INFO_USER_SUFFIX_LOGIN_TIME ".login-time" + +/** + * VIR_DOMAIN_GUEST_INFO_OS_ID: + * + * A string identifying the operating system. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_OS_ID "os.id" + +/** + * VIR_DOMAIN_GUEST_INFO_OS_NAME: + * + * The name of the operating system, suitable for presentation to a user, as + * a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_OS_NAME "os.name" + +/** + * VIR_DOMAIN_GUEST_INFO_OS_PRETTY_NAME: + * + * A pretty name for the operating system, suitable for presentation to a + * user, as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_OS_PRETTY_NAME "os.pretty-name" + +/** + * VIR_DOMAIN_GUEST_INFO_OS_VERSION: + * + * The version of the operating system suitable for presentation to a user, + * as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_OS_VERSION "os.version" + +/** + * VIR_DOMAIN_GUEST_INFO_OS_VERSION_ID: + * + * The version id of the operating system suitable for processing by scripts, + * as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_OS_VERSION_ID "os.version-id" + +/** + * VIR_DOMAIN_GUEST_INFO_OS_KERNEL_RELEASE: + * + * The release of the operating system kernel, as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_OS_KERNEL_RELEASE "os.kernel-release" + +/** + * VIR_DOMAIN_GUEST_INFO_OS_KERNEL_VERSION: + * + * The version of the operating system kernel, as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_OS_KERNEL_VERSION "os.kernel-version" + +/** + * VIR_DOMAIN_GUEST_INFO_OS_MACHINE: + * + * The machine hardware name as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_OS_MACHINE "os.machine" + +/** + * VIR_DOMAIN_GUEST_INFO_OS_VARIANT: + * + * A specific variant or edition of the operating system suitable for + * presentation to a user, as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_OS_VARIANT "os.variant" + +/** + * VIR_DOMAIN_GUEST_INFO_OS_VARIANT_ID: + * + * The id for a specific variant or edition of the operating system, as a + * string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_OS_VARIANT_ID "os.variant-id" + /** * virDomainGuestInfoTypes: * diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 8485dad668..77f1865c21 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -13205,27 +13205,9 @@ virDomainSetVcpu(virDomainPtr domain, * keys. * * VIR_DOMAIN_GUEST_INFO_OS: - * Return information about the operating system running within the guest. The - * typed parameter keys are in this format: - * - * "os.id" - a string identifying the operating system - * "os.name" - the name of the operating system, suitable for presentation - * to a user, as a string - * "os.pretty-name" - a pretty name for the operating system, suitable for - * presentation to a user, as a string - * "os.version" - the version of the operating system suitable for - * presentation to a user, as a string - * "os.version-id" - the version id of the operating system suitable for - * processing by scripts, as a string - * "os.kernel-release" - the release of the operating system kernel, as a - * string - * "os.kernel-version" - the version of the operating system kernel, as a - * string - * "os.machine" - the machine hardware name as a string - * "os.variant" - a specific variant or edition of the operating system - * suitable for presentation to a user, as a string - * "os.variant-id" - the id for a specific variant or edition of the - * operating system, as a string + * Return information about the operating system running within the guest. + * The VIR_DOMAIN_GUEST_INFO_OS_* constants define the known typed parameter + * keys. * * VIR_DOMAIN_GUEST_INFO_TIMEZONE: * Returns information about the timezone within the domain. The typed diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index cb9cce7f6a..88b293b29e 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2294,16 +2294,16 @@ qemuAgentGetOSInfo(qemuAgent *agent, } \ } \ } while (0) - OSINFO_ADD_PARAM("id", "os.id"); - OSINFO_ADD_PARAM("name", "os.name"); - OSINFO_ADD_PARAM("pretty-name", "os.pretty-name"); - OSINFO_ADD_PARAM("version", "os.version"); - OSINFO_ADD_PARAM("version-id", "os.version-id"); - OSINFO_ADD_PARAM("machine", "os.machine"); - OSINFO_ADD_PARAM("variant", "os.variant"); - OSINFO_ADD_PARAM("variant-id", "os.variant-id"); - OSINFO_ADD_PARAM("kernel-release", "os.kernel-release"); - OSINFO_ADD_PARAM("kernel-version", "os.kernel-version"); + OSINFO_ADD_PARAM("id", VIR_DOMAIN_GUEST_INFO_OS_ID); + OSINFO_ADD_PARAM("name", VIR_DOMAIN_GUEST_INFO_OS_NAME); + OSINFO_ADD_PARAM("pretty-name", VIR_DOMAIN_GUEST_INFO_OS_PRETTY_NAME); + OSINFO_ADD_PARAM("version", VIR_DOMAIN_GUEST_INFO_OS_VERSION); + OSINFO_ADD_PARAM("version-id", VIR_DOMAIN_GUEST_INFO_OS_VERSION_ID); + OSINFO_ADD_PARAM("machine", VIR_DOMAIN_GUEST_INFO_OS_MACHINE); + OSINFO_ADD_PARAM("variant", VIR_DOMAIN_GUEST_INFO_OS_VARIANT); + OSINFO_ADD_PARAM("variant-id", VIR_DOMAIN_GUEST_INFO_OS_VARIANT_ID); + OSINFO_ADD_PARAM("kernel-release", VIR_DOMAIN_GUEST_INFO_OS_KERNEL_RELEASE); + OSINFO_ADD_PARAM("kernel-version", VIR_DOMAIN_GUEST_INFO_OS_KERNEL_VERSION); 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@redhat.com> --- 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@redhat.com> --- include/libvirt/libvirt-domain.h | 10 ++++++++++ src/libvirt-domain.c | 11 +++++------ src/qemu/qemu_driver.c | 4 +++- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 29c4d9c9e0..78ff82384c 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -6607,6 +6607,16 @@ int virDomainSetLaunchSecurityState(virDomainPtr domain, */ #define VIR_DOMAIN_GUEST_INFO_TIMEZONE_OFFSET "timezone.offset" + +/** + * VIR_DOMAIN_GUEST_INFO_HOSTNAME_HOSTNAME: + * + * The hostname of the domain as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_HOSTNAME_HOSTNAME "hostname" + /** * virDomainGuestInfoTypes: * diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index b2dac9864d..bd4ea7c729 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -13214,6 +13214,11 @@ virDomainSetVcpu(virDomainPtr domain, * The VIR_DOMAIN_GUEST_INFO_TIMEZONE_* constants define the known typed parameter * keys. * + * VIR_DOMAIN_GUEST_INFO_HOSTNAME: + * Returns information about the hostname of the domain. + * The VIR_DOMAIN_GUEST_INFO_HOSTNAME_* constants define the known typed parameter + * keys. + * * VIR_DOMAIN_GUEST_INFO_FILESYSTEM: * Returns information about the filesystems within the domain. The typed * parameter keys are in this format: @@ -13248,12 +13253,6 @@ virDomainSetVcpu(virDomainPtr domain, * "disk.<num>.guest_alias" - optional alias assigned to the disk, on Linux * this is a name assigned by device mapper * - * VIR_DOMAIN_GUEST_INFO_HOSTNAME: - * Returns information about the hostname of the domain. The typed - * parameter keys are in this format: - * - * "hostname" - the hostname of the domain - * * VIR_DOMAIN_GUEST_INFO_INTERFACES: * Returns information about the interfaces within the domain. The typed * parameter keys are in this format: diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 80c918312b..bfb4ef9170 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19511,7 +19511,9 @@ qemuDomainGetGuestInfo(virDomainPtr dom, goto exitagent; if (hostname && - virTypedParamsAddString(params, nparams, &maxparams, "hostname", hostname) < 0) + virTypedParamsAddString(params, nparams, &maxparams, + VIR_DOMAIN_GUEST_INFO_HOSTNAME_HOSTNAME, + hostname) < 0) goto exitagent; if (supportedTypes & VIR_DOMAIN_GUEST_INFO_FILESYSTEM) { -- 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@redhat.com> --- 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@redhat.com> --- include/libvirt/libvirt-domain.h | 101 +++++++++++++++++++++++++++++++ src/libvirt-domain.c | 19 +----- src/qemu/qemu_driver.c | 25 +++++--- 3 files changed, 121 insertions(+), 24 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 34c931e260..002996b18c 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -6732,6 +6732,107 @@ int virDomainSetLaunchSecurityState(virDomainPtr domain, */ #define VIR_DOMAIN_GUEST_INFO_FS_SUFFIX_DISK_SUFFIX_DEVICE ".device" + +/** + * VIR_DOMAIN_GUEST_INFO_DISK_COUNT: + * + * The number of disks defined on this domain as an unsigned int. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_DISK_COUNT "disk.count" + +/** + * VIR_DOMAIN_GUEST_INFO_DISK_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 + * disk suffix parameters to form a complete parameter name. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_DISK_PREFIX "disk." + +/** + * VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_NAME: + * + * Device node (Linux) or device UNC (Windows) as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_NAME ".name" + +/** + * VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_PARTITION: + * + * Whether this is a partition (true) or disk (false) as a boolean. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_PARTITION ".partition" + +/** + * VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_DEPENDENCY_COUNT: + * + * The number of device dependencies as an unsigned int. + * + * e.g. for LVs of the LVM this will hold the list of PVs, for LUKS encrypted + * volume this will contain the disk where the volume is placed. (Linux). + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_DEPENDENCY_COUNT ".dependency.count" + +/** + * VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_DEPENDENCY_PREFIX: + * + * The parameter name prefix to access each dependency entry. Concatenate the + * disk prefix, the disk entry number formatted as an unsigned integer, the + * dependency prefix, the dependency entry number formatted as an unsigned + * integer and one of the dependency suffix parameters to form a complete + * parameter name. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_DEPENDENCY_PREFIX ".dependency." + +/** + * VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_DEPENDENCY_SUFFIX_NAME: + * + * A dependency name as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_DEPENDENCY_SUFFIX_NAME ".name" + +/** + * VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_SERIAL: + * + * Optional disk serial number as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_SERIAL ".serial" + +/** + * VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_ALIAS: + * + * The device alias of the disk (e.g. sda) as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_ALIAS ".alias" + +/** + * VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_GUEST_ALIAS: + * + * Optional alias assigned to the disk, on Linux this is a name assigned by + * device mapper, as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_GUEST_ALIAS ".guest_alias" + /** * virDomainGuestInfoTypes: * diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 324797337c..097d628c7b 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -13225,22 +13225,9 @@ virDomainSetVcpu(virDomainPtr domain, * keys. * * VIR_DOMAIN_GUEST_INFO_DISKS: - * Returns information about the disks within the domain. The typed - * parameter keys are in this format: - * - * "disk.count" - the number of disks defined on this domain - * as an unsigned int - * "disk.<num>.name" - device node (Linux) or device UNC (Windows) - * "disk.<num>.partition" - whether this is a partition or disk - * "disk.<num>.dependency.count" - the number of device dependencies - * e.g. for LVs of the LVM this will - * hold the list of PVs, for LUKS encrypted volume this will - * contain the disk where the volume is placed. (Linux) - * "disk.<num>.dependency.<num>.name" - a dependency - * "disk.<num>.serial" - optional disk serial number (as string) - * "disk.<num>.alias" - the device alias of the disk (e.g. sda) - * "disk.<num>.guest_alias" - optional alias assigned to the disk, on Linux - * this is a name assigned by device mapper + * Returns information about the disks within the domain. + * The VIR_DOMAIN_GUEST_INFO_DISK_* constants define the known typed parameter + * keys. * * VIR_DOMAIN_GUEST_INFO_INTERFACES: * Returns information about the interfaces within the domain. The typed diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0df1732a56..a637c36170 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19215,20 +19215,23 @@ qemuAgentDiskInfoFormatParams(qemuAgentDiskInfo **info, size_t i, j, ndeps; if (virTypedParamsAddUInt(params, nparams, maxparams, - "disk.count", ndisks) < 0) + VIR_DOMAIN_GUEST_INFO_DISK_COUNT, + ndisks) < 0) return; for (i = 0; i < ndisks; i++) { char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "disk.%zu.name", i); + VIR_DOMAIN_GUEST_INFO_DISK_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_NAME, i); if (virTypedParamsAddString(params, nparams, maxparams, param_name, info[i]->name) < 0) return; g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "disk.%zu.partition", i); + VIR_DOMAIN_GUEST_INFO_DISK_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_PARTITION, i); if (virTypedParamsAddBoolean(params, nparams, maxparams, param_name, info[i]->partition) < 0) return; @@ -19236,14 +19239,17 @@ qemuAgentDiskInfoFormatParams(qemuAgentDiskInfo **info, if (info[i]->dependencies) { ndeps = g_strv_length(info[i]->dependencies); g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "disk.%zu.dependency.count", i); + VIR_DOMAIN_GUEST_INFO_DISK_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_DEPENDENCY_COUNT, i); if (ndeps && virTypedParamsAddUInt(params, nparams, maxparams, param_name, ndeps) < 0) return; for (j = 0; j < ndeps; j++) { g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "disk.%zu.dependency.%zu.name", i, j); + VIR_DOMAIN_GUEST_INFO_DISK_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_DEPENDENCY_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_DEPENDENCY_SUFFIX_NAME, i, j); if (virTypedParamsAddString(params, nparams, maxparams, param_name, info[i]->dependencies[j]) < 0) return; @@ -19256,7 +19262,8 @@ qemuAgentDiskInfoFormatParams(qemuAgentDiskInfo **info, if (address->serial) { g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "disk.%zu.serial", i); + VIR_DOMAIN_GUEST_INFO_DISK_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_SERIAL, i); if (virTypedParamsAddString(params, nparams, maxparams, param_name, address->serial) < 0) return; @@ -19271,7 +19278,8 @@ qemuAgentDiskInfoFormatParams(qemuAgentDiskInfo **info, address->unit); if (diskdef) { g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "disk.%zu.alias", i); + VIR_DOMAIN_GUEST_INFO_DISK_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_ALIAS, i); if (diskdef->dst && virTypedParamsAddString(params, nparams, maxparams, param_name, diskdef->dst) < 0) @@ -19281,7 +19289,8 @@ qemuAgentDiskInfoFormatParams(qemuAgentDiskInfo **info, if (info[i]->alias) { g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "disk.%zu.guest_alias", i); + VIR_DOMAIN_GUEST_INFO_DISK_PREFIX "%zu" + VIR_DOMAIN_GUEST_INFO_DISK_SUFFIX_GUEST_ALIAS, i); if (virTypedParamsAddString(params, nparams, maxparams, param_name, info[i]->alias) < 0) return; -- 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@redhat.com> --- 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
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@redhat.com> --- include/libvirt/libvirt-domain.h | 20 ++++++++++++++++++++ src/libvirt-domain.c | 9 +++------ src/qemu/qemu_driver.c | 6 ++++-- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 1d988daf96..5b014adcd0 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2782,6 +2782,26 @@ struct _virDomainStatsRecord { int nparams; }; + +/** + * VIR_DOMAIN_STATS_STATE_STATE: + * + * State of the VM, returned as int from virDomainState enum. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_STATE_STATE "state.state" + +/** + * VIR_DOMAIN_STATS_STATE_REASON: + * + * Reason for entering given state, returned as int from virDomain*Reason + * enum corresponding to given state. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_STATE_REASON "state.reason" + /** * virDomainStatsTypes: * diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index b945f22efe..b33b12374b 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12237,12 +12237,9 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * (although not necessarily implemented for each hypervisor): * * VIR_DOMAIN_STATS_STATE: - * Return domain state and reason for entering that state. The typed - * parameter keys are in this format: - * - * "state.state" - state of the VM, returned as int from virDomainState enum - * "state.reason" - reason for entering given state, returned as int from - * virDomain*Reason enum corresponding to given state. + * Return domain state and reason for entering that state. + * The VIR_DOMAIN_STATS_STATE_* constants define the known typed + * parameter keys. * * VIR_DOMAIN_STATS_CPU_TOTAL: * Return CPU statistics and usage information. The typed parameter keys diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b091e3f850..55fc45fef7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16562,8 +16562,10 @@ qemuDomainGetStatsState(virQEMUDriver *driver G_GNUC_UNUSED, virTypedParamList *params, unsigned int privflags G_GNUC_UNUSED) { - virTypedParamListAddInt(params, dom->state.state, "state.state"); - virTypedParamListAddInt(params, dom->state.reason, "state.reason"); + virTypedParamListAddInt(params, dom->state.state, + VIR_DOMAIN_STATS_STATE_STATE); + virTypedParamListAddInt(params, dom->state.reason, + VIR_DOMAIN_STATS_STATE_REASON); } -- 2.48.1
On Tue, Mar 04, 2025 at 14:04:03 +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@redhat.com> --- include/libvirt/libvirt-domain.h | 20 ++++++++++++++++++++ src/libvirt-domain.c | 9 +++------ src/qemu/qemu_driver.c | 6 ++++-- 3 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 1d988daf96..5b014adcd0 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2782,6 +2782,26 @@ struct _virDomainStatsRecord { int nparams; };
+ +/** + * VIR_DOMAIN_STATS_STATE_STATE: + * + * State of the VM, returned as int from virDomainState enum. + * + * Since: 11.2.0
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.
+ */ +#define VIR_DOMAIN_STATS_STATE_STATE "state.state" + +/** + * VIR_DOMAIN_STATS_STATE_REASON: + * + * Reason for entering given state, returned as int from virDomain*Reason + * enum corresponding to given state. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_STATE_REASON "state.reason"
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
On Tue, Mar 04, 2025 at 03:48:51PM +0100, Peter Krempa wrote:
On Tue, Mar 04, 2025 at 14:04:03 +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@redhat.com> --- include/libvirt/libvirt-domain.h | 20 ++++++++++++++++++++ src/libvirt-domain.c | 9 +++------ src/qemu/qemu_driver.c | 6 ++++-- 3 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 1d988daf96..5b014adcd0 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2782,6 +2782,26 @@ struct _virDomainStatsRecord { int nparams; };
+ +/** + * VIR_DOMAIN_STATS_STATE_STATE: + * + * State of the VM, returned as int from virDomainState enum. + * + * Since: 11.2.0
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.
If we did the archeology we could do "Since: 11.2.0 (constant only, data since 8.2.0)" but would need to hack our API build script to ignore stuff in the (...) brackets ? 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 :|
On Tue, Mar 04, 2025 at 14:51:55 +0000, Daniel P. Berrangé wrote:
On Tue, Mar 04, 2025 at 03:48:51PM +0100, Peter Krempa wrote:
On Tue, Mar 04, 2025 at 14:04:03 +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@redhat.com> --- include/libvirt/libvirt-domain.h | 20 ++++++++++++++++++++ src/libvirt-domain.c | 9 +++------ src/qemu/qemu_driver.c | 6 ++++-- 3 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 1d988daf96..5b014adcd0 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2782,6 +2782,26 @@ struct _virDomainStatsRecord { int nparams; };
+ +/** + * VIR_DOMAIN_STATS_STATE_STATE: + * + * State of the VM, returned as int from virDomainState enum. + * + * Since: 11.2.0
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.
If we did the archeology we could do
"Since: 11.2.0 (constant only, data since 8.2.0)"
but would need to hack our API build script to ignore stuff in the (...) brackets ?
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.
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@redhat.com> --- include/libvirt/libvirt-domain.h | 128 +++++++++++++++++++++++++++++++ src/libvirt-domain.c | 30 +------- src/qemu/qemu_driver.c | 44 +++++++---- 3 files changed, 162 insertions(+), 40 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 5b014adcd0..7e9f998f2f 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2802,6 +2802,134 @@ struct _virDomainStatsRecord { */ #define VIR_DOMAIN_STATS_STATE_REASON "state.reason" + +/** + * VIR_DOMAIN_STATS_CPU_TIME: + * + * Total cpu time spent for this domain in nanoseconds as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_TIME "cpu.time" + +/** + * VIR_DOMAIN_STATS_CPU_USER: + * + * User cpu time spent in nanoseconds as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_USER "cpu.user" + +/** + * VIR_DOMAIN_STATS_CPU_SYSTEM: + * + * System cpu time spent in nanoseconds as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_SYSTEM "cpu.system" + +/** + * VIR_DOMAIN_STATS_CPU_HALTPOLL_SUCCESS_TIME: + * + * Halt-polling cpu usage about the VCPU polled until a virtual interrupt was + * delivered in nanoseconds as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_HALTPOLL_SUCCESS_TIME "cpu.haltpoll.success.time" + +/** + * VIR_DOMAIN_STATS_CPU_HALTPOLL_FAIL_TIME: + * + * Halt-polling cpu usage about the VCPU had to schedule out (either because + * the maximum poll time was reached or it needed to yield the CPU) in + * nanoseconds as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_HALTPOLL_FAIL_TIME "cpu.haltpoll.fail.time" + +/** + * VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_COUNT: + * + * The number of cache monitors for this domain as an unsigned int. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_COUNT "cpu.cache.monitor.count" + +/** + * VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_PREFIX: + * + * The parameter name prefix to access each cache monitor entry. Concatenate + * the prefix, the entry number formatted as an unsigned integer and one of + * the cache monitor suffix parameters to form a complete parameter name. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_PREFIX "cpu.cache.monitor." + +/** + * VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_NAME: + * + * The name of cache monitor as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_NAME ".name" + +/** + * VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_VCPUS: + * + * Vcpu list of cache monitor as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_VCPUS ".vcpus" + +/** + * VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_COUNT: + * + * The number of cache banks in cache monitor as an unsigned int. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_COUNT ".bank.count" + +/** + * VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_PREFIX: + * + * The parameter name prefix to access each cache monitor bank entry. + * Concatenate the cache monitor prefix, the cache monitor entry number + * formatted as an unsigned integer, the bank prefix, the bank entry number + * formatted as an unsigned integer and one of the cache monitor bank suffix + * parameters to form a complete parameter name. + * + * Since: 11.2.0 + */ +#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" + +/** + * VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_SUFFIX_BYTES: + * + * The number of bytes of last level cache that the domain is using as an + * unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_SUFFIX_BYTES ".bytes" + /** * virDomainStatsTypes: * diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index b33b12374b..6b80206f25 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12242,33 +12242,9 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * parameter keys. * * VIR_DOMAIN_STATS_CPU_TOTAL: - * Return CPU statistics and usage information. The typed parameter keys - * are in this format: - * - * "cpu.time" - total cpu time spent for this domain in nanoseconds - * as unsigned long long. - * "cpu.user" - user cpu time spent in nanoseconds as unsigned long long. - * "cpu.system" - system cpu time spent in nanoseconds as unsigned long - * long. - * "cpu.haltpoll.success.time" - halt-polling cpu usage about the VCPU polled - * until a virtual interrupt was delivered in - * nanoseconds as unsigned long long. - * "cpu.haltpoll.fail.time" - halt-polling cpu usage about the VCPU had to schedule - * out (either because the maximum poll time was reached - * or it needed to yield the CPU) in nanoseconds as - * unsigned long long. - * "cpu.cache.monitor.count" - the number of cache monitors for this domain - * "cpu.cache.monitor.<num>.name" - the name of cache monitor <num> - * "cpu.cache.monitor.<num>.vcpus" - vcpu list of cache monitor <num> - * "cpu.cache.monitor.<num>.bank.count" - the number of cache banks in - * cache monitor <num> - * "cpu.cache.monitor.<num>.bank.<index>.id" - host allocated cache id for - * bank <index> in cache - * monitor <num> - * "cpu.cache.monitor.<num>.bank.<index>.bytes" - the number of bytes of - * last level cache that the - * domain is using on cache - * bank <index> + * Return CPU statistics and usage information. + * The VIR_DOMAIN_STATS_CPU_* constants define the known typed + * parameter keys. * * VIR_DOMAIN_STATS_BALLOON: * Return memory balloon device information. diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 55fc45fef7..ff149a46ef 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16778,16 +16778,26 @@ qemuDomainGetStatsCpuCache(virQEMUDriver *driver, return; } - virTypedParamListAddUInt(params, nresdata, "cpu.cache.monitor.count"); + virTypedParamListAddUInt(params, nresdata, + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_COUNT); for (i = 0; i < nresdata; i++) { - virTypedParamListAddString(params, resdata[i]->name, "cpu.cache.monitor.%zu.name", i); - virTypedParamListAddString(params, resdata[i]->vcpus, "cpu.cache.monitor.%zu.vcpus", i); - virTypedParamListAddUInt(params, resdata[i]->nstats, "cpu.cache.monitor.%zu.bank.count", i); + virTypedParamListAddString(params, resdata[i]->name, + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_PREFIX "%zu" + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_NAME, i); + virTypedParamListAddString(params, resdata[i]->vcpus, + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_PREFIX "%zu" + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_VCPUS, i); + virTypedParamListAddUInt(params, resdata[i]->nstats, + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_PREFIX "%zu" + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_COUNT, i); for (j = 0; j < resdata[i]->nstats; j++) { - virTypedParamListAddUInt(params, resdata[i]->stats[j]->id, - "cpu.cache.monitor.%zu.bank.%zu.id", i, j); + virTypedParamListAddUInt( + params, resdata[i]->stats[j]->id, + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_PREFIX "%zu" + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_PREFIX "%zu" + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_SUFFIX_ID, i, j); /* 'resdata[i]->stats[j]->vals[0]' keeps the value of how many last * level cache in bank j currently occupied by the vcpus listed in @@ -16798,8 +16808,11 @@ qemuDomainGetStatsCpuCache(virQEMUDriver *driver, * than 4G bytes in size, to keep the 'domstats' interface * historically consistent, it is safe to report the value with a * truncated 'UInt' data type here. */ - virTypedParamListAddUInt(params, (unsigned int)resdata[i]->stats[j]->vals[0], - "cpu.cache.monitor.%zu.bank.%zu.bytes", i, j); + virTypedParamListAddUInt( + params, (unsigned int)resdata[i]->stats[j]->vals[0], + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_PREFIX "%zu" + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_PREFIX "%zu" + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_SUFFIX_BYTES, i, j); } } @@ -16822,11 +16835,14 @@ qemuDomainGetStatsCpuCgroup(virDomainObj *dom, return; if (virCgroupGetCpuacctUsage(priv->cgroup, &cpu_time) == 0) - virTypedParamListAddULLong(params, cpu_time, "cpu.time"); + virTypedParamListAddULLong(params, cpu_time, + VIR_DOMAIN_STATS_CPU_TIME); if (virCgroupGetCpuacctStat(priv->cgroup, &user_time, &sys_time) == 0) { - virTypedParamListAddULLong(params, user_time, "cpu.user"); - virTypedParamListAddULLong(params, sys_time, "cpu.system"); + virTypedParamListAddULLong(params, user_time, + VIR_DOMAIN_STATS_CPU_USER); + virTypedParamListAddULLong(params, sys_time, + VIR_DOMAIN_STATS_CPU_SYSTEM); } } @@ -16933,8 +16949,10 @@ qemuDomainGetStatsCpuHaltPollTime(virDomainObj *dom, virHostCPUGetHaltPollTime(dom->pid, &haltPollSuccess, &haltPollFail) < 0) return; - virTypedParamListAddULLong(params, haltPollSuccess, "cpu.haltpoll.success.time"); - virTypedParamListAddULLong(params, haltPollFail, "cpu.haltpoll.fail.time"); + virTypedParamListAddULLong(params, haltPollSuccess, + VIR_DOMAIN_STATS_CPU_HALTPOLL_SUCCESS_TIME); + virTypedParamListAddULLong(params, haltPollFail, + VIR_DOMAIN_STATS_CPU_HALTPOLL_FAIL_TIME); return; } -- 2.48.1
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@redhat.com> --- include/libvirt/libvirt-domain.h | 128 +++++++++++++++++++++++++++++++ src/libvirt-domain.c | 30 +------- src/qemu/qemu_driver.c | 44 +++++++---- 3 files changed, 162 insertions(+), 40 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 5b014adcd0..7e9f998f2f 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2802,6 +2802,134 @@ struct _virDomainStatsRecord { */ #define VIR_DOMAIN_STATS_STATE_REASON "state.reason"
+ +/** + * VIR_DOMAIN_STATS_CPU_TIME: + * + * Total cpu time spent for this domain in nanoseconds as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_TIME "cpu.time" + +/** + * VIR_DOMAIN_STATS_CPU_USER: + * + * User cpu time spent in nanoseconds as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_USER "cpu.user" + +/** + * VIR_DOMAIN_STATS_CPU_SYSTEM: + * + * System cpu time spent in nanoseconds as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_SYSTEM "cpu.system" + +/** + * VIR_DOMAIN_STATS_CPU_HALTPOLL_SUCCESS_TIME: + * + * Halt-polling cpu usage about the VCPU polled until a virtual interrupt was + * delivered in nanoseconds as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_HALTPOLL_SUCCESS_TIME "cpu.haltpoll.success.time" + +/** + * VIR_DOMAIN_STATS_CPU_HALTPOLL_FAIL_TIME: + * + * Halt-polling cpu usage about the VCPU had to schedule out (either because + * the maximum poll time was reached or it needed to yield the CPU) in + * nanoseconds as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_HALTPOLL_FAIL_TIME "cpu.haltpoll.fail.time" + +/** + * VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_COUNT: + * + * The number of cache monitors for this domain as an unsigned int. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_COUNT "cpu.cache.monitor.count" + +/** + * VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_PREFIX: + * + * The parameter name prefix to access each cache monitor entry. Concatenate + * the prefix, the entry number formatted as an unsigned integer and one of + * the cache monitor suffix parameters to form a complete parameter name. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_PREFIX "cpu.cache.monitor." + +/** + * VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_NAME: + * + * The name of cache monitor as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_NAME ".name" + +/** + * VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_VCPUS: + * + * Vcpu list of cache monitor as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_VCPUS ".vcpus" + +/** + * VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_COUNT: + * + * The number of cache banks in cache monitor as an unsigned int. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_COUNT ".bank.count" + +/** + * VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_PREFIX: + * + * The parameter name prefix to access each cache monitor bank entry. + * Concatenate the cache monitor prefix, the cache monitor entry number + * formatted as an unsigned integer, the bank prefix, the bank entry number + * formatted as an unsigned integer and one of the cache monitor bank suffix + * parameters to form a complete parameter name. + * + * Since: 11.2.0 + */ +#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>.
+ +/** + * VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_SUFFIX_BYTES: + * + * The number of bytes of last level cache that the domain is using as an + * unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_SUFFIX_BYTES ".bytes" + /** * virDomainStatsTypes: * diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index b33b12374b..6b80206f25 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12242,33 +12242,9 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * parameter keys. * * VIR_DOMAIN_STATS_CPU_TOTAL: - * Return CPU statistics and usage information. The typed parameter keys - * are in this format: - * - * "cpu.time" - total cpu time spent for this domain in nanoseconds - * as unsigned long long. - * "cpu.user" - user cpu time spent in nanoseconds as unsigned long long. - * "cpu.system" - system cpu time spent in nanoseconds as unsigned long - * long. - * "cpu.haltpoll.success.time" - halt-polling cpu usage about the VCPU polled - * until a virtual interrupt was delivered in - * nanoseconds as unsigned long long. - * "cpu.haltpoll.fail.time" - halt-polling cpu usage about the VCPU had to schedule - * out (either because the maximum poll time was reached - * or it needed to yield the CPU) in nanoseconds as - * unsigned long long. - * "cpu.cache.monitor.count" - the number of cache monitors for this domain - * "cpu.cache.monitor.<num>.name" - the name of cache monitor <num> - * "cpu.cache.monitor.<num>.vcpus" - vcpu list of cache monitor <num> - * "cpu.cache.monitor.<num>.bank.count" - the number of cache banks in - * cache monitor <num> - * "cpu.cache.monitor.<num>.bank.<index>.id" - host allocated cache id for - * bank <index> in cache - * monitor <num> - * "cpu.cache.monitor.<num>.bank.<index>.bytes" - the number of bytes of - * last level cache that the - * domain is using on cache - * bank <index> + * Return CPU statistics and usage information. + * The VIR_DOMAIN_STATS_CPU_* constants define the known typed + * parameter keys. * * VIR_DOMAIN_STATS_BALLOON: * Return memory balloon device information. diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 55fc45fef7..ff149a46ef 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16778,16 +16778,26 @@ qemuDomainGetStatsCpuCache(virQEMUDriver *driver, return; }
- virTypedParamListAddUInt(params, nresdata, "cpu.cache.monitor.count"); + virTypedParamListAddUInt(params, nresdata, + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_COUNT);
for (i = 0; i < nresdata; i++) { - virTypedParamListAddString(params, resdata[i]->name, "cpu.cache.monitor.%zu.name", i); - virTypedParamListAddString(params, resdata[i]->vcpus, "cpu.cache.monitor.%zu.vcpus", i); - virTypedParamListAddUInt(params, resdata[i]->nstats, "cpu.cache.monitor.%zu.bank.count", i); + virTypedParamListAddString(params, resdata[i]->name, + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_PREFIX "%zu" + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_NAME, i); + virTypedParamListAddString(params, resdata[i]->vcpus, + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_PREFIX "%zu" + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_VCPUS, i); + virTypedParamListAddUInt(params, resdata[i]->nstats, + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_PREFIX "%zu" + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_COUNT, i);
Broken alignment.
for (j = 0; j < resdata[i]->nstats; j++) { - virTypedParamListAddUInt(params, resdata[i]->stats[j]->id, - "cpu.cache.monitor.%zu.bank.%zu.id", i, j); + virTypedParamListAddUInt( + params, resdata[i]->stats[j]->id, + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_PREFIX "%zu" + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_PREFIX "%zu" + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_SUFFIX_ID, i, j);
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".
/* 'resdata[i]->stats[j]->vals[0]' keeps the value of how many last * level cache in bank j currently occupied by the vcpus listed in @@ -16798,8 +16808,11 @@ qemuDomainGetStatsCpuCache(virQEMUDriver *driver, * than 4G bytes in size, to keep the 'domstats' interface * historically consistent, it is safe to report the value with a * truncated 'UInt' data type here. */ - virTypedParamListAddUInt(params, (unsigned int)resdata[i]->stats[j]->vals[0], - "cpu.cache.monitor.%zu.bank.%zu.bytes", i, j); + virTypedParamListAddUInt( + params, (unsigned int)resdata[i]->stats[j]->vals[0], + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_PREFIX "%zu" + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_PREFIX "%zu" + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_SUFFIX_BYTES, i, j);
here too.
} }
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
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@redhat.com> --- include/libvirt/libvirt-domain.h | 128 +++++++++++++++++++++++++++++++ src/libvirt-domain.c | 30 +------- src/qemu/qemu_driver.c | 44 +++++++---- 3 files changed, 162 insertions(+), 40 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 5b014adcd0..7e9f998f2f 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2802,6 +2802,134 @@ struct _virDomainStatsRecord { */ #define VIR_DOMAIN_STATS_STATE_REASON "state.reason"
+ +/** + * VIR_DOMAIN_STATS_CPU_TIME: + * + * Total cpu time spent for this domain in nanoseconds as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_TIME "cpu.time" + +/** + * VIR_DOMAIN_STATS_CPU_USER: + * + * User cpu time spent in nanoseconds as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_USER "cpu.user" + +/** + * VIR_DOMAIN_STATS_CPU_SYSTEM: + * + * System cpu time spent in nanoseconds as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_SYSTEM "cpu.system" + +/** + * VIR_DOMAIN_STATS_CPU_HALTPOLL_SUCCESS_TIME: + * + * Halt-polling cpu usage about the VCPU polled until a virtual interrupt was + * delivered in nanoseconds as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_HALTPOLL_SUCCESS_TIME "cpu.haltpoll.success.time" + +/** + * VIR_DOMAIN_STATS_CPU_HALTPOLL_FAIL_TIME: + * + * Halt-polling cpu usage about the VCPU had to schedule out (either because + * the maximum poll time was reached or it needed to yield the CPU) in + * nanoseconds as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_HALTPOLL_FAIL_TIME "cpu.haltpoll.fail.time" + +/** + * VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_COUNT: + * + * The number of cache monitors for this domain as an unsigned int. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_COUNT "cpu.cache.monitor.count" + +/** + * VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_PREFIX: + * + * The parameter name prefix to access each cache monitor entry. Concatenate + * the prefix, the entry number formatted as an unsigned integer and one of + * the cache monitor suffix parameters to form a complete parameter name. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_PREFIX "cpu.cache.monitor." + +/** + * VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_NAME: + * + * The name of cache monitor as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_NAME ".name" + +/** + * VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_VCPUS: + * + * Vcpu list of cache monitor as a string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_VCPUS ".vcpus" + +/** + * VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_COUNT: + * + * The number of cache banks in cache monitor as an unsigned int. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_COUNT ".bank.count" + +/** + * VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_PREFIX: + * + * The parameter name prefix to access each cache monitor bank entry. + * Concatenate the cache monitor prefix, the cache monitor entry number + * formatted as an unsigned integer, the bank prefix, the bank entry number + * formatted as an unsigned integer and one of the cache monitor bank suffix + * parameters to form a complete parameter name. + * + * Since: 11.2.0 + */ +#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.
for (j = 0; j < resdata[i]->nstats; j++) { - virTypedParamListAddUInt(params, resdata[i]->stats[j]->id, - "cpu.cache.monitor.%zu.bank.%zu.id", i, j); + virTypedParamListAddUInt( + params, resdata[i]->stats[j]->id, + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_PREFIX "%zu" + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_PREFIX "%zu" + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_SUFFIX_ID, i, j);
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".
Yeah, I was in two minds over whether to leverage that rule in this case, since the lines would end up pretty significantly over 80. If folks are ok with real long lines, i'll unwrap all the constant usage.
+ virTypedParamListAddUInt( + params, (unsigned int)resdata[i]->stats[j]->vals[0], + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_PREFIX "%zu" + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_PREFIX "%zu" + VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_SUFFIX_BYTES, i, j);
here too.
....and in many following patches
} }
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
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 :|
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@redhat.com> --- 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.
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@redhat.com> --- include/libvirt/libvirt-domain.h | 136 +++++++++++++++++++++++++++++++ src/libvirt-domain.c | 36 +------- src/qemu/qemu_driver.c | 34 ++++---- 3 files changed, 156 insertions(+), 50 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 7e9f998f2f..562bc6e17e 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2930,6 +2930,142 @@ struct _virDomainStatsRecord { */ #define VIR_DOMAIN_STATS_CPU_CACHE_MONITOR_SUFFIX_BANK_SUFFIX_BYTES ".bytes" + +/** + * VIR_DOMAIN_STATS_BALLOON_CURRENT: + * + * The memory in kiB currently used as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BALLOON_CURRENT "balloon.current" + +/** + * VIR_DOMAIN_STATS_BALLOON_MAXIMUM: + * + * The maximum memory in kiB allowed as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BALLOON_MAXIMUM "balloon.maximum" + +/** + * VIR_DOMAIN_STATS_BALLOON_SWAP_IN: + * + * The amount of data read from swap space (in KiB) as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BALLOON_SWAP_IN "balloon.swap_in" + +/** + * VIR_DOMAIN_STATS_BALLOON_SWAP_OUT: + * + * The amount of memory written out to swap space (in KiB) as unsigned long + * long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BALLOON_SWAP_OUT "balloon.swap_out" + +/** + * VIR_DOMAIN_STATS_BALLOON_MAJOR_FAULT: + * + * The number of page faults when disk IO was required as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BALLOON_MAJOR_FAULT "balloon.major_fault" + +/** + * VIR_DOMAIN_STATS_BALLOON_MINOR_FAULT: + * + * The number of other page faults as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BALLOON_MINOR_FAULT "balloon.minor_fault" + +/** + * VIR_DOMAIN_STATS_BALLOON_UNUSED: + * + * The amount of memory left unused by the system (in KiB) as unsigned long + * long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BALLOON_UNUSED "balloon.unused" + +/** + * VIR_DOMAIN_STATS_BALLOON_AVAILABLE: + * + * The amount of usable memory as seen by the domain (in KiB) as unsigned long + * long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BALLOON_AVAILABLE "balloon.available" + +/** + * VIR_DOMAIN_STATS_BALLOON_RSS: + * + * Resident Set Size of running domain's process (in KiB) as unsigned long + * long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BALLOON_RSS "balloon.rss" + +/** + * VIR_DOMAIN_STATS_BALLOON_USABLE: + * + * The amount of memory which can be reclaimed by balloon without causing host + * swapping (in KiB) as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BALLOON_USABLE "balloon.usable" + +/** + * VIR_DOMAIN_STATS_BALLOON_LAST_UPDATE: + * + * Timestamp of the last update of statistics (in seconds) as unsigned long + * long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BALLOON_LAST_UPDATE "balloon.last-update" + +/** + * VIR_DOMAIN_STATS_BALLOON_DISK_CACHES: + * + * The amount of memory that can be reclaimed without additional I/O, + * typically disk (in KiB) as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BALLOON_DISK_CACHES "balloon.disk_caches" + +/** + * VIR_DOMAIN_STATS_BALLOON_HUGETLB_PGALLOC: + * + * The number of successful huge page allocations from inside the domain via + * virtio balloon as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BALLOON_HUGETLB_PGALLOC "balloon.hugetlb_pgalloc" + +/** + * VIR_DOMAIN_STATS_BALLOON_HUGETLB_PGFAIL: + * + * The number of failed huge page allocations from inside the domain via + * virtio balloon as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_BALLOON_HUGETLB_PGFAIL "balloon.hugetlb_pgfail" + /** * virDomainStatsTypes: * diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 6b80206f25..6c86ad566f 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12248,40 +12248,8 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * * VIR_DOMAIN_STATS_BALLOON: * Return memory balloon device information. - * The typed parameter keys are in this format: - * - * "balloon.current" - the memory in kiB currently used - * as unsigned long long. - * "balloon.maximum" - the maximum memory in kiB allowed - * as unsigned long long. - * "balloon.swap_in" - the amount of data read from swap space (in KiB) - * as unsigned long long - * "balloon.swap_out" - the amount of memory written out to swap space - * (in KiB) as unsigned long long - * "balloon.major_fault" - the number of page faults when disk IO was - * required as unsigned long long - * "balloon.minor_fault" - the number of other page faults - * as unsigned long long - * "balloon.unused" - the amount of memory left unused by the system - * (in KiB) as unsigned long long - * "balloon.available" - the amount of usable memory as seen by the domain - * (in KiB) as unsigned long long - * "balloon.rss" - Resident Set Size of running domain's process - * (in KiB) as unsigned long long - * "balloon.usable" - the amount of memory which can be reclaimed by balloon - * without causing host swapping (in KiB) - * as unsigned long long - * "balloon.last-update" - timestamp of the last update of statistics - * (in seconds) as unsigned long long - * "balloon.disk_caches" - the amount of memory that can be reclaimed - * without additional I/O, typically disk (in KiB) - * as unsigned long long - * "balloon.hugetlb_pgalloc" - the number of successful huge page allocations - * from inside the domain via virtio balloon - * as unsigned long long - * "balloon.hugetlb_pgfail" - the number of failed huge page allocations - * from inside the domain via virtio balloon - * as unsigned long long + * The VIR_DOMAIN_STATS_BALLOON_* constants define the known typed + * parameter keys. * * VIR_DOMAIN_STATS_VCPU: * Return virtual CPU statistics. diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ff149a46ef..fb6e3ced73 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17005,8 +17005,10 @@ qemuDomainGetStatsBalloon(virQEMUDriver *driver G_GNUC_UNUSED, cur_balloon = dom->def->mem.cur_balloon; } - virTypedParamListAddULLong(params, cur_balloon, "balloon.current"); - virTypedParamListAddULLong(params, virDomainDefGetMemoryTotal(dom->def), "balloon.maximum"); + virTypedParamListAddULLong(params, cur_balloon, + VIR_DOMAIN_STATS_BALLOON_CURRENT); + virTypedParamListAddULLong(params, virDomainDefGetMemoryTotal(dom->def), + VIR_DOMAIN_STATS_BALLOON_MAXIMUM); if (!HAVE_JOB(privflags) || !virDomainObjIsActive(dom)) return; @@ -17016,23 +17018,23 @@ qemuDomainGetStatsBalloon(virQEMUDriver *driver G_GNUC_UNUSED, if (nr_stats < 0) return; -#define STORE_MEM_RECORD(TAG, NAME) \ +#define STORE_MEM_RECORD(TAG) \ if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_ ##TAG) \ - virTypedParamListAddULLong(params, stats[i].val, "balloon." NAME); + virTypedParamListAddULLong(params, stats[i].val, VIR_DOMAIN_STATS_BALLOON_ ##TAG) for (i = 0; i < nr_stats; i++) { - STORE_MEM_RECORD(SWAP_IN, "swap_in") - STORE_MEM_RECORD(SWAP_OUT, "swap_out") - STORE_MEM_RECORD(MAJOR_FAULT, "major_fault") - STORE_MEM_RECORD(MINOR_FAULT, "minor_fault") - STORE_MEM_RECORD(UNUSED, "unused") - STORE_MEM_RECORD(AVAILABLE, "available") - STORE_MEM_RECORD(RSS, "rss") - STORE_MEM_RECORD(LAST_UPDATE, "last-update") - STORE_MEM_RECORD(USABLE, "usable") - STORE_MEM_RECORD(DISK_CACHES, "disk_caches") - STORE_MEM_RECORD(HUGETLB_PGALLOC, "hugetlb_pgalloc") - STORE_MEM_RECORD(HUGETLB_PGFAIL, "hugetlb_pgfail") + STORE_MEM_RECORD(SWAP_IN); + STORE_MEM_RECORD(SWAP_OUT); + STORE_MEM_RECORD(MAJOR_FAULT); + STORE_MEM_RECORD(MINOR_FAULT); + STORE_MEM_RECORD(UNUSED); + STORE_MEM_RECORD(AVAILABLE); + STORE_MEM_RECORD(RSS); + STORE_MEM_RECORD(LAST_UPDATE); + STORE_MEM_RECORD(USABLE); + STORE_MEM_RECORD(DISK_CACHES); + STORE_MEM_RECORD(HUGETLB_PGALLOC); + STORE_MEM_RECORD(HUGETLB_PGFAIL); } #undef STORE_MEM_RECORD -- 2.48.1
On Tue, Mar 04, 2025 at 14:04:05 +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@redhat.com> --- include/libvirt/libvirt-domain.h | 136 +++++++++++++++++++++++++++++++ src/libvirt-domain.c | 36 +------- src/qemu/qemu_driver.c | 34 ++++---- 3 files changed, 156 insertions(+), 50 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 7e9f998f2f..562bc6e17e 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2930,6 +2930,142 @@ struct _virDomainStatsRecord {
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
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@redhat.com> --- 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. + * + * 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 + * + * 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. - * The actual size of the array corresponds to "vcpu.current". - * The array size will never exceed "vcpu.maximum". - * The typed parameter keys are in this format: + * The VIR_DOMAIN_STATS_VCPU_* constants define the known typed + * parameter keys. * - * "vcpu.current" - current number of online virtual CPUs as unsigned int. - * "vcpu.maximum" - maximum number of online virtual CPUs as unsigned int. - * "vcpu.<num>.state" - state of the virtual CPU <num>, as int - * from virVcpuState enum. - * "vcpu.<num>.time" - virtual cpu time spent by virtual CPU <num> - * as unsigned long long. - * "vcpu.<num>.wait" - time the vCPU <num> wants to run, but the host - * scheduler has something else running ahead of it. - * "vcpu.<num>.halted" - virtual CPU <num> is halted, may indicate the - * processor is idle or even disabled, depending - * on the architecture) - * "vcpu.<num>.delay" - time the vCPU <num> 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) - * - * 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: * * "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 - * 'max' - peak value - * - * The returned value may be either an unsigned long long or a boolean. - * Meaning is hypervisor specific. Please see the disclaimer for the - * VIR_DOMAIN_STATS_VM group below. + * The returned value may be either an unsigned long long or a boolean. + * Meaning is hypervisor specific. Please see the disclaimer for the + * VIR_DOMAIN_STATS_VM group below. * * VIR_DOMAIN_STATS_INTERFACE: * Return network interface statistics (from domain point of view). diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fb6e3ced73..2e652e68e9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17065,14 +17065,15 @@ qemuDomainAddStatsFromHashTable(GHashTable *stats, switch (data->type) { case QEMU_MONITOR_QUERY_STATS_TYPE_CUMULATIVE: - type = "sum"; + type = VIR_DOMAIN_STATS_CUSTOM_SUFFIX_TYPE_SUM; break; + case QEMU_MONITOR_QUERY_STATS_TYPE_INSTANT: - type = "cur"; + type = VIR_DOMAIN_STATS_CUSTOM_SUFFIX_TYPE_CUR; break; case QEMU_MONITOR_QUERY_STATS_TYPE_PEAK: - type = "max"; + type = VIR_DOMAIN_STATS_CUSTOM_SUFFIX_TYPE_MAX; break; case QEMU_MONITOR_QUERY_STATS_TYPE_LOG2_HISTOGRAM: @@ -17087,14 +17088,14 @@ qemuDomainAddStatsFromHashTable(GHashTable *stats, if (virJSONValueGetBoolean(value, &stat) < 0) continue; - virTypedParamListAddBoolean(params, stat, "%s.%s.%s", prefix, key, type); + virTypedParamListAddBoolean(params, stat, "%s.%s%s", prefix, key, type); } else { unsigned long long stat; if (virJSONValueGetNumberUlong(value, &stat) < 0) continue; - virTypedParamListAddULLong(params, stat, "%s.%s.%s", prefix, key, type); + virTypedParamListAddULLong(params, stat, "%s.%s%s", prefix, key, type); } } } @@ -17115,8 +17116,10 @@ qemuDomainGetStatsVcpu(virQEMUDriver *driver G_GNUC_UNUSED, qemuDomainObjPrivate *priv = dom->privateData; g_autoptr(virJSONValue) queried_stats = NULL; - virTypedParamListAddUInt(params, virDomainDefGetVcpus(dom->def), "vcpu.current"); - virTypedParamListAddUInt(params, virDomainDefGetVcpusMax(dom->def), "vcpu.maximum"); + virTypedParamListAddUInt(params, virDomainDefGetVcpus(dom->def), + VIR_DOMAIN_STATS_VCPU_CURRENT); + virTypedParamListAddUInt(params, virDomainDefGetVcpusMax(dom->def), + VIR_DOMAIN_STATS_VCPU_MAXIMUM); cpuinfo = g_new0(virVcpuInfo, virDomainDefGetVcpus(dom->def)); cpuwait = g_new0(unsigned long long, virDomainDefGetVcpus(dom->def)); @@ -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); - virTypedParamListAddInt(params, cpuinfo[i].state, "vcpu.%u.state", cpuinfo[i].number); + virTypedParamListAddInt(params, cpuinfo[i].state, + VIR_DOMAIN_STATS_VCPU_PREFIX "%u" + VIR_DOMAIN_STATS_VCPU_SUFFIX_STATE, + cpuinfo[i].number); /* stats below are available only if the VM is alive */ if (!virDomainObjIsActive(dom)) continue; - virTypedParamListAddULLong(params, cpuinfo[i].cpuTime, "vcpu.%u.time", cpuinfo[i].number); - virTypedParamListAddULLong(params, cpuwait[i], "vcpu.%u.wait", cpuinfo[i].number); - virTypedParamListAddULLong(params, cpudelay[i], "vcpu.%u.delay", cpuinfo[i].number); + virTypedParamListAddULLong(params, cpuinfo[i].cpuTime, + VIR_DOMAIN_STATS_VCPU_PREFIX "%u" + VIR_DOMAIN_STATS_VCPU_SUFFIX_TIME, + cpuinfo[i].number); + virTypedParamListAddULLong(params, cpuwait[i], + VIR_DOMAIN_STATS_VCPU_PREFIX "%u" + VIR_DOMAIN_STATS_VCPU_SUFFIX_WAIT, + cpuinfo[i].number); + virTypedParamListAddULLong(params, cpudelay[i], + VIR_DOMAIN_STATS_VCPU_PREFIX "%u" + VIR_DOMAIN_STATS_VCPU_SUFFIX_DELAY, + cpuinfo[i].number); /* state below is extracted from the individual vcpu structs */ if (!(vcpu = virDomainDefGetVcpu(dom->def, cpuinfo[i].number))) @@ -17167,7 +17183,9 @@ qemuDomainGetStatsVcpu(virQEMUDriver *driver G_GNUC_UNUSED, if (vcpupriv->halted != VIR_TRISTATE_BOOL_ABSENT) { virTypedParamListAddBoolean(params, vcpupriv->halted == VIR_TRISTATE_BOOL_YES, - "vcpu.%u.halted", cpuinfo[i].number); + VIR_DOMAIN_STATS_VCPU_PREFIX "%u" + VIR_DOMAIN_STATS_VCPU_SUFFIX_HALTED, + cpuinfo[i].number); } if (!queried_stats) -- 2.48.1
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@redhat.com> --- 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@redhat.com>
On Thu, Mar 06, 2025 at 03:43:18PM +0100, Peter Krempa wrote:
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@redhat.com> --- 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/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.
I'm not sure I see the problem here. This is aligned at 4-spaces which matches the surrounding code, and looks to render ok to me. 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 :|
On Tue, Mar 11, 2025 at 14:01:38 +0000, Daniel P. Berrangé wrote:
On Thu, Mar 06, 2025 at 03:43:18PM +0100, Peter Krempa wrote:
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@redhat.com> --- 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/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.
I'm not sure I see the problem here. This is aligned at 4-spaces which matches the surrounding code, and looks to render ok to me.
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@redhat.com> --- include/libvirt/libvirt-domain.h | 102 +++++++++++++++++++++++++++++++ src/libvirt-domain.c | 15 +---- src/qemu/qemu_driver.c | 27 ++++---- 3 files changed, 120 insertions(+), 24 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 3f84cbce65..33d73da501 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3189,6 +3189,108 @@ struct _virDomainStatsRecord { */ #define VIR_DOMAIN_STATS_CUSTOM_SUFFIX_TYPE_MAX ".max" + +/** + * VIR_DOMAIN_STATS_NET_COUNT: + * + * Number of network interfaces on this domain as unsigned int. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_NET_COUNT "net.count" + +/** + * VIR_DOMAIN_STATS_NET_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 + * network suffix parameters to form a complete parameter name. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_NET_PREFIX "net." + +/** + * VIR_DOMAIN_STATS_NET_SUFFIX_NAME: + * + * Name of the interface as string. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_NET_SUFFIX_NAME ".name" + +/** + * VIR_DOMAIN_STATS_NET_SUFFIX_RX_BYTES: + * + * Bytes received as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_NET_SUFFIX_RX_BYTES ".rx.bytes" + +/** + * VIR_DOMAIN_STATS_NET_SUFFIX_RX_PKTS: + * + * Packets received as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_NET_SUFFIX_RX_PKTS ".rx.pkts" + +/** + * VIR_DOMAIN_STATS_NET_SUFFIX_RX_ERRS: + * + * Receive errors as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_NET_SUFFIX_RX_ERRS ".rx.errs" + +/** + * VIR_DOMAIN_STATS_NET_SUFFIX_RX_DROP: + * + * Receive packets dropped as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_NET_SUFFIX_RX_DROP ".rx.drop" + +/** + * VIR_DOMAIN_STATS_NET_SUFFIX_TX_BYTES: + * + * Bytes transmitted as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_NET_SUFFIX_TX_BYTES ".tx.bytes" + +/** + * VIR_DOMAIN_STATS_NET_SUFFIX_TX_PKTS: + * + * Packets transmitted as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_NET_SUFFIX_TX_PKTS ".rx.pkts" + +/** + * VIR_DOMAIN_STATS_NET_SUFFIX_TX_ERRS: + * + * Transmission errors as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_NET_SUFFIX_TX_ERRS ".tx.errs" + +/** + * VIR_DOMAIN_STATS_NET_SUFFIX_TX_DROP: + * + * Transmit packets dropped as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_NET_SUFFIX_TX_DROP ".tx.drop" + /** * virDomainStatsTypes: * diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 78bc053151..cf5d718a0c 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12273,19 +12273,8 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * * VIR_DOMAIN_STATS_INTERFACE: * Return network interface statistics (from domain point of view). - * The typed parameter keys are in this format: - * - * "net.count" - number of network interfaces on this domain - * as unsigned int. - * "net.<num>.name" - name of the interface <num> as string. - * "net.<num>.rx.bytes" - bytes received as unsigned long long. - * "net.<num>.rx.pkts" - packets received as unsigned long long. - * "net.<num>.rx.errs" - receive errors as unsigned long long. - * "net.<num>.rx.drop" - receive packets dropped as unsigned long long. - * "net.<num>.tx.bytes" - bytes transmitted as unsigned long long. - * "net.<num>.tx.pkts" - packets transmitted as unsigned long long. - * "net.<num>.tx.errs" - transmission errors as unsigned long long. - * "net.<num>.tx.drop" - transmit packets dropped as unsigned long long. + * The VIR_DOMAIN_STATS_NET_* constants define the known typed + * parameter keys. * * VIR_DOMAIN_STATS_BLOCK: * Return block devices statistics. By default, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2e652e68e9..1f1aa57b75 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17200,7 +17200,9 @@ qemuDomainGetStatsVcpu(virQEMUDriver *driver G_GNUC_UNUSED, #define QEMU_ADD_NET_PARAM(params, num, name, value) \ if (value >= 0)\ - virTypedParamListAddULLong((params), (value), "net.%zu.%s", (num), (name)); + virTypedParamListAddULLong((params), (value), \ + VIR_DOMAIN_STATS_NET_PREFIX "%zu" \ + VIR_DOMAIN_STATS_NET_SUFFIX_ ## name, (num)); static void qemuDomainGetStatsInterface(virQEMUDriver *driver G_GNUC_UNUSED, @@ -17213,7 +17215,8 @@ qemuDomainGetStatsInterface(virQEMUDriver *driver G_GNUC_UNUSED, if (!virDomainObjIsActive(dom)) return; - virTypedParamListAddUInt(params, dom->def->nnets, "net.count"); + virTypedParamListAddUInt(params, dom->def->nnets, + VIR_DOMAIN_STATS_NET_COUNT); /* Check the path is one of the domain's network interfaces. */ for (i = 0; i < dom->def->nnets; i++) { @@ -17226,7 +17229,9 @@ qemuDomainGetStatsInterface(virQEMUDriver *driver G_GNUC_UNUSED, actualType = virDomainNetGetActualType(net); - virTypedParamListAddString(params, net->ifname, "net.%zu.name", i); + virTypedParamListAddString(params, net->ifname, + VIR_DOMAIN_STATS_NET_PREFIX "%zu" + VIR_DOMAIN_STATS_NET_SUFFIX_NAME, i); if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) { if (virNetDevOpenvswitchInterfaceStats(net->ifname, &tmp) < 0) { @@ -17241,14 +17246,14 @@ qemuDomainGetStatsInterface(virQEMUDriver *driver G_GNUC_UNUSED, } } - QEMU_ADD_NET_PARAM(params, i, "rx.bytes", tmp.rx_bytes); - QEMU_ADD_NET_PARAM(params, i, "rx.pkts", tmp.rx_packets); - QEMU_ADD_NET_PARAM(params, i, "rx.errs", tmp.rx_errs); - QEMU_ADD_NET_PARAM(params, i, "rx.drop", tmp.rx_drop); - QEMU_ADD_NET_PARAM(params, i, "tx.bytes", tmp.tx_bytes); - QEMU_ADD_NET_PARAM(params, i, "tx.pkts", tmp.tx_packets); - QEMU_ADD_NET_PARAM(params, i, "tx.errs", tmp.tx_errs); - QEMU_ADD_NET_PARAM(params, i, "tx.drop", tmp.tx_drop); + QEMU_ADD_NET_PARAM(params, i, RX_BYTES, tmp.rx_bytes); + QEMU_ADD_NET_PARAM(params, i, RX_PKTS, tmp.rx_packets); + QEMU_ADD_NET_PARAM(params, i, RX_ERRS, tmp.rx_errs); + QEMU_ADD_NET_PARAM(params, i, RX_DROP, tmp.rx_drop); + QEMU_ADD_NET_PARAM(params, i, TX_BYTES, tmp.tx_bytes); + QEMU_ADD_NET_PARAM(params, i, TX_PKTS, tmp.tx_packets); + QEMU_ADD_NET_PARAM(params, i, TX_ERRS, tmp.tx_errs); + QEMU_ADD_NET_PARAM(params, i, TX_DROP, tmp.tx_drop); } } -- 2.48.1
On Tue, Mar 04, 2025 at 14:04:07 +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@redhat.com> --- include/libvirt/libvirt-domain.h | 102 +++++++++++++++++++++++++++++++ src/libvirt-domain.c | 15 +---- src/qemu/qemu_driver.c | 27 ++++---- 3 files changed, 120 insertions(+), 24 deletions(-)
[...]
+/** + * VIR_DOMAIN_STATS_NET_SUFFIX_TX_BYTES: + * + * Bytes transmitted as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_NET_SUFFIX_TX_BYTES ".tx.bytes" + +/** + * VIR_DOMAIN_STATS_NET_SUFFIX_TX_PKTS: + * + * Packets transmitted as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_NET_SUFFIX_TX_PKTS ".rx.pkts"
".tx.pkts" Reviewed-by: Peter Krempa <pkrempa@redhat.com>
On Thu, Mar 06, 2025 at 03:47:00PM +0100, Peter Krempa wrote:
On Tue, Mar 04, 2025 at 14:04:07 +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@redhat.com> --- include/libvirt/libvirt-domain.h | 102 +++++++++++++++++++++++++++++++ src/libvirt-domain.c | 15 +---- src/qemu/qemu_driver.c | 27 ++++---- 3 files changed, 120 insertions(+), 24 deletions(-)
[...]
+/** + * VIR_DOMAIN_STATS_NET_SUFFIX_TX_BYTES: + * + * Bytes transmitted as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_NET_SUFFIX_TX_BYTES ".tx.bytes" + +/** + * VIR_DOMAIN_STATS_NET_SUFFIX_TX_PKTS: + * + * Packets transmitted as unsigned long long. + * + * Since: 11.2.0 + */ +#define VIR_DOMAIN_STATS_NET_SUFFIX_TX_PKTS ".rx.pkts"
".tx.pkts"
Doh, I did a before & after comparison, but seems the guest I used had a slirp NIC backend and so lacked these fields.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
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 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@redhat.com> --- 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
On Tue, Mar 04, 2025 at 14:04:08 +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@redhat.com> --- include/libvirt/libvirt-domain.h | 172 +++++++++++++++++++++++++++++++ src/libvirt-domain.c | 39 +------ src/qemu/qemu_driver.c | 79 ++++++++++---- 3 files changed, 233 insertions(+), 57 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
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@redhat.com> --- 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
On Tue, Mar 04, 2025 at 14:04:09 +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@redhat.com> --- include/libvirt/libvirt-domain.h | 224 +++++++++++++++++++++++++++++++ src/libvirt-domain.c | 68 +--------- src/qemu/qemu_driver.c | 35 ++++- 3 files changed, 260 insertions(+), 67 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
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@redhat.com> --- 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
On Tue, Mar 04, 2025 at 14:04:10 +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@redhat.com> --- include/libvirt/libvirt-domain.h | 56 ++++++++++++++++++++++++++++++++ src/libvirt-domain.c | 24 ++------------ src/qemu/qemu_driver.c | 12 ++++--- 3 files changed, 66 insertions(+), 26 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
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@redhat.com> --- 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
On Tue, Mar 04, 2025 at 14:04:11 +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@redhat.com> --- 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/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,
[...]
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);
formatting
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);
... Reviewed-by: Peter Krempa <pkrempa@redhat.com>
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@redhat.com> --- 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
On Tue, Mar 04, 2025 at 14:04:12 +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@redhat.com> --- 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/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,
[...]
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);
formatting
} }
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
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@redhat.com> --- 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
On Tue, Mar 04, 2025 at 14:04:13 +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@redhat.com> --- 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. - *
This ought to be squashed into 17/19.
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@redhat.com> --- 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
On Tue, Mar 04, 2025 at 14:04:14 +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@redhat.com> --- include/libvirt/libvirt-domain.h | 12 ++++++++++++ src/libvirt-domain.c | 12 +++++------- 2 files changed, 17 insertions(+), 7 deletions(-)
[...]
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. *
Please don't disturb the star culmination ;)
* WARNING: * The stats reported in this group are runtime-collected and
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
On Tue, Mar 04, 2025 at 02:03:55PM +0000, Daniel P. Berrangé wrote:
As proof of the value, adding these constants has lead to be find sooooooooooo many mistakes in the Golang bindings where we forgot to add new domain stats/guest info data items, and one place where we typod the name.
Illustrated in https://gitlab.com/libvirt/libvirt-go-module/-/merge_requests/67 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 :|
participants (2)
-
Daniel P. Berrangé -
Peter Krempa