[libvirt] [PATCHv3 17/19] blockjob: implement shallow commit flag in qemu

Eric Blake eblake at redhat.com
Wed Oct 17 22:30:28 UTC 2012


Now that we can crawl the chain of backing files, we can do
argument validation and implement the 'shallow' flag.  In
testing this, I discovered that it can be handy to pass the
shallow flag and an explicit base, as a means of validating
that the base is indeed the file we expected.

* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Crawl through
chain to implement shallow flag.
* src/libvirt.c (virDomainBlockCommit): Relax API.
---

v3: rebase to changes earlier in series

 src/libvirt.c          |  2 --
 src/qemu/qemu_driver.c | 63 ++++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c
index e3ddf27..6d65965 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -19381,8 +19381,6 @@ int virDomainBlockCommit(virDomainPtr dom, const char *disk,
     }

     virCheckNonNullArgGoto(disk, error);
-    if (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW)
-        virCheckNullArgGoto(base, error);

     if (conn->driver->domainBlockCommit) {
         int ret;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c63d16b..73804fe 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12664,8 +12664,12 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
     int ret = -1;
     int idx;
     virDomainDiskDefPtr disk;
+    const char *top_canon = NULL;
+    virStorageFileMetadataPtr top_meta = NULL;
+    const char *top_parent = NULL;
+    const char *base_canon = NULL;

-    virCheckFlags(0, -1);
+    virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW, -1);

     if (!(vm = qemuDomObjFromDomain(dom)))
         goto cleanup;
@@ -12696,20 +12700,55 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
                        disk->dst);
         goto endjob;
     }
+    if (qemuDomainDetermineDiskChain(driver, disk, false) < 0)
+        goto endjob;

-    if (!top)
-        top = disk->src;
+    if (!top) {
+        top_canon = disk->src;
+        top_meta = disk->backingChain;
+    } else if (!(top_canon = virStorageFileChainLookup(disk->backingChain,
+                                                       disk->src,
+                                                       top, &top_meta,
+                                                       &top_parent))) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("could not find top '%s' in chain for '%s'"),
+                       top, path);
+        goto endjob;
+    }
+    if (!top_meta || !top_meta->backingStore) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("top '%s' in chain for '%s' has no backing file"),
+                       top, path);
+        goto endjob;
+    }
+    if (!base && (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW)) {
+        base_canon = top_meta->backingStore;
+    } else if (!(base_canon = virStorageFileChainLookup(top_meta, top_canon,
+                                                        base, NULL, NULL))) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("could not find base '%s' below '%s' in chain "
+                         "for '%s'"),
+                       base ? base : "(default)", top_canon, path);
+        goto endjob;
+    }
+    /* Note that this code exploits the fact that
+     * virStorageFileChainLookup guarantees a simple pointer
+     * comparison will work, rather than needing full-blown STREQ.  */
+    if ((flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW) &&
+        base_canon != top_meta->backingStore) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("base '%s' is not immediately below '%s' in chain "
+                         "for '%s'"),
+                       base, top_canon, path);
+        goto endjob;
+    }

-    /* XXX For now, we are relying on qemu to check that 'top' and
-     * 'base' resolve to members of the backing chain in correct
-     * order; but if we ever get more paranoid and track the backing
-     * chain ourself, we should be pre-validating the data rather than
-     * relying on qemu.  For that matter, we need to be changing the
-     * SELinux label on both 'base' and the parent of 'top', so that
-     * qemu can open(O_RDWR) those files for the duration of the
-     * commit.  */
+    /* XXX We need to be changing the SELinux label on both 'base' and
+     * the parent of 'top', so that qemu can open(O_RDWR) those files
+     * for the duration of the commit.  */
     qemuDomainObjEnterMonitor(driver, vm);
-    ret = qemuMonitorBlockCommit(priv->mon, device, top, base, bandwidth);
+    ret = qemuMonitorBlockCommit(priv->mon, device, top_canon, base_canon,
+                                 bandwidth);
     qemuDomainObjExitMonitor(driver, vm);

 endjob:
-- 
1.7.11.7




More information about the libvir-list mailing list