[libvirt] [PATCH v2 5/8] blockcopy: tweak how rebase calls into copy

Eric Blake eblake at redhat.com
Tue Aug 26 11:21:26 UTC 2014


In order to implement the new virDomainBlockCopy, the existing
block copy internal implementation needs to be adjusted.  The
new function will parse XML into a storage source, and parse
typed parameters into integers, then call into the same common
backend.  For now, it's easier to keep the same implementation
limits that only local file destinations are suported, but now
the check needs to be explicit.  Furthermore, the existing
semantics of allocating vm in one function and freeing it in
another is awkward to work with, but the helper function has
to be able to tell the caller if the domain unexpectedly died
while locks were dropped for running a monitor command.

* src/qemu/qemu_driver.c (qemuDomainBlockCopy): Rename...
(qemuDomainBlockCopyCommon): ...and adjust parameters.
(qemuDomainBlockRebase): Adjust caller.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 src/qemu/qemu_driver.c | 104 +++++++++++++++++++++++++++++--------------------
 1 file changed, 61 insertions(+), 43 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ad75bd9..f3c5387 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15276,12 +15276,14 @@ qemuDomainBlockJobSetSpeed(virDomainPtr dom, const char *path,
 }

 static int
-qemuDomainBlockCopy(virDomainObjPtr vm,
-                    virConnectPtr conn,
-                    const char *path,
-                    const char *dest, const char *format,
-                    unsigned long bandwidth, unsigned int flags)
+qemuDomainBlockCopyCommon(virDomainObjPtr *vmptr,
+                          virConnectPtr conn,
+                          const char *path,
+                          virStorageSourcePtr dest,
+                          unsigned long bandwidth,
+                          unsigned int flags)
 {
+    virDomainObjPtr vm = *vmptr;
     virQEMUDriverPtr driver = conn->privateData;
     qemuDomainObjPrivatePtr priv;
     char *device = NULL;
@@ -15292,10 +15294,11 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
     bool need_unlink = false;
     virStorageSourcePtr mirror = NULL;
     virQEMUDriverConfigPtr cfg = NULL;
+    const char *format = NULL;

     /* Preliminaries: find the disk we are editing, sanity checks */
-    virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
-                  VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT, -1);
+    virCheckFlags(VIR_DOMAIN_BLOCK_COPY_SHALLOW |
+                  VIR_DOMAIN_BLOCK_COPY_REUSE_EXT, -1);

     priv = vm->privateData;
     cfg = virQEMUDriverGetConfig(driver);
@@ -15341,7 +15344,7 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
         goto endjob;

     if ((flags & VIR_DOMAIN_BLOCK_REBASE_SHALLOW) &&
-        STREQ_NULLABLE(format, "raw") &&
+        dest->format == VIR_STORAGE_FILE_RAW &&
         disk->src->backingStore->path) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("disk '%s' has backing file, so raw shallow copy "
@@ -15351,15 +15354,20 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
     }

     /* Prepare the destination file.  */
-    if (stat(dest, &st) < 0) {
+    /* XXX Allow non-file mirror destinations */
+    if (!virStorageSourceIsLocalStorage(dest)) {
+        virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+                       _("non-file destination not supported yet"));
+    }
+    if (stat(dest->path, &st) < 0) {
         if (errno != ENOENT) {
             virReportSystemError(errno, _("unable to stat for disk %s: %s"),
-                                 disk->dst, dest);
+                                 disk->dst, dest->path);
             goto endjob;
         } else if (flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT) {
             virReportSystemError(errno,
                                  _("missing destination file for disk %s: %s"),
-                                 disk->dst, dest);
+                                 disk->dst, dest->path);
             goto endjob;
         }
     } else if (!S_ISBLK(st.st_mode) && st.st_size &&
@@ -15367,22 +15375,14 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("external destination file for disk %s already "
                          "exists and is not a block device: %s"),
-                       disk->dst, dest);
+                       disk->dst, dest->path);
         goto endjob;
     }

-    if (VIR_ALLOC(mirror) < 0)
+    if (!(mirror = virStorageSourceCopy(dest, false)))
         goto endjob;
-    /* XXX Allow non-file mirror destinations */
-    mirror->type = VIR_STORAGE_TYPE_FILE;

