conf: let snapshots share disk source struct

Now that we have a common struct, it's time to start using it!
Since external snapshots make a longer backing chain, it is
only natural to use the same struct for the file created by
the snapshot as what we use for <domain> disks.

* src/conf/snapshot_conf.h (_virDomainSnapshotDiskDef): Use common
struct instead of open-coded duplicate fields.
* src/conf/snapshot_conf.c (virDomainSnapshotDiskDefClear)
(virDomainSnapshotDiskDefParseXML, virDomainSnapshotAlignDisks)
(virDomainSnapshotDiskDefFormat)
(virDomainSnapshotDiskGetActualType): Adjust clients.
* src/qemu/qemu_conf.c (qemuTranslateSnapshotDiskSourcePool):
Likewise.
* src/qemu/qemu_driver.c (qemuDomainSnapshotDiskGetSourceString)
(qemuDomainSnapshotCreateInactiveExternal)
(qemuDomainSnapshotPrepareDiskExternalOverlayActive)
(qemuDomainSnapshotPrepareDiskExternal)
(qemuDomainSnapshotPrepare)
(qemuDomainSnapshotCreateSingleDiskActive): Likewise.
* src/storage/storage_driver.c
(virStorageFileInitFromSnapshotDef): Likewise.
Signed-off-by: NEric Blake <eblake@redhat.com>

src/conf/snapshot_conf.c

@@ -83,9 +83,7 @@ static void
 virDomainSnapshotDiskDefClear(virDomainSnapshotDiskDefPtr disk)
 {
     VIR_FREE(disk->name);
-    VIR_FREE(disk->file);
-    virStorageNetHostDefFree(disk->nhosts, disk->hosts);
-    disk->nhosts = 0;
+    virStorageSourceClear(&disk->src);
 }
 
 void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def)
