From 1aefc4d0bab87ed9b1a531fad029cb0c24db220f Mon Sep 17 00:00:00 2001

Message-Id: <1aefc4d0bab87ed9b1a531fad029cb0c24db220f@dist-git>

From: Eric Blake <eblake@redhat.com>

Date: Tue, 17 Mar 2015 15:12:49 -0600

Subject: [PATCH] qemu: read backing chain names from qemu

7.1.z: https://bugzilla.redhat.com/show_bug.cgi?id=1203119

7.2: https://bugzilla.redhat.com/show_bug.cgi?id=1199182

https://bugzilla.redhat.com/show_bug.cgi?id=1199182 documents that

after a series of disk snapshots into existing destination images,

followed by active commits of the top image, it is possible for

qemu 2.2 and earlier to end up tracking a different name for the

image than what it would have had when opening the chain afresh.

That is, when starting with the chain 'a <- b <- c', the name

associated with 'b' is how it was spelled in the metadata of 'c',

but when starting with 'a', taking two snapshots into 'a <- b <- c',

then committing 'c' back into 'b', the name associated with 'b' is

now the name used when taking the first snapshot.

Sadly, older qemu doesn't know how to treat different spellings of

the same filename as identical files (it uses strcmp() instead of

checking for the same inode), which means libvirt's attempt to

commit an image using solely the names learned from qcow2 metadata

fails with a cryptic:

error: internal error: unable to execute QEMU command 'block-commit': Top image file /tmp/images/c/../b/b not found

even though the file exists.  Trying to teach libvirt the rules on

which name qemu will expect is not worth the effort (besides, we'd

have to remember it across libvirtd restarts, and track whether a

file was opened via metadata or via snapshot creation for a given

qemu process); it is easier to just always directly ask qemu what

string it expects to see in the first place.

As a safety valve, we validate that any name returned by qemu

still maps to the same local file as we have tracked it, so that

a compromised qemu cannot accidentally cause us to act on an

incorrect file.

* src/qemu/qemu_monitor.h (qemuMonitorDiskNameLookup): New

prototype.

* src/qemu/qemu_monitor_json.h (qemuMonitorJSONDiskNameLookup):

Likewise.

* src/qemu/qemu_monitor.c (qemuMonitorDiskNameLookup): New function.

* src/qemu/qemu_monitor_json.c (qemuMonitorJSONDiskNameLookup)

(qemuMonitorJSONDiskNameLookupOne): Likewise.

* src/qemu/qemu_driver.c (qemuDomainBlockCommit)

(qemuDomainBlockJobImpl): Use it.

Signed-off-by: Eric Blake <eblake@redhat.com>

(cherry picked from commit f9ea3d60119e82c02c00fbf3678c3ed20634dea1)

Conflicts:

	src/qemu/qemu_driver.c - context with older monitor wrap semantics

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 src/qemu/qemu_driver.c       |  28 ++++++------

 src/qemu/qemu_monitor.c      |  18 ++++++++

 src/qemu/qemu_monitor.h      |   8 +++-

 src/qemu/qemu_monitor_json.c | 102 ++++++++++++++++++++++++++++++++++++++++++-

 src/qemu/qemu_monitor_json.h |   9 +++-

 5 files changed, 148 insertions(+), 17 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c

index 9dc243a..4293817 100644

--- a/src/qemu/qemu_driver.c

+++ b/src/qemu/qemu_driver.c

@@ -15720,9 +15720,6 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,

         goto endjob;

     if (baseSource) {

-        if (qemuGetDriveSourceString(baseSource, NULL, &basePath) < 0)

-            goto endjob;

-

         if (flags & VIR_DOMAIN_BLOCK_REBASE_RELATIVE) {

             if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CHANGE_BACKING_FILE)) {

                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",

@@ -15746,8 +15743,12 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,

     }

     qemuDomainObjEnterMonitor(driver, vm);

-    ret = qemuMonitorBlockJob(priv->mon, device, basePath, backingPath,

-                              bandwidth, info, mode, async);

+    if (baseSource)

+        basePath = qemuMonitorDiskNameLookup(priv->mon, device, disk->src,

+                                             baseSource);

+    if (!baseSource || basePath)

+        ret = qemuMonitorBlockJob(priv->mon, device, basePath, backingPath,

+                                  bandwidth, info, mode, async);

     qemuDomainObjExitMonitor(driver, vm);

     if (info && info->type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT &&

         disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)