-    if (format) {
-        if ((mirror->format = virStorageFileFormatTypeFromString(format)) <= 0) {
-            virReportError(VIR_ERR_INVALID_ARG, _("unrecognized format '%s'"),
-                           format);
-            goto endjob;
-        }
-    } else {
+    if (!dest->format) {
         if (!(flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT)) {
             mirror->format = disk->src->format;
         } else {
@@ -15390,24 +15390,23 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
              * also passed the RAW flag (and format is non-NULL), or it is
              * safe for us to probe the format from the file that we will
              * be using.  */
-            mirror->format = virStorageFileProbeFormat(dest, cfg->user,
+            mirror->format = virStorageFileProbeFormat(dest->path, cfg->user,
                                                        cfg->group);
         }
     }

     /* pre-create the image file */
     if (!(flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT)) {
-        int fd = qemuOpenFile(driver, vm, dest, O_WRONLY | O_TRUNC | O_CREAT,
+        int fd = qemuOpenFile(driver, vm, dest->path,
+                              O_WRONLY | O_TRUNC | O_CREAT,
                               &need_unlink, NULL);
         if (fd < 0)
             goto endjob;
         VIR_FORCE_CLOSE(fd);
     }

-    if (!format && mirror->format > 0)
+    if (mirror->format > 0)
         format = virStorageFileFormatTypeToString(mirror->format);
-    if (VIR_STRDUP(mirror->path, dest) < 0)
-        goto endjob;

     if (virStorageSourceInitChainElement(mirror, disk->src, false) < 0)
         goto endjob;
@@ -15421,8 +15420,8 @@ qemuDomainBlockCopy(virDomainObjPtr vm,

     /* Actually start the mirroring */
     qemuDomainObjEnterMonitor(driver, vm);
-    ret = qemuMonitorDriveMirror(priv->mon, device, dest, format, bandwidth,
-                                 flags);
+    ret = qemuMonitorDriveMirror(priv->mon, device, dest->path, format,
+                                 bandwidth, flags);
     virDomainAuditDisk(vm, NULL, mirror, "mirror", ret >= 0);
     qemuDomainObjExitMonitor(driver, vm);
     if (ret < 0) {
@@ -15442,16 +15441,14 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
                  vm->def->name);

  endjob:
-    if (need_unlink && unlink(dest))
-        VIR_WARN("unable to unlink just-created %s", dest);
+    if (need_unlink && unlink(dest->path))
+        VIR_WARN("unable to unlink just-created %s", dest->path);
     virStorageSourceFree(mirror);
     if (!qemuDomainObjEndJob(driver, vm))
-        vm = NULL;
+        *vmptr = NULL;

  cleanup:
     VIR_FREE(device);
-    if (vm)
-        virObjectUnlock(vm);
     virObjectUnref(cfg);
     return ret;
 }
@@ -15461,6 +15458,8 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
                       unsigned long bandwidth, unsigned int flags)
 {
     virDomainObjPtr vm;
+    int ret = -1;
+    virStorageSourcePtr dest = NULL;

     virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
                   VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |
@@ -15476,17 +15475,36 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
         return -1;
     }

-    if (flags & VIR_DOMAIN_BLOCK_REBASE_COPY) {
-        const char *format = NULL;
-        if (flags & VIR_DOMAIN_BLOCK_REBASE_COPY_RAW)
-            format = "raw";
-        flags &= ~(VIR_DOMAIN_BLOCK_REBASE_COPY |
-                   VIR_DOMAIN_BLOCK_REBASE_COPY_RAW);
-        return qemuDomainBlockCopy(vm, dom->conn, path, base, format, bandwidth, flags);
+    if (!(flags & VIR_DOMAIN_BLOCK_REBASE_COPY))
+        return qemuDomainBlockJobImpl(vm, dom->conn, path, base, bandwidth,
+                                      NULL, BLOCK_JOB_PULL, flags);
+
+    if (VIR_ALLOC(dest) < 0)
+        goto cleanup;
+    dest->type = VIR_STORAGE_TYPE_FILE;
+    if (VIR_STRDUP(dest->path, base) < 0)
+        goto cleanup;
+    if (flags & VIR_DOMAIN_BLOCK_REBASE_COPY_RAW)
+        dest->format = VIR_STORAGE_FILE_RAW;
+    flags &= ~(VIR_DOMAIN_BLOCK_REBASE_COPY |
+               VIR_DOMAIN_BLOCK_REBASE_COPY_RAW);
+    if (flags & VIR_DOMAIN_BLOCK_REBASE_RELATIVE) {
+        /* FIXME: should this check be in libvirt.c? */
+        virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+                       _("Relative rebase incompatible with block copy"));
+        goto cleanup;
     }
+    /* We rely on the fact that VIR_DOMAIN_BLOCK_REBASE_SHALLOW
+     * and VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT map to the same value
+     * as for block copy. */
+    ret = qemuDomainBlockCopyCommon(&vm, dom->conn, path, dest,
+                                    bandwidth, flags);

-    return qemuDomainBlockJobImpl(vm, dom->conn, path, base, bandwidth, NULL,
-                                  BLOCK_JOB_PULL, flags);
+ cleanup:
+    if (vm)
+        virObjectUnlock(vm);
+    virStorageSourceFree(dest);
+    return ret;
 }

 static int
-- 
1.9.3




More information about the libvir-list mailing list