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

31 Mar 2014

On 03/29/14 23:20, Eric Blake wrote:
...
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@redhat.com>
---
 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
...
@@ -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,
Hmm, now that we have a common struct, we can unify the source formatter
and have it accept the common struct instead of splitting out separate
fields ...
...
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);

...
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),
same here. This function can now be refactored to take the struct
instead of the fields separately.
...
-                                    disk->file,
-                                    disk->protocol,
-                                    disk->nhosts,
-                                    disk->hosts,
+                                    disk->src.path,
+                                    disk->src.protocol,
+                                    disk->src.nhosts,
+                                    disk->src.hosts,
                                     NULL,
                                     NULL,
                                     source);

...
@@ -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;
And here we can introduce a function to copy the source instead of doing
it manually.
...
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;
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);
 }
ACK to the patch. The proposed changes are definitely separable and
better to review separately.

Peter

    

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

Peter Krempa