[libvirt] [PATCH 12/n] conf: let snapshots share disk source struct

Saturday, 29 March 2014

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: Eric Blake <eblake(a)redhat.com&gt;
---
 src/conf/snapshot_conf.c     | 68 +++++++++++++++++------------------
 src/conf/snapshot_conf.h     |  8 ++---
 src/qemu/qemu_conf.c         |  2 +-
 src/qemu/qemu_driver.c       | 86 ++++++++++++++++++++++----------------------
 src/storage/storage_driver.c |  8 ++---
 5 files changed, 85 insertions(+), 87 deletions(-)

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index ebb3bb4..3bd4732 100644
--- a/src/conf/snapshot_conf.c
+++ b/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;
 }
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index f6fec88..1444d77 100644
--- a/src/conf/snapshot_conf.h
+++ b/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 */
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 209558d..198ee2f 100644
--- a/src/qemu/qemu_conf.c
+++ b/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",
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9d48a46..9a2de12 100644
--- a/src/qemu/qemu_driver.c
+++ b/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;
 }

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index cdb8536..6fca3d2 100644
--- a/src/storage/storage_driver.c
+++ b/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);
 }


-- 
1.9.0


    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

[libvirt] [PATCH 12/n] conf: let snapshots share disk source struct