diff mbox

[libvirt,v2,3/8] Add support for fetching statistics of completed jobs

Message ID 57306D95.2080503@univention.de
State New
Headers show

Commit Message

Philipp Hahn May 9, 2016, 10:59 a.m. UTC 
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 <jdenemar@redhat.com>
...
> --- 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
diff mbox

Patch

From c4f6dfb25042f420fdd1728865686552d928d90e Mon Sep 17 00:00:00 2001
Message-Id: <c4f6dfb25042f420fdd1728865686552d928d90e.1462786017.git.hahn@univention.de>
From: Philipp Hahn <hahn@univention.de>
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 @@  qemuMonitorJSONGetMigrationStatusReply(virJSONValuePtr reply,
         virJSONValueObjectGetNumberUlong(ram, "normal", &status->ram_normal);
         virJSONValueObjectGetNumberUlong(ram, "normal-bytes",
                                          &status->ram_normal_bytes);
+        }
 
         virJSONValuePtr disk = virJSONValueObjectGet(ret, "disk");
         if (disk) {
-- 
2.1.4