[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]