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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list