@@ -134,39 +132,39 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
     }
 
     if ((type = virXMLPropString(node, "type"))) {
-        if ((def->type = virStorageTypeFromString(type)) < 0 ||
-            def->type == VIR_STORAGE_TYPE_VOLUME ||
-            def->type == VIR_STORAGE_TYPE_DIR) {
+        if ((def->src.type = virStorageTypeFromString(type)) < 0 ||
+            def->src.type == VIR_STORAGE_TYPE_VOLUME ||
+            def->src.type == VIR_STORAGE_TYPE_DIR) {
             virReportError(VIR_ERR_XML_ERROR,
                            _("unknown disk snapshot type '%s'"), type);
             goto cleanup;
         }
     } else {
-        def->type = VIR_STORAGE_TYPE_FILE;
+        def->src.type = VIR_STORAGE_TYPE_FILE;
     }
 
     for (cur = node->children; cur; cur = cur->next) {
         if (cur->type != XML_ELEMENT_NODE)
             continue;
 
-        if (!def->file &&
+        if (!def->src.path &&
             xmlStrEqual(cur->name, BAD_CAST "source")) {
 
             if (virDomainDiskSourceDefParse(cur,
-                                            def->type,
-                                            &def->file,
-                                            &def->protocol,
-                                            &def->nhosts,
-                                            &def->hosts,
+                                            def->src.type,
+                                            &def->src.path,
+                                            &def->src.protocol,
+                                            &def->src.nhosts,
+                                            &def->src.hosts,
                                             NULL) < 0)
                 goto cleanup;
 
-        } else if (!def->format &&
+        } else if (!def->src.format &&
                    xmlStrEqual(cur->name, BAD_CAST "driver")) {
             char *driver = virXMLPropString(cur, "type");
             if (driver) {
-                def->format = virStorageFileFormatTypeFromString(driver);
-                if (def->format <= 0) {
+                def->src.format = virStorageFileFormatTypeFromString(driver);
+                if (def->src.format <= 0) {
                     virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                                    _("unknown disk snapshot driver '%s'"),
                                    driver);
@@ -178,7 +176,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
         }
     }
 
-    if (!def->snapshot && (def->file || def->format))
+    if (!def->snapshot && (def->src.path || def->src.format))
         def->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
 
     ret = 0;
@@ -511,12 +509,12 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
                            disk->name, tmp);
             goto cleanup;
         }
-        if (disk->file &&
+        if (disk->src.path &&
             disk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            _("file '%s' for disk '%s' requires "
                              "use of external snapshot mode"),
-                           disk->file, disk->name);
+                           disk->src.path, disk->name);
             goto cleanup;
         }
         if (STRNEQ(disk->name, def->dom->disks[idx]->dst)) {
@@ -543,7 +541,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
             goto cleanup;
         disk->index = i;
         disk->snapshot = def->dom->disks[i]->snapshot;
-        disk->type = VIR_STORAGE_TYPE_FILE;
+        disk->src.type = VIR_STORAGE_TYPE_FILE;
         if (!disk->snapshot)
             disk->snapshot = default_snapshot;
     }
@@ -556,16 +554,17 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
         virDomainSnapshotDiskDefPtr disk = &def->disks[i];
 
         if (disk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL &&
-            !disk->file) {
+            !disk->src.path) {
             const char *original = virDomainDiskGetSource(def->dom->disks[i]);
             const char *tmp;
             struct stat sb;
 
-            if (disk->type != VIR_STORAGE_TYPE_FILE) {
+            if (disk->src.type != VIR_STORAGE_TYPE_FILE) {
                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                                _("cannot generate external snapshot name "
                                  "for disk '%s' on a '%s' device"),
-                               disk->name, virStorageTypeToString(disk->type));
+                               disk->name,
+                               virStorageTypeToString(disk->src.type));
                 goto cleanup;
             }
 
@@ -587,7 +586,8 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
 
             tmp = strrchr(original, '.');
             if (!tmp || strchr(tmp, '/')) {
-                if (virAsprintf(&disk->file, "%s.%s", original, def->name) < 0)
+                if (virAsprintf(&disk->src.path, "%s.%s", original,
+                                def->name) < 0)
                     goto cleanup;
             } else {
                 if ((tmp - original) > INT_MAX) {
@@ -595,7 +595,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
                                    _("integer overflow"));
                     goto cleanup;
                 }
-                if (virAsprintf(&disk->file, "%.*s.%s",
+                if (virAsprintf(&disk->src.path, "%.*s.%s",
                                 (int) (tmp - original), original,
                                 def->name) < 0)
                     goto cleanup;
@@ -614,7 +614,7 @@ static void
 virDomainSnapshotDiskDefFormat(virBufferPtr buf,
                                virDomainSnapshotDiskDefPtr disk)
 {
-    int type = disk->type;
+    int type = disk->src.type;
 
     if (!disk->name)
         return;
@@ -624,7 +624,7 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf,
         virBufferAsprintf(buf, " snapshot='%s'",
                           virDomainSnapshotLocationTypeToString(disk->snapshot));
 
-    if (!disk->file && disk->format == 0) {
+    if (!disk->src.path && disk->src.format == 0) {
         virBufferAddLit(buf, "/>\n");
         return;
     }
@@ -632,16 +632,16 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf,
     virBufferAsprintf(buf, " type='%s'>\n", virStorageTypeToString(type));
     virBufferAdjustIndent(buf, 2);
 
-    if (disk->format > 0)
+    if (disk->src.format > 0)
         virBufferEscapeString(buf, "<driver type='%s'/>\n",
-                              virStorageFileFormatTypeToString(disk->format));
+                              virStorageFileFormatTypeToString(disk->src.format));
     virDomainDiskSourceDefFormatInternal(buf,
                                          type,
-                                         disk->file,
+                                         disk->src.path,
                                          0,
-                                         disk->protocol,
-                                         disk->nhosts,
-                                         disk->hosts,
+                                         disk->src.protocol,
+                                         disk->src.nhosts,
+                                         disk->src.hosts,
                                          0, NULL, NULL, 0);
 
     virBufferAdjustIndent(buf, -2);
@@ -1308,5 +1308,5 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain,
 int
 virDomainSnapshotDiskGetActualType(virDomainSnapshotDiskDefPtr def)
 {
-    return def->type;
+    return def->src.type;
 }

src/conf/snapshot_conf.h

@@ -51,12 +51,8 @@ struct _virDomainSnapshotDiskDef {
     char *name;     /* name matching the <target dev='...' of the domain */
     int index;      /* index within snapshot->dom->disks that matches name */
     int snapshot;   /* enum virDomainSnapshotLocation */
-    int type;       /* enum virStorageType */
-    char *file;     /* new source file when snapshot is external */
-    int format;     /* enum virStorageFileFormat */
-    int protocol;   /* network source protocol */
-    size_t nhosts;  /* network source hosts count */
-    virStorageNetHostDefPtr hosts; /* network source hosts */
+
+    virStorageSource src; /* new wrapper file when snapshot is external */
 };
 
 /* Stores the complete snapshot metadata */

src/qemu/qemu_conf.c

@@ -1422,7 +1422,7 @@ int
 qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn ATTRIBUTE_UNUSED,
                                     virDomainSnapshotDiskDefPtr def)
 {
-    if (def->type != VIR_STORAGE_TYPE_VOLUME)
+    if (def->src.type != VIR_STORAGE_TYPE_VOLUME)
         return 0;
 
     virReportError(VIR_ERR_INTERNAL_ERROR, "%s",

src/qemu/qemu_driver.c

@@ -12028,10 +12028,10 @@ qemuDomainSnapshotDiskGetSourceString(virDomainSnapshotDiskDefPtr disk,
     *source = NULL;
 
     return qemuGetDriveSourceString(virDomainSnapshotDiskGetActualType(disk),
-                                    disk->file,
-                                    disk->protocol,
-                                    disk->nhosts,
-                                    disk->hosts,
+                                    disk->src.path,
+                                    disk->src.protocol,
+                                    disk->src.nhosts,
+                                    disk->src.hosts,
                                     NULL,
                                     NULL,
                                     source);
@@ -12178,14 +12178,14 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver,
         if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
             continue;
 
-        if (!snapdisk->format)
-            snapdisk->format = VIR_STORAGE_FILE_QCOW2;
+        if (!snapdisk->src.format)
+            snapdisk->src.format = VIR_STORAGE_FILE_QCOW2;
 
         /* creates cmd line args: qemu-img create -f qcow2 -o */
         if (!(cmd = virCommandNewArgList(qemuImgPath,
                                          "create",
                                          "-f",
-                                         virStorageFileFormatTypeToString(snapdisk->format),
+                                         virStorageFileFormatTypeToString(snapdisk->src.format),
                                          "-o",
                                          NULL)))
             goto cleanup;
@@ -12209,10 +12209,10 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver,
         }
 
         /* adds cmd line args: /path/to/target/file */
-        virCommandAddArg(cmd, snapdisk->file);
+        virCommandAddArg(cmd, snapdisk->src.path);
 
         /* If the target does not exist, we're going to create it possibly */
-        if (!virFileExists(snapdisk->file))
+        if (!virFileExists(snapdisk->src.path))
             ignore_value(virBitmapSetBit(created, i));
 
         if (virCommandRun(cmd, NULL) < 0)
@@ -12229,11 +12229,11 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver,
 
         if (snapdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
             VIR_FREE(defdisk->src.path);
-            if (VIR_STRDUP(defdisk->src.path, snapdisk->file) < 0) {
+            if (VIR_STRDUP(defdisk->src.path, snapdisk->src.path) < 0) {
                 /* we cannot rollback here in a sane way */
                 goto cleanup;
             }
-            defdisk->src.format = snapdisk->format;
+            defdisk->src.format = snapdisk->src.format;
         }
     }
 
@@ -12247,9 +12247,9 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver,
         ssize_t bit = -1;
         while ((bit = virBitmapNextSetBit(created, bit)) >= 0) {
             snapdisk = &(snap->def->disks[bit]);
-            if (unlink(snapdisk->file) < 0)
+            if (unlink(snapdisk->src.path) < 0)
                 VIR_WARN("Failed to remove snapshot image '%s'",
-                         snapdisk->file);
+                         snapdisk->src.path);
         }
     }
     virBitmapFree(created);
@@ -12417,7 +12417,7 @@ qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d
         return 0;
 
     case VIR_STORAGE_TYPE_NETWORK:
-        switch ((enum virStorageNetProtocol) disk->protocol) {
+        switch ((enum virStorageNetProtocol) disk->src.protocol) {
         case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
             return 0;
 
@@ -12434,7 +12434,7 @@ qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("external active snapshots are not supported on "
                              "'network' disks using '%s' protocol"),
-                           virStorageNetProtocolTypeToString(disk->protocol));
+                           virStorageNetProtocolTypeToString(disk->src.protocol));
             return -1;
 
         }
@@ -12515,19 +12515,19 @@ qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn,
         if (errno != ENOENT) {
             virReportSystemError(errno,
                                  _("unable to stat for disk %s: %s"),
-                                 snapdisk->name, snapdisk->file);
+                                 snapdisk->name, snapdisk->src.path);
             goto cleanup;
         } else if (reuse) {
             virReportSystemError(errno,
                                  _("missing existing file for disk %s: %s"),
-                                 snapdisk->name, snapdisk->file);
+                                 snapdisk->name, snapdisk->src.path);
             goto cleanup;
         }
     } else if (!S_ISBLK(st.st_mode) && st.st_size && !reuse) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("external snapshot file for disk %s already "
                          "exists and is not a block device: %s"),