@@ -16313,12 +16314,6 @@ qemuDomainBlockCommit(virDomainPtr dom,

                                            VIR_DISK_CHAIN_READ_WRITE) < 0))

         goto endjob;

-    if (qemuGetDriveSourceString(topSource, NULL, &topPath) < 0)

-        goto endjob;

-

-    if (qemuGetDriveSourceString(baseSource, NULL, &basePath) < 0)

-        goto endjob;

-

     if (flags & VIR_DOMAIN_BLOCK_COMMIT_RELATIVE &&

         topSource != disk->src) {

         if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CHANGE_BACKING_FILE)) {

@@ -16349,9 +16344,14 @@ qemuDomainBlockCommit(virDomainPtr dom,

         disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT;

     }

     qemuDomainObjEnterMonitor(driver, vm);

-    ret = qemuMonitorBlockCommit(priv->mon, device,

-                                 topPath, basePath, backingPath,

-                                 bandwidth);

+    basePath = qemuMonitorDiskNameLookup(priv->mon, device, disk->src,

+                                         baseSource);

+    topPath = qemuMonitorDiskNameLookup(priv->mon, device, disk->src,

+                                        topSource);

+    if (basePath && topPath)

+        ret = qemuMonitorBlockCommit(priv->mon, device,

+                                     topPath, basePath, backingPath,

+                                     bandwidth);

     qemuDomainObjExitMonitor(driver, vm);

     if (ret == 0)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c

index 0b1b80e..2bb6fdb 100644

--- a/src/qemu/qemu_monitor.c

+++ b/src/qemu/qemu_monitor.c

@@ -3464,6 +3464,24 @@ qemuMonitorSupportsActiveCommit(qemuMonitorPtr mon)

 }

+/* Determine the name that qemu is using for tracking the backing

+ * element TARGET within the chain starting at TOP.  */

+char *

+qemuMonitorDiskNameLookup(qemuMonitorPtr mon,

+                          const char *device,

+                          virStorageSourcePtr top,

+                          virStorageSourcePtr target)

+{

+    if (!mon->json) {

+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",

+                       _("JSON monitor is required"));

+        return NULL;

+    }

+

+    return qemuMonitorJSONDiskNameLookup(mon, device, top, target);

+}

+

+

 /* Use the block-job-complete monitor command to pivot a block copy

  * job.  */

 int

diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h

index 8930744..df6a8c0 100644

--- a/src/qemu/qemu_monitor.h

+++ b/src/qemu/qemu_monitor.h

@@ -1,7 +1,7 @@

 /*

  * qemu_monitor.h: interaction with QEMU monitor console

  *

- * Copyright (C) 2006-2014 Red Hat, Inc.

+ * Copyright (C) 2006-2015 Red Hat, Inc.

  * Copyright (C) 2006 Daniel P. Berrange

  *

  * This library is free software; you can redistribute it and/or

@@ -739,6 +739,12 @@ int qemuMonitorBlockCommit(qemuMonitorPtr mon,

     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)

     ATTRIBUTE_NONNULL(4);

 bool qemuMonitorSupportsActiveCommit(qemuMonitorPtr mon);

+char *qemuMonitorDiskNameLookup(qemuMonitorPtr mon,

+                                const char *device,

+                                virStorageSourcePtr top,

+                                virStorageSourcePtr target)

+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)

+    ATTRIBUTE_NONNULL(4);

 int qemuMonitorArbitraryCommand(qemuMonitorPtr mon,

                                 const char *cmd,

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c

index 9b3d17a..e58c88b 100644

--- a/src/qemu/qemu_monitor_json.c

+++ b/src/qemu/qemu_monitor_json.c

@@ -1,7 +1,7 @@

 /*

  * qemu_monitor_json.c: interaction with QEMU monitor console

  *

- * Copyright (C) 2006-2014 Red Hat, Inc.

+ * Copyright (C) 2006-2015 Red Hat, Inc.

  * Copyright (C) 2006 Daniel P. Berrange

  *

  * This library is free software; you can redistribute it and/or

@@ -4221,6 +4221,106 @@ qemuMonitorJSONDrivePivot(qemuMonitorPtr mon, const char *device,

 }

+static char *

+qemuMonitorJSONDiskNameLookupOne(virJSONValuePtr image,

+                                 virStorageSourcePtr top,

+                                 virStorageSourcePtr target)

+{

+    virJSONValuePtr backing;

+    char *ret;

+

+    /* The caller will report a generic message if we return NULL

+     * without an error; but in some cases we can improve by reporting

+     * a more specific message.  */

