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

[Peter Krempa]

On 08/27/14 22:50, Eric Blake wrote:
> 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.

I changed this sentence to:

 * Note that any of the domain list filtering flags in @flags will be rejected
 * by this function.
 *

> 
> Looks reasonable, so ACK with the grammar fixes
> 

And all the other tweaks you've suggested. 

Peter



-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140828/2069d8b8/attachment-0001.sig>