[libvirt] [PATCH 4/5] conf: store mirroring information in virStorageSource

Eric Blake eblake at redhat.com
Thu May 22 04:50:47 UTC 2014


The current implementation of 'virsh blockcopy' (virDomainBlockRebase)
is limited to copying to a local file name.  But future patches want
to extend it to also copy to network disks.  This patch converts over
to a virStorageSourcePtr, although it should have no semantic change
visible to the user, in anticipation of those future patches being
able to use more fields for non-file destinations.

* src/conf/domain_conf.h (_virDomainDiskDef): Change type of
mirror information.
* src/conf/domain_conf.c (virDomainDiskDefParseXML): Localize
mirror parsing into new object.
(virDomainDiskDefFormat): Adjust clients.
* src/qemu/qemu_domain.c (qemuDomainDeviceDefPostParse):
Likewise.
* src/qemu/qemu_driver.c (qemuDomainBlockPivot)
(qemuDomainBlockJobImpl, qemuDomainBlockCopy): Likewise.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 src/conf/domain_conf.c | 50 +++++++++++++++++++++----------------------
 src/conf/domain_conf.h |  3 +--
 src/qemu/qemu_domain.c |  8 +++----
 src/qemu/qemu_driver.c | 58 +++++++++++++++++++++++++++++++-------------------
 4 files changed, 66 insertions(+), 53 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 45f2691..57142cb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5202,9 +5202,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
     char *ioeventfd = NULL;
     char *event_idx = NULL;
     char *copy_on_read = NULL;
-    char *mirror = NULL;
-    char *mirrorFormat = NULL;
-    bool mirroring = false;
     char *devaddr = NULL;
     virStorageEncryptionPtr encryption = NULL;
     char *serial = NULL;
@@ -5353,19 +5350,37 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
                 event_idx = virXMLPropString(cur, "event_idx");
                 copy_on_read = virXMLPropString(cur, "copy_on_read");
                 discard = virXMLPropString(cur, "discard");
-            } else if (!mirror && xmlStrEqual(cur->name, BAD_CAST "mirror") &&
+            } else if (!def->mirror &&
+                       xmlStrEqual(cur->name, BAD_CAST "mirror") &&
                        !(flags & VIR_DOMAIN_XML_INACTIVE)) {
                 char *ready;
-                mirror = virXMLPropString(cur, "file");
-                if (!mirror) {
+                char *mirrorFormat;
+
+                if (VIR_ALLOC(def->mirror) < 0)
+                    goto error;
+                def->mirror->type = VIR_STORAGE_TYPE_FILE;
+                def->mirror->path = virXMLPropString(cur, "file");
+                if (!def->mirror->path) {
                     virReportError(VIR_ERR_XML_ERROR, "%s",
                                    _("mirror requires file name"));
                     goto error;
                 }
                 mirrorFormat = virXMLPropString(cur, "format");
+                if (mirrorFormat) {
+                    def->mirror->format =
+                        virStorageFileFormatTypeFromString(mirrorFormat);
+                    if (def->mirror->format <= 0) {
+                        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                                       _("unknown mirror format value '%s'"),
+                                       mirrorFormat);
+                        VIR_FREE(mirrorFormat);
+                        goto error;
+                    }
+                    VIR_FREE(mirrorFormat);
+                }
                 ready = virXMLPropString(cur, "ready");
                 if (ready) {
-                    mirroring = true;
+                    def->mirroring = true;
                     VIR_FREE(ready);
                 }
             } else if (xmlStrEqual(cur->name, BAD_CAST "auth")) {
@@ -5885,9 +5900,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
     authUsername = NULL;
     def->src->driverName = driverName;
     driverName = NULL;
-    def->mirror = mirror;
-    mirror = NULL;
-    def->mirroring = mirroring;
     def->src->encryption = encryption;
     encryption = NULL;
     def->serial = serial;
@@ -5909,16 +5921,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
         }
     }

-    if (mirrorFormat) {
-        def->mirrorFormat = virStorageFileFormatTypeFromString(mirrorFormat);
-        if (def->mirrorFormat <= 0) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                           _("unknown mirror format value '%s'"),
-                           driverType);
-            goto error;
-        }
-    }
-
     if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE
         && virDomainDiskDefAssignAddress(xmlopt, def) < 0)
         goto error;
@@ -5943,8 +5945,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
     VIR_FREE(authUsage);
     VIR_FREE(driverType);
     VIR_FREE(driverName);
-    VIR_FREE(mirror);
-    VIR_FREE(mirrorFormat);
     VIR_FREE(cachetag);
     VIR_FREE(error_policy);
     VIR_FREE(rerror_policy);
@@ -15136,10 +15136,10 @@ virDomainDiskDefFormat(virBufferPtr buf,
      * for live domains, therefore we ignore it on input except for
      * the internal parse on libvirtd restart.  */
     if (def->mirror && !(flags & VIR_DOMAIN_XML_INACTIVE)) {
-        virBufferEscapeString(buf, "<mirror file='%s'", def->mirror);
-        if (def->mirrorFormat)
+        virBufferEscapeString(buf, "<mirror file='%s'", def->mirror->path);
+        if (def->mirror->format)
             virBufferAsprintf(buf, " format='%s'",
-                              virStorageFileFormatTypeToString(def->mirrorFormat));
+                              virStorageFileFormatTypeToString(def->mirror->format));
         if (def->mirroring)
             virBufferAddLit(buf, " ready='yes'");
         virBufferAddLit(buf, "/>\n");
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index f388865..ca145a6 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -606,8 +606,7 @@ struct _virDomainDiskDef {
     int tray_status; /* enum virDomainDiskTray */
     int removable; /* enum virDomainFeatureState */

-    char *mirror;
-    int mirrorFormat; /* virStorageFileFormat */
+    virStorageSourcePtr mirror;
     bool mirroring;

     struct {
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0cc8b9a..4b6de62 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -885,8 +885,8 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,

                  /* default disk format for mirrored drive */
                 if (disk->mirror &&
-                    disk->mirrorFormat == VIR_STORAGE_FILE_NONE)
-                    disk->mirrorFormat = VIR_STORAGE_FILE_AUTO;
+                    disk->mirror->format == VIR_STORAGE_FILE_NONE)
+                    disk->mirror->format = VIR_STORAGE_FILE_AUTO;
             } else {
                 /* default driver if probing is forbidden */
                 if (!virDomainDiskGetDriver(disk) &&
@@ -901,8 +901,8 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,

                  /* default disk format for mirrored drive */
                 if (disk->mirror &&
-                    disk->mirrorFormat == VIR_STORAGE_FILE_NONE)
-                    disk->mirrorFormat = VIR_STORAGE_FILE_RAW;
+                    disk->mirror->format == VIR_STORAGE_FILE_NONE)
+                    disk->mirror->format = VIR_STORAGE_FILE_RAW;
             }
         }
     }
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0f946d1..4a9dee9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14789,13 +14789,22 @@ qemuDomainBlockPivot(virConnectPtr conn,
     int ret = -1, rc;
     qemuDomainObjPrivatePtr priv = vm->privateData;
     virDomainBlockJobInfo info;
-    const char *format = virStorageFileFormatTypeToString(disk->mirrorFormat);
+    const char *format = NULL;
     bool resume = false;
     char *oldsrc = NULL;
     int oldformat;
     virStorageSourcePtr oldchain = NULL;
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);

+    if (!disk->mirror) {
+        virReportError(VIR_ERR_OPERATION_INVALID,
+                       _("pivot of disk '%s' requires an active copy job"),
+                       disk->dst);
+        goto cleanup;
+    }
+
+    format = virStorageFileFormatTypeToString(disk->mirror->format);
+
     /* Probe the status, if needed.  */
     if (!disk->mirroring) {
         qemuDomainObjEnterMonitor(driver, vm);
@@ -14853,8 +14862,8 @@ qemuDomainBlockPivot(virConnectPtr conn,
     oldsrc = disk->src->path;
     oldformat = disk->src->format;
     oldchain = disk->src->backingStore;
-    disk->src->path = disk->mirror;
-    disk->src->format = disk->mirrorFormat;
+    disk->src->path = disk->mirror->path;
+    disk->src->format = disk->mirror->format;
     disk->src->backingStore = NULL;
     if (qemuDomainDetermineDiskChain(driver, vm, disk, false) < 0) {
         disk->src->path = oldsrc;
@@ -14862,7 +14871,7 @@ qemuDomainBlockPivot(virConnectPtr conn,
         disk->src->backingStore = oldchain;
         goto cleanup;
     }
-    if (disk->mirrorFormat && disk->mirrorFormat != VIR_STORAGE_FILE_RAW &&
+    if (disk->mirror->format && disk->mirror->format != VIR_STORAGE_FILE_RAW &&
         (virDomainLockDiskAttach(driver->lockManager, cfg->uri,
                                  vm, disk) < 0 ||
          qemuSetupDiskCgroup(vm, disk) < 0 ||
@@ -14876,7 +14885,7 @@ qemuDomainBlockPivot(virConnectPtr conn,

     /* Attempt the pivot.  */
     qemuDomainObjEnterMonitor(driver, vm);
-    ret = qemuMonitorDrivePivot(priv->mon, device, disk->mirror, format);
+    ret = qemuMonitorDrivePivot(priv->mon, device, disk->mirror->path, format);
     qemuDomainObjExitMonitor(driver, vm);

     if (ret == 0) {
@@ -14889,7 +14898,7 @@ qemuDomainBlockPivot(virConnectPtr conn,
          * for now, we leak the access to the original.  */
         VIR_FREE(oldsrc);
         virStorageSourceFree(oldchain);
-        disk->mirror = NULL;
+        disk->mirror->path = NULL;
     } else {
         /* On failure, qemu abandons the mirror, and reverts back to
          * the source disk (RHEL 6.3 has a bug where the revert could
@@ -14903,9 +14912,9 @@ qemuDomainBlockPivot(virConnectPtr conn,
         disk->src->format = oldformat;
         virStorageSourceFree(disk->src->backingStore);
         disk->src->backingStore = oldchain;
-        VIR_FREE(disk->mirror);
     }
-    disk->mirrorFormat = VIR_STORAGE_FILE_NONE;
+    virStorageSourceFree(disk->mirror);
+    disk->mirror = NULL;
     disk->mirroring = false;

  cleanup:
@@ -15031,8 +15040,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
     if (mode == BLOCK_JOB_ABORT && disk->mirror) {
         /* XXX We should also revoke security labels and disk lease on
          * the mirror, and audit that fact, before dropping things.  */
-        VIR_FREE(disk->mirror);
-        disk->mirrorFormat = VIR_STORAGE_FILE_NONE;
+        virStorageSourceFree(disk->mirror);
+        disk->mirror = NULL;
         disk->mirroring = false;
     }

@@ -15167,7 +15176,7 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
     int idx;
     struct stat st;
     bool need_unlink = false;
-    char *mirror = NULL;
+    virStorageSourcePtr mirror = NULL;
     virQEMUDriverConfigPtr cfg = NULL;

     /* Preliminaries: find the disk we are editing, sanity checks */
@@ -15248,6 +15257,9 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
         goto endjob;
     }

+    if (VIR_ALLOC(mirror) < 0)
+        goto endjob;
+
     if (!(flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT)) {
         int fd = qemuOpenFile(driver, vm, dest, O_WRONLY | O_TRUNC | O_CREAT,
                               &need_unlink, NULL);
@@ -15255,10 +15267,10 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
             goto endjob;
         VIR_FORCE_CLOSE(fd);
         if (!format)
-            disk->mirrorFormat = disk->src->format;
+            disk->mirror->format = disk->src->format;
     } else if (format) {
-        disk->mirrorFormat = virStorageFileFormatTypeFromString(format);
-        if (disk->mirrorFormat <= 0) {
+        disk->mirror->format = virStorageFileFormatTypeFromString(format);
+        if (disk->mirror->format <= 0) {
             virReportError(VIR_ERR_INVALID_ARG, _("unrecognized format '%s'"),
                            format);
             goto endjob;
@@ -15268,12 +15280,12 @@ 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.  */
-        disk->mirrorFormat = virStorageFileProbeFormat(dest, cfg->user,
-                                                       cfg->group);
+        disk->mirror->format = virStorageFileProbeFormat(dest, cfg->user,
+                                                         cfg->group);
     }
-    if (!format && disk->mirrorFormat > 0)
-        format = virStorageFileFormatTypeToString(disk->mirrorFormat);
-    if (VIR_STRDUP(mirror, dest) < 0)
+    if (!format && disk->mirror->format > 0)
+        format = virStorageFileFormatTypeToString(disk->mirror->format);
+    if (VIR_STRDUP(mirror->path, dest) < 0)
         goto endjob;

     if (qemuDomainPrepareDiskChainElement(driver, vm, disk, dest,
@@ -15303,9 +15315,11 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
  endjob:
     if (need_unlink && unlink(dest))
         VIR_WARN("unable to unlink just-created %s", dest);
-    if (ret < 0 && disk)
-        disk->mirrorFormat = VIR_STORAGE_FILE_NONE;
-    VIR_FREE(mirror);
+    if (ret < 0 && disk) {
+        virStorageSourceFree(disk->mirror);
+        disk->mirror = NULL;
+    }
+    virStorageSourceFree(mirror);
     if (!qemuDomainObjEndJob(driver, vm))
         vm = NULL;

-- 
1.9.0




More information about the libvir-list mailing list