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.1Couple 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 :|
© 2016 - 2025 Red Hat, Inc.