Re: [libvirt] [PATCH 2/6] Add support for fetching statistics of completed jobs

2014-09-08 Thread Jiri Denemark
On Fri, Sep 05, 2014 at 14:42:00 -0400, John Ferlan wrote:
 
 
 On 09/01/2014 11:05 AM, Jiri Denemark wrote:
  virDomainGetJobStats gains new VIR_DOMAIN_JOB_STATS_COMPLETED flag that
  can be used to fetch statistics of a completed job rather than a
  currently running job.
  
  Signed-off-by: Jiri Denemark jdene...@redhat.com
  ---
   include/libvirt/libvirt.h.in | 11 +++
   src/libvirt.c|  8 +++-
   src/qemu/qemu_domain.c   |  1 +
   src/qemu/qemu_domain.h   |  1 +
   src/qemu/qemu_driver.c   | 25 +++--
   src/qemu/qemu_migration.c|  6 ++
   src/qemu/qemu_monitor_json.c |  3 ++-
   7 files changed, 47 insertions(+), 8 deletions(-)
  
 
 This seems AOK - see my note at the end about qemu_monitor_json.c though.
 
 
...
  diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
  index 62e7d5d..263fa8b 100644
  --- a/src/qemu/qemu_monitor_json.c
  +++ b/src/qemu/qemu_monitor_json.c
  @@ -2478,7 +2478,8 @@ 
  qemuMonitorJSONGetMigrationStatusReply(virJSONValuePtr reply,
   if (rc == 0)
   status-downtime_set = true;
   
  -if (status-status == QEMU_MONITOR_MIGRATION_STATUS_ACTIVE) {
  +if (status-status == QEMU_MONITOR_MIGRATION_STATUS_ACTIVE ||
  +status-status == QEMU_MONITOR_MIGRATION_STATUS_COMPLETED) {
   virJSONValuePtr ram = virJSONValueObjectGet(ret, ram);
   if (!ram) {
   virReportError(VIR_ERR_INTERNAL_ERROR, %s,
  
 
 While you're in this module - the following 3 calls don't check status
 (and were flagged by my new coverity):
 
 virJSONValueObjectGetNumberUlong(ret, total-time, status-total_time);
 
 virJSONValueObjectGetNumberUlong(ram, normal, status-ram_normal);
 
 virJSONValueObjectGetNumberUlong(ram, normal-bytes,
 status-ram_normal_bytes);

QEMU does not always report all values so we just ignore failures to get
some of them. I'll look at them and use ignore_value() when appropriate.

 I know somewhat outside the bounds of these changes but since they're
 in qemuMonitorJSONGetMigrationStatusReply() and used by the changes here
 - there's enough of a reason to adjust that I think.
 
 I don't see where total_time is ever used though...

Right, we compute total time ourselves. This is just for completeness to
parse all fields QEMU supports in case we want to use them sometime
somewhere :-)

Jirka

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


Re: [libvirt] [PATCH 2/6] Add support for fetching statistics of completed jobs

2014-09-05 Thread John Ferlan


On 09/01/2014 11:05 AM, Jiri Denemark wrote:
 virDomainGetJobStats gains new VIR_DOMAIN_JOB_STATS_COMPLETED flag that
 can be used to fetch statistics of a completed job rather than a
 currently running job.
 
 Signed-off-by: Jiri Denemark jdene...@redhat.com
 ---
  include/libvirt/libvirt.h.in | 11 +++
  src/libvirt.c|  8 +++-
  src/qemu/qemu_domain.c   |  1 +
  src/qemu/qemu_domain.h   |  1 +
  src/qemu/qemu_driver.c   | 25 +++--
  src/qemu/qemu_migration.c|  6 ++
  src/qemu/qemu_monitor_json.c |  3 ++-
  7 files changed, 47 insertions(+), 8 deletions(-)
 

This seems AOK - see my note at the end about qemu_monitor_json.c though.


 diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
 index 9358314..9670e06 100644
 --- a/include/libvirt/libvirt.h.in
 +++ b/include/libvirt/libvirt.h.in
 @@ -4306,6 +4306,17 @@ struct _virDomainJobInfo {
  unsigned long long fileRemaining;
  };
  
 +/**
 + * virDomainGetJobStatsFlags:
 + *
 + * Flags OR'ed together to provide specific behavior when querying domain
 + * job statistics.
 + */
 +typedef enum {
 +VIR_DOMAIN_JOB_STATS_COMPLETED = 1  0, /* return stats of a recently
 +  * completed job */
 +} virDomainGetJobStatsFlags;
 +
  int virDomainGetJobInfo(virDomainPtr dom,
  virDomainJobInfoPtr info);
  int virDomainGetJobStats(virDomainPtr domain,
 diff --git a/src/libvirt.c b/src/libvirt.c
 index 5d8f01c..6fa0a6b 100644
 --- a/src/libvirt.c
 +++ b/src/libvirt.c
 @@ -17567,7 +17567,7 @@ virDomainGetJobInfo(virDomainPtr domain, 
 virDomainJobInfoPtr info)
   * @type: where to store the job type (one of virDomainJobType)
   * @params: where to store job statistics
   * @nparams: number of items in @params
 - * @flags: extra flags; not used yet, so callers should always pass 0
 + * @flags: bitwise-OR of virDomainGetJobStatsFlags
   *
   * Extract information about progress of a background job on a domain.
   * Will return an error if the domain is not active. The function returns
 @@ -17577,6 +17577,12 @@ virDomainGetJobInfo(virDomainPtr domain, 
 virDomainJobInfoPtr info)
   * may receive fields that they do not understand in case they talk to a
   * newer server.
   *
 + * When @flags contains VIR_DOMAIN_JOB_STATS_COMPLETED, the function will
 + * return statistics about a recently completed job. Specifically, this
 + * flag may be used to query statistics of a completed incoming migration.
 + * Statistics of a completed job are automatically destroyed once read or
 + * when libvirtd is restarted.
 + *

   ^
Yeah - my question from patch 1 with respect to checking access for
job.current especially...  The job.completed seems fine.

   * Returns 0 in case of success and -1 in case of failure.
   */
  int
 diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
 index 2c709e9..18a3761 100644
 --- a/src/qemu/qemu_domain.c
 +++ b/src/qemu/qemu_domain.c
 @@ -199,6 +199,7 @@ static void
  qemuDomainObjFreeJob(qemuDomainObjPrivatePtr priv)
  {
  VIR_FREE(priv-job.current);
 +VIR_FREE(priv-job.completed);
  virCondDestroy(priv-job.cond);
  virCondDestroy(priv-job.asyncCond);
  }
 diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
 index 119590e..365238b 100644
 --- a/src/qemu/qemu_domain.h
 +++ b/src/qemu/qemu_domain.h
 @@ -124,6 +124,7 @@ struct qemuDomainJobObj {
  unsigned long long mask;/* Jobs allowed during async job */
  bool dump_memory_only;  /* use dump-guest-memory to do dump 
 */
  qemuDomainJobInfoPtr current;   /* async job progress data */
 +qemuDomainJobInfoPtr completed; /* statistics data of a recently 
 completed job */
  bool asyncAbort;/* abort of async job requested */
  };
  
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 265315d..cd6932a 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -11651,9 +11651,10 @@ qemuDomainGetJobStats(virDomainPtr dom,
  {
  virDomainObjPtr vm;
  qemuDomainObjPrivatePtr priv;
 +qemuDomainJobInfoPtr jobInfo;
  int ret = -1;
  
 -virCheckFlags(0, -1);
 +virCheckFlags(VIR_DOMAIN_JOB_STATS_COMPLETED, -1);
  
  if (!(vm = qemuDomObjFromDomain(dom)))
  goto cleanup;
 @@ -11663,13 +11664,19 @@ qemuDomainGetJobStats(virDomainPtr dom,
  if (virDomainGetJobStatsEnsureACL(dom-conn, vm-def)  0)
  goto cleanup;
  
 -if (!virDomainObjIsActive(vm)) {
 +if (!(flags  VIR_DOMAIN_JOB_STATS_COMPLETED) 
 +!virDomainObjIsActive(vm)) {
  virReportError(VIR_ERR_OPERATION_INVALID,
 %s, _(domain is not running));
  goto cleanup;
  }
  
 -if (!priv-job.current) {
 +if (flags  VIR_DOMAIN_JOB_STATS_COMPLETED)
 +jobInfo = priv-job.completed;
 +else
 +jobInfo = priv-job.current;
 +
 +

[libvirt] [PATCH 2/6] Add support for fetching statistics of completed jobs

2014-09-01 Thread Jiri Denemark
virDomainGetJobStats gains new VIR_DOMAIN_JOB_STATS_COMPLETED flag that
can be used to fetch statistics of a completed job rather than a
currently running job.

Signed-off-by: Jiri Denemark jdene...@redhat.com
---
 include/libvirt/libvirt.h.in | 11 +++
 src/libvirt.c|  8 +++-
 src/qemu/qemu_domain.c   |  1 +
 src/qemu/qemu_domain.h   |  1 +
 src/qemu/qemu_driver.c   | 25 +++--
 src/qemu/qemu_migration.c|  6 ++
 src/qemu/qemu_monitor_json.c |  3 ++-
 7 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 9358314..9670e06 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -4306,6 +4306,17 @@ struct _virDomainJobInfo {
 unsigned long long fileRemaining;
 };
 
+/**
+ * virDomainGetJobStatsFlags:
+ *
+ * Flags OR'ed together to provide specific behavior when querying domain
+ * job statistics.
+ */
+typedef enum {
+VIR_DOMAIN_JOB_STATS_COMPLETED = 1  0, /* return stats of a recently
+  * completed job */
+} virDomainGetJobStatsFlags;
+
 int virDomainGetJobInfo(virDomainPtr dom,
 virDomainJobInfoPtr info);
 int virDomainGetJobStats(virDomainPtr domain,
diff --git a/src/libvirt.c b/src/libvirt.c
index 5d8f01c..6fa0a6b 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -17567,7 +17567,7 @@ virDomainGetJobInfo(virDomainPtr domain, 
virDomainJobInfoPtr info)
  * @type: where to store the job type (one of virDomainJobType)
  * @params: where to store job statistics
  * @nparams: number of items in @params
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: bitwise-OR of virDomainGetJobStatsFlags
  *
  * Extract information about progress of a background job on a domain.
  * Will return an error if the domain is not active. The function returns
@@ -17577,6 +17577,12 @@ virDomainGetJobInfo(virDomainPtr domain, 
virDomainJobInfoPtr info)
  * may receive fields that they do not understand in case they talk to a
  * newer server.
  *
+ * When @flags contains VIR_DOMAIN_JOB_STATS_COMPLETED, the function will
+ * return statistics about a recently completed job. Specifically, this
+ * flag may be used to query statistics of a completed incoming migration.
+ * Statistics of a completed job are automatically destroyed once read or
+ * when libvirtd is restarted.
+ *
  * Returns 0 in case of success and -1 in case of failure.
  */
 int
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 2c709e9..18a3761 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -199,6 +199,7 @@ static void
 qemuDomainObjFreeJob(qemuDomainObjPrivatePtr priv)
 {
 VIR_FREE(priv-job.current);
+VIR_FREE(priv-job.completed);
 virCondDestroy(priv-job.cond);
 virCondDestroy(priv-job.asyncCond);
 }
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 119590e..365238b 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -124,6 +124,7 @@ struct qemuDomainJobObj {
 unsigned long long mask;/* Jobs allowed during async job */
 bool dump_memory_only;  /* use dump-guest-memory to do dump */
 qemuDomainJobInfoPtr current;   /* async job progress data */
+qemuDomainJobInfoPtr completed; /* statistics data of a recently 
completed job */
 bool asyncAbort;/* abort of async job requested */
 };
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 265315d..cd6932a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11651,9 +11651,10 @@ qemuDomainGetJobStats(virDomainPtr dom,
 {
 virDomainObjPtr vm;
 qemuDomainObjPrivatePtr priv;
+qemuDomainJobInfoPtr jobInfo;
 int ret = -1;
 
-virCheckFlags(0, -1);
+virCheckFlags(VIR_DOMAIN_JOB_STATS_COMPLETED, -1);
 
 if (!(vm = qemuDomObjFromDomain(dom)))
 goto cleanup;
@@ -11663,13 +11664,19 @@ qemuDomainGetJobStats(virDomainPtr dom,
 if (virDomainGetJobStatsEnsureACL(dom-conn, vm-def)  0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
+if (!(flags  VIR_DOMAIN_JOB_STATS_COMPLETED) 
+!virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_OPERATION_INVALID,
%s, _(domain is not running));
 goto cleanup;
 }
 
-if (!priv-job.current) {
+if (flags  VIR_DOMAIN_JOB_STATS_COMPLETED)
+jobInfo = priv-job.completed;
+else
+jobInfo = priv-job.current;
+
+if (!jobInfo) {
 *type = VIR_DOMAIN_JOB_NONE;
 *params = NULL;
 *nparams = 0;
@@ -11682,11 +11689,17 @@ qemuDomainGetJobStats(virDomainPtr dom,
  * of incoming migration which we don't currently
  * monitor actively in the background thread
  */
-if (qemuDomainJobInfoUpdateTime(priv-job.current)  0 ||
-qemuDomainJobInfoToParams(priv-job.current,
-