-                       snapdisk->name, snapdisk->file);
+                       snapdisk->name, snapdisk->src.path);
         goto cleanup;
     }
 
@@ -12654,15 +12654,15 @@ qemuDomainSnapshotPrepare(virConnectPtr conn,
             break;
 
         case VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL:
-            if (!disk->format) {
-                disk->format = VIR_STORAGE_FILE_QCOW2;
-            } else if (disk->format != VIR_STORAGE_FILE_QCOW2 &&
-                       disk->format != VIR_STORAGE_FILE_QED) {
+            if (!disk->src.format) {
+                disk->src.format = VIR_STORAGE_FILE_QCOW2;
+            } else if (disk->src.format != VIR_STORAGE_FILE_QCOW2 &&
+                       disk->src.format != VIR_STORAGE_FILE_QED) {
                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                                _("external snapshot format for disk %s "
                                  "is unsupported: %s"),
                                disk->name,
-                               virStorageFileFormatTypeToString(disk->format));
+                               virStorageFileFormatTypeToString(disk->src.format));
                 goto cleanup;
             }
 
@@ -12750,7 +12750,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
     char *newsource = NULL;
     virStorageNetHostDefPtr newhosts = NULL;
     virStorageNetHostDefPtr persistHosts = NULL;
-    int format = snap->format;
+    int format = snap->src.format;
     const char *formatStr = NULL;
     char *persistSource = NULL;
     int ret = -1;
@@ -12781,14 +12781,14 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
     if (qemuDomainSnapshotDiskGetSourceString(snap, &source) < 0)
         goto cleanup;
 
-    if (VIR_STRDUP(newsource, snap->file) < 0)
+    if (VIR_STRDUP(newsource, snap->src.path) < 0)
         goto cleanup;
 
     if (persistDisk &&
-        VIR_STRDUP(persistSource, snap->file) < 0)
+        VIR_STRDUP(persistSource, snap->src.path) < 0)
         goto cleanup;
 
-    switch (snap->type) {
+    switch (snap->src.type) {
     case VIR_STORAGE_TYPE_BLOCK:
         reuse = true;
         /* fallthrough */
@@ -12813,13 +12813,15 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
         break;
 
     case VIR_STORAGE_TYPE_NETWORK:
-        switch (snap->protocol) {
+        switch (snap->src.protocol) {
         case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
-            if (!(newhosts = virStorageNetHostDefCopy(snap->nhosts, snap->hosts)))
+            if (!(newhosts = virStorageNetHostDefCopy(snap->src.nhosts,
+                                                      snap->src.hosts)))
                 goto cleanup;
 
             if (persistDisk &&
-                !(persistHosts = virStorageNetHostDefCopy(snap->nhosts, snap->hosts)))
+                !(persistHosts = virStorageNetHostDefCopy(snap->src.nhosts,
+                                                          snap->src.hosts)))
                 goto cleanup;
 
             break;
@@ -12828,7 +12830,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
             virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
                            _("snapshots on volumes using '%s' protocol "
                              "are not supported"),
-                           virStorageNetProtocolTypeToString(snap->protocol));
+                           virStorageNetProtocolTypeToString(snap->src.protocol));
             goto cleanup;
         }
         break;