+    if (!top || !image)

+        return NULL;

+    if (top != target) {

+        backing = virJSONValueObjectGet(image, "backing-image");

+        return qemuMonitorJSONDiskNameLookupOne(backing, top->backingStore,

+                                                target);

+    }

+    if (VIR_STRDUP(ret, virJSONValueObjectGetString(image, "filename")) < 0)

+        return NULL;

+    /* Sanity check - the name qemu gave us should resolve to the same

+       file tracked by our target description. */

+    if (virStorageSourceIsLocalStorage(target) &&

+        STRNEQ(ret, target->path) &&

+        !virFileLinkPointsTo(ret, target->path)) {

+        virReportError(VIR_ERR_INTERNAL_ERROR,

+                       _("qemu block name '%s' doesn't match expected '%s'"),

+                       ret, target->path);

+        VIR_FREE(ret);

+    }

+    return ret;

+}

+

+

+char *

+qemuMonitorJSONDiskNameLookup(qemuMonitorPtr mon,

+                              const char *device,

+                              virStorageSourcePtr top,

+                              virStorageSourcePtr target)

+{

+    char *ret = NULL;

+    virJSONValuePtr cmd = NULL;

+    virJSONValuePtr reply = NULL;

+    virJSONValuePtr devices;

+    size_t i;

+

+    cmd = qemuMonitorJSONMakeCommand("query-block", NULL);

+    if (!cmd)

+        return NULL;

+    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)

+        goto cleanup;

+

+    devices = virJSONValueObjectGet(reply, "return");

+    if (!devices || devices->type != VIR_JSON_TYPE_ARRAY) {

+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",

+                       _("block info reply was missing device list"));

+        goto cleanup;

+    }

+

+    for (i = 0; i < virJSONValueArraySize(devices); i++) {

+        virJSONValuePtr dev = virJSONValueArrayGet(devices, i);

+        virJSONValuePtr inserted;

+        virJSONValuePtr image;

+        const char *thisdev;

+

+        if (!dev || dev->type != VIR_JSON_TYPE_OBJECT) {

+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",

+                           _("block info device entry was not in expected format"));

+            goto cleanup;

+        }

+

+        if (!(thisdev = virJSONValueObjectGetString(dev, "device"))) {

+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",

+                           _("block info device entry was not in expected format"));

+            goto cleanup;

+        }

+

+        if (STREQ(thisdev, device)) {

+            if ((inserted = virJSONValueObjectGet(dev, "inserted")) &&

+                (image = virJSONValueObjectGet(inserted, "image"))) {

+                ret = qemuMonitorJSONDiskNameLookupOne(image, top, target);

+            }

+            break;

+        }

+    }

+    /* Guarantee an error when returning NULL, but don't override a

+     * more specific error if one was already generated.  */

+    if (!ret && !virGetLastError())

+        virReportError(VIR_ERR_INTERNAL_ERROR,

+                       _("unable to find backing name for device %s"),

+                       device);

+

+ cleanup:

+    virJSONValueFree(cmd);

+    virJSONValueFree(reply);

+

+    return ret;

+}

+

+

 int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon,

                                     const char *cmd_str,

                                     char **reply_str,

diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h

index ff20029..1028802 100644

--- a/src/qemu/qemu_monitor_json.h

+++ b/src/qemu/qemu_monitor_json.h

@@ -1,7 +1,7 @@

 /*

  * qemu_monitor_json.h: interaction with QEMU monitor console

  *

- * Copyright (C) 2006-2009, 2011-2014 Red Hat, Inc.

+ * Copyright (C) 2006-2009, 2011-2015 Red Hat, Inc.

  * Copyright (C) 2006 Daniel P. Berrange

  *

  * This library is free software; you can redistribute it and/or

@@ -280,6 +280,13 @@ int qemuMonitorJSONBlockCommit(qemuMonitorPtr mon,

                                unsigned long long bandwidth)

     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);

+char *qemuMonitorJSONDiskNameLookup(qemuMonitorPtr mon,

+                                    const char *device,

+                                    virStorageSourcePtr top,

+                                    virStorageSourcePtr target)

+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)

+    ATTRIBUTE_NONNULL(4);

+

 int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon,

                                     const char *cmd_str,

                                     char **reply_str,

-- 

2.3.3