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

2014-08-28 Thread 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





signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2014-08-27 Thread Peter Krempa
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,
+
+VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS = 1  31, /* enforce 
requested stats */
+} virConnectGetAllDomainStatsFlags;
+
 int virConnectGetAllDomainStats(virConnectPtr conn,
 unsigned int stats,
 virDomainStatsRecordPtr **retStats,
diff --git a/src/libvirt.c b/src/libvirt.c
index 17ec679..cae93f9 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -21549,6 +21549,26 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
  * Using 0 for @stats returns all stats groups supported by the given
  * hypervisor.
  *
+ * Specifying VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS as @flags makes
+ * the function return error in case some of the stat types in @stats were
+ * not recognized by the daemon.
+ *
+ * Similarly to virConnectListAllDomains @flags can contain various flags to
+ * filter the list of domains to provide stats for.
+ *
+ * VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE selects online domains while
+ * VIR_CONNECT_GET_ALL_DOMAINS_STATS_INACTIVE selects offline ones.
+ *
+ * VIR_CONNECT_GET_ALL_DOMAINS_STATS_PERSISTENT and
+ * VIR_CONNECT_GET_ALL_DOMAINS_STATS_TRANSIENT allow to filter the list
+ * according to their persistence.
+ *
+ * To filter the list of VMs by domain state @flags can contain
+ * VIR_CONNECT_GET_ALL_DOMAINS_STATS_RUNNING,
+ * VIR_CONNECT_GET_ALL_DOMAINS_STATS_PAUSED,
+ * VIR_CONNECT_GET_ALL_DOMAINS_STATS_SHUTOFF and/or
+ * VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER for all other states.
+ *
  * Returns the count of returned statistics structures on success, -1 on error.
  * The requested data are returned in the @retStats parameter. The returned
  * array should be freed by the caller. See virDomainStatsRecordListFree.
@@ -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;
  *
  * Query statistics for domains provided by @doms. Note that all domains in
  * @doms must share the same connection.
@@ -21615,6 +21635,13 @@ virConnectGetAllDomainStats(virConnectPtr conn,
  * Using 0 for @stats returns all stats groups supported by the given
  * hypervisor.
  *
+ * Specifying VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS as @flags makes
+ * the function return error in case some of the stat types in @stats were
+ * not recognized by the daemon.
+ *
+ * Note that specifying any of the domain list filtering flags in @flags
+ * doesn't have effect on this function.
+ *
  * Returns the count of returned statistics structures on success, -1 on error.
  * The requested data are returned in the @retStats parameter. The returned
  * array should be freed by the caller. See virDomainStatsRecordListFree.
-- 
2.0.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


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

2014-08-27 Thread Eric Blake
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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list