Re: [libvirt] [PATCHv3 2/5] lib: Add few flags for the bulk stats APIs

On 08/27/2014 12:25 PM, Peter Krempa wrote: > Add domain list filtering functions and a flag to enforce checking > whether the remote daemon supports the requested stats groups. > --- > include/libvirt/libvirt.h.in | 15 +++++++++++++++ > src/libvirt.c | 29 ++++++++++++++++++++++++++++- > 2 files changed, 43 insertions(+), 1 deletion(-) > > diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in > index e79c9ad..9358314 100644 > --- a/include/libvirt/libvirt.h.in > +++ b/include/libvirt/libvirt.h.in > @@ -2513,6 +2513,21 @@ typedef enum { > VIR_DOMAIN_STATS_STATE = (1 << 0), /* return domain state */ > } virDomainStatsTypes; > > +typedef enum { > + VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE = VIR_CONNECT_LIST_DOMAINS_ACTIVE, > + VIR_CONNECT_GET_ALL_DOMAINS_STATS_INACTIVE = VIR_CONNECT_LIST_DOMAINS_INACTIVE, > + > + VIR_CONNECT_GET_ALL_DOMAINS_STATS_PERSISTENT = VIR_CONNECT_LIST_DOMAINS_PERSISTENT, > + VIR_CONNECT_GET_ALL_DOMAINS_STATS_TRANSIENT = VIR_CONNECT_LIST_DOMAINS_TRANSIENT, > + > + VIR_CONNECT_GET_ALL_DOMAINS_STATS_RUNNING = VIR_CONNECT_LIST_DOMAINS_RUNNING, > + VIR_CONNECT_GET_ALL_DOMAINS_STATS_PAUSED = VIR_CONNECT_LIST_DOMAINS_PAUSED, > + VIR_CONNECT_GET_ALL_DOMAINS_STATS_SHUTOFF = VIR_CONNECT_LIST_DOMAINS_SHUTOFF, > + VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER = VIR_CONNECT_LIST_DOMAINS_OTHER, Is this going to confuse the python bindings? (I know in other places where we have set up one enum to match another that it didn't seem to work). But fixing that would be a separate patch; it may be that docs/apibuild.py needs to learn how to do symbolic name dereferencing before generating the xml that feeds the python bindings (see also commit 9b291bb). > + * > + * Similarly to virConnectListAllDomains @flags can contain various flags to s/ListAllDomains/ListAllDomains,/ > @@ -21590,7 +21610,7 @@ virConnectGetAllDomainStats(virConnectPtr conn, > * @doms: NULL terminated array of domains > * @stats: stats to return, binary-OR of virDomainStatsTypes > * @retStats: Pointer that will be filled with the array of returned stats. > - * @flags: extra flags; not used yet, so callers should always pass 0 > + * @flags: extra flags; binary-OR of virConnectGetAllDomainStatsFlags; Why the trailing semicolon? > + * > + * Note that specifying any of the domain list filtering flags in @flags > + * doesn't have effect on this function. Fair enough, I suppose. But I'd rather we error out if filtering flags were requested, instead of silently ignoring them, so that if we later change our mind, we can do in-place filtering. I'll probably have more to say on this on a later patch.

Looks reasonable, so ACK with the grammar fixes