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.