conf: store mirroring information in virStorageSource

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@redhat.com>

src/conf/domain_conf.c

@@ -1203,7 +1203,7 @@ virDomainDiskDefFree(virDomainDiskDefPtr def)
     virStorageSourceFree(def->src);
     VIR_FREE(def->serial);
     VIR_FREE(def->dst);
-    VIR_FREE(def->mirror);
+    virStorageSourceFree(def->mirror);
     VIR_FREE(def->wwn);
     VIR_FREE(def->vendor);
     VIR_FREE(def->product);
@@ -5223,9 +5223,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;
@@ -5374,19 +5371,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");
+                char *mirrorFormat;
-                if (!mirror) {
+
+                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")) {
@@ -5906,9 +5921,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;
@@ -5930,16 +5942,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;
@@ -5964,8 +5966,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);
@@ -15160,10 +15160,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);
+        virBufferEscapeString(buf, "<mirror file='%s'", def->mirror->path);
-        if (def->mirrorFormat)
+        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");

src/conf/domain_conf.h

@@ -606,8 +606,7 @@ struct _virDomainDiskDef {
     int tray_status; /* enum virDomainDiskTray */
     int removable; /* enum virDomainFeatureState */
 
-    char *mirror;
+    virStorageSourcePtr mirror;
-    int mirrorFormat; /* virStorageFileFormat */
     bool mirroring;
 
     struct {

src/qemu/qemu_domain.c

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

src/qemu/qemu_driver.c

@@ -14864,13 +14864,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);
@@ -14928,8 +14937,8 @@ qemuDomainBlockPivot(virConnectPtr conn,
     oldsrc = disk->src->path;
     oldformat = disk->src->format;
     oldchain = disk->src->backingStore;
-    disk->src->path = disk->mirror;
+    disk->src->path = disk->mirror->path;
-    disk->src->format = disk->mirrorFormat;
+    disk->src->format = disk->mirror->format;
     disk->src->backingStore = NULL;
     if (qemuDomainDetermineDiskChain(driver, vm, disk, false) < 0) {
         disk->src->path = oldsrc;
@@ -14937,7 +14946,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 ||
@@ -14951,7 +14960,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) {
@@ -14964,7 +14973,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
@@ -14978,9 +14987,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:
@@ -15106,8 +15115,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);
+        virStorageSourceFree(disk->mirror);
-        disk->mirrorFormat = VIR_STORAGE_FILE_NONE;
+        disk->mirror = NULL;
         disk->mirroring = false;
     }
 
@@ -15242,7 +15251,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 */
@@ -15323,6 +15332,11 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
         goto endjob;
     }
 
+    if (VIR_ALLOC(mirror) < 0)
+        goto endjob;
+    /* XXX Allow non-file mirror destinations */
+    mirror->type = VIR_STORAGE_TYPE_FILE;
+
     if (!(flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT)) {
         int fd = qemuOpenFile(driver, vm, dest, O_WRONLY | O_TRUNC | O_CREAT,
                               &need_unlink, NULL);
@@ -15330,10 +15344,10 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
             goto endjob;
         VIR_FORCE_CLOSE(fd);
         if (!format)
-            disk->mirrorFormat = disk->src->format;
+            mirror->format = disk->src->format;
     } else if (format) {
-        disk->mirrorFormat = virStorageFileFormatTypeFromString(format);
+        mirror->format = virStorageFileFormatTypeFromString(format);
-        if (disk->mirrorFormat <= 0) {
+        if (mirror->format <= 0) {
             virReportError(VIR_ERR_INVALID_ARG, _("unrecognized format '%s'"),
                            format);
             goto endjob;
@@ -15343,12 +15357,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,
+        mirror->format = virStorageFileProbeFormat(dest, cfg->user,
-                                                       cfg->group);
+                                                   cfg->group);
     }
-    if (!format && disk->mirrorFormat > 0)
+    if (!format && mirror->format > 0)
-        format = virStorageFileFormatTypeToString(disk->mirrorFormat);
+        format = virStorageFileFormatTypeToString(mirror->format);
-    if (VIR_STRDUP(mirror, dest) < 0)
+    if (VIR_STRDUP(mirror->path, dest) < 0)
         goto endjob;
 
     if (qemuDomainPrepareDiskChainElementPath(driver, vm, disk, dest,
@@ -15378,9 +15392,11 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
  endjob:
     if (need_unlink && unlink(dest))
         VIR_WARN("unable to unlink just-created %s", dest);
-    if (ret < 0 && disk)
+    if (ret < 0 && disk) {
-        disk->mirrorFormat = VIR_STORAGE_FILE_NONE;
+        virStorageSourceFree(disk->mirror);
-    VIR_FREE(mirror);
+        disk->mirror = NULL;
+    }
+    virStorageSourceFree(mirror);
     if (!qemuDomainObjEndJob(driver, vm))
         vm = NULL;
 

src/qemu/qemu_process.c

@@ -1033,12 +1033,15 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
              type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT) &&
             status == VIR_DOMAIN_BLOCK_JOB_COMPLETED)
             qemuDomainDetermineDiskChain(driver, vm, disk, true);
-        if (disk->mirror && type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY &&
+        if (disk->mirror && type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) {
-            status == VIR_DOMAIN_BLOCK_JOB_READY)
+            if (status == VIR_DOMAIN_BLOCK_JOB_READY) {
-            disk->mirroring = true;
+                disk->mirroring = true;
-        if (disk->mirror && type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY &&
+            } else if (status == VIR_DOMAIN_BLOCK_JOB_FAILED) {
-            status == VIR_DOMAIN_BLOCK_JOB_FAILED)
+                virStorageSourceFree(disk->mirror);
-            VIR_FREE(disk->mirror);
+                disk->mirror = NULL;
+                disk->mirroring = false;
+            }
+        }
     }
 
     virObjectUnlock(vm);