Re: [libvirt] [PATCH 2/6] Add support for fetching statistics of completed jobs
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
> > ---
> > 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
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
> ---
> 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_DOMAI
[libvirt] [PATCH 2/6] Add support for fetching statistics of completed jobs
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
---
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,
-
