Re: [libvirt] [PATCH v2 3/8] Add support for fetching statistics of completed jobs
Hi,
FYI as tracking down that failure did cost me some time and hopefully
this summary will help other to avoid this situation:
Am 09.09.2014 um 11:54 schrieb 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
...
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -2500,7 +2500,8 @@ qemuMonitorJSONGetMigrationStatusReply(virJSONValuePtr
> reply,
...
> -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",
>_("migration was active, but RAM 'transferred' "
> "data was missing"));
This change from libvirt v1.2.9-rc1~203 breaks migration with
qemu-kvm-1.1.2, as that ancient implementation does *not* export
transfer statistics for completed jobs:
> qemuMonitorJSONCommandWithFd:286 : Send command
> '{"execute":"query-migrate","id":"libvirt-41"}' for write with FD -1
> qemuMonitorJSONIOProcessLine:179 : Line [{"return": {"status": "active",
> "ram": {"total": 2164654080, "remaining": 22474752, "transferred":
> 175117413}}, "id": "libvirt-41"}]
...
> qemuMonitorJSONCommandWithFd:286 : Send command
> '{"execute":"query-migrate","id":"libvirt-42"}' for write with FD -1
> qemuMonitorJSONIOProcessLine:179 : Line [{"return": {"status": "completed"},
> "id": "libvirt-42"}]
As you can see, there is not "ram" section and the migration is aborted
with the message:
> internal error: migration was active, but no RAM info was set
qemu-kvm/qmp-commands.hx even states this:
> - "ram": only present if "status" is "active", it is a json-object with the
> following RAM information (in bytes):
but the example some lines below is wrong:
> 2. Migration is done and has succeeded
>
> -> { "execute": "query-migrate" }
> <- { "return": {
> "status": "completed",
> "ram":{
That example has been updated by v1.2.0-rc0~29^2~2, but forgot to update
the specification above.
The attached hack for libvirt makes migration work again for me by
making "ram" optional in case of "completed".
Philipp
From c4f6dfb25042f420fdd1728865686552d928d90e Mon Sep 17 00:00:00 2001
Message-Id:
From: Philipp Hahn
Date: Mon, 9 May 2016 10:52:09 +0200
Subject: [PATCH] Bug #40318 libvirt: Handle qemu-kvm-1.1.2 migration
incompatibility
Organization: Univention GmbH, Bremen, Germany
To: libvir-list@redhat.com
The change from libvirt v1.2.9-rc1~203 breaks migration with qemu-kvm-1.1.2, as
that ancient implementation does *not* export transfer statistics for completed
jobs:
> qemuMonitorJSONCommandWithFd:286 : Send command '{"execute":"query-migrate","id":"libvirt-41"}' for write with FD -1
> qemuMonitorJSONIOProcessLine:179 : Line [{"return": {"status": "active", "ram": {"total": 2164654080, "remaining": 22474752, "transferred": 175117413}}, "id": "libvirt-41"}]
...
> qemuMonitorJSONCommandWithFd:286 : Send command '{"execute":"query-migrate","id":"libvirt-42"}' for write with FD -1
> qemuMonitorJSONIOProcessLine:179 : Line [{"return": {"status": "completed"}, "id": "libvirt-42"}]
As you can see, there is not "ram" section and the migration is aborted with
the message:
> internal error: migration was active, but no RAM info was set
qemu-kvm/qmp-commands.hx even states this:
> - "ram": only present if "status" is "active", it is a json-object with the
> following RAM information (in bytes):
but the example some lines below is wrong:
> 2. Migration is done and has succeeded
>
> -> { "execute": "query-migrate" }
> <- { "return": {
> "status": "completed",
> "ram":{
That example has been updated by v1.2.0-rc0~29^2~2, but forgot to update the
specification above.
---
src/qemu/qemu_monitor_json.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index a62c02f..d3b7b90 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2483,8 +2483,11 @@ qemuMonitorJSONGetMigrationStatusReply(virJSONValuePtr reply,
if (!ram) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("migration was active, but no RAM info was set"));
+if (status->status == QEMU_MONITOR_MIGRATION_STATUS_ACTIVE) {
+// qemu-kvm-1.1.2 does NOT report 'ram':{...} on complete
return -1;
-}
+}
+} else {
if (virJSONValueObjectGetNumberUlong(ram, "transferred",
&status->ram_transferred) < 0) {
@@ -2514,6 +2517,7 @@ qemuMonitorJSONGetMigrationS
[libvirt] [PATCH v2 3/8] 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
---
Notes:
Version 2:
- no change
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 aced31c..94b942c 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -4320,6 +4320,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 4806535..00b4d6f 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 8c94e27..78d6e8c 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 99c7d6a..2e16cde 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 a5533c5..fd82af1 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 ||
-qemuDomainJobIn