@@ -12836,13 +12838,13 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
     default:
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
                        _("snapshots are not supported on '%s' volumes"),
-                       virStorageTypeToString(snap->type));
+                       virStorageTypeToString(snap->src.type));
         goto cleanup;
     }
 
     /* create the actual snapshot */
-    if (snap->format)
-        formatStr = virStorageFileFormatTypeToString(snap->format);
+    if (snap->src.format)
+        formatStr = virStorageFileFormatTypeToString(snap->src.format);
 
     /* The monitor is only accessed if qemu doesn't support transactions.
      * Otherwise the following monitor command only constructs the command.
@@ -12874,9 +12876,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
 
     disk->src.path = newsource;
     disk->src.format = format;
-    disk->src.type = snap->type;
-    disk->src.protocol = snap->protocol;
-    disk->src.nhosts = snap->nhosts;
+    disk->src.type = snap->src.type;
+    disk->src.protocol = snap->src.protocol;
+    disk->src.nhosts = snap->src.nhosts;
     disk->src.hosts = newhosts;
 
     newsource = NULL;
@@ -12889,9 +12891,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
 
         persistDisk->src.path = persistSource;
         persistDisk->src.format = format;
-        persistDisk->src.type = snap->type;
-        persistDisk->src.protocol = snap->protocol;
-        persistDisk->src.nhosts = snap->nhosts;
+        persistDisk->src.type = snap->src.type;
+        persistDisk->src.protocol = snap->src.protocol;
+        persistDisk->src.nhosts = snap->src.nhosts;
         persistDisk->src.hosts = persistHosts;
 
         persistSource = NULL;
@@ -12906,8 +12908,8 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
     VIR_FREE(source);
     VIR_FREE(newsource);
     VIR_FREE(persistSource);
-    virStorageNetHostDefFree(snap->nhosts, newhosts);
-    virStorageNetHostDefFree(snap->nhosts, persistHosts);
+    virStorageNetHostDefFree(snap->src.nhosts, newhosts);
+    virStorageNetHostDefFree(snap->src.nhosts, persistHosts);
     return ret;
 }
 

src/storage/storage_driver.c

@@ -2802,10 +2802,10 @@ virStorageFilePtr
 virStorageFileInitFromSnapshotDef(virDomainSnapshotDiskDefPtr disk)
 {
     return virStorageFileInitInternal(virDomainSnapshotDiskGetActualType(disk),
-                                      disk->file,
-                                      disk->protocol,
-                                      disk->nhosts,
-                                      disk->hosts);
+                                      disk->src.path,
+                                      disk->src.protocol,
+                                      disk->src.nhosts,
+                                      disk->src.hosts);
 }