[PATCH 08/17] virDomainSnapshotAlignDisks: Allow alternate domain definition when redefining

[Peter Krempa <pkrempa@xxxxxxxxxx>]

Due to historical reasons we allow users to redefine an existing
snapshot without providing the domain definition which would correspond
to it. In such case we'd use the domain definition from the snapshot
that is being redefined.

To prevent callers from doing complex moving of the domain definition
object back and forth between the snapshot definitions we can add an
argument to virDomainSnapshotAlignDisks which will allow us to pass in
the alternate definition if the one from the snapshot is missing.

Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
---
 src/conf/snapshot_conf.c | 14 ++++++++++++--
 src/conf/snapshot_conf.h |  1 +
 src/qemu/qemu_snapshot.c |  2 +-
 src/test/test_driver.c   |  2 +-
 4 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 43f984912e..be5deadc8c 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -535,7 +535,7 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDef *def,
         if (external)
             align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;

-        if (virDomainSnapshotAlignDisks(def, align_location, true) < 0)
+        if (virDomainSnapshotAlignDisks(def, NULL, align_location, true) < 0)
             return -1;
     }

@@ -622,16 +622,22 @@ virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDef *def)
 /**
  * virDomainSnapshotAlignDisks:
  * @snapdef: Snapshot definition to align
+ * @existingDomainDef: definition of the domain belonging to a redefined snapshot
  * @default_snapshot: snapshot location to assign to disks which don't have any
  * @uniform_internal_snapshot: Require that for an internal snapshot all disks
  *                             take part in the internal snapshot
  *
- * Align snapdef->disks to snapdef->parent.dom, filling in any missing disks or
+ * Align snapdef->disks to domain definition, filling in any missing disks or
  * snapshot state defaults given by the domain, with a fallback to
  * @default_snapshot. Ensure that there are no duplicate snapshot disk
  * definitions in @snapdef and there are no disks described in @snapdef but
  * missing from the domain definition.
  *
+ * By default the domain definition from @snapdef->parent.dom is used, but when
+ * redefining an existing snapshot the domain definition may be omitted in
+ * @snapdef. In such case callers must pass in the definition from the snapsot
+ * being redefined as @existingDomainDef. In all other cases callers pass NULL.
+ *
  * When @uniform_internal_snapshot is true and @default_snapshot is
  * VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL, all disks in @snapdef must take part
  * in the internal snapshot. This is for hypervisors where granularity of an
@@ -643,6 +649,7 @@ virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDef *def)
  */
 int
 virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapdef,
+                            virDomainDef *existingDomainDef,
                             virDomainSnapshotLocation default_snapshot,
                             bool uniform_internal_snapshot)
 {
@@ -653,6 +660,9 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapdef,
     size_t oldndisks;
     size_t i;

+    if (!domdef)
+        domdef = existingDomainDef;
+
     if (!domdef) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("missing domain in snapshot"));
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 505c9f785d..446d0870f4 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -126,6 +126,7 @@ char *virDomainSnapshotDefFormat(const char *uuidstr,
                                  virDomainXMLOption *xmlopt,
                                  unsigned int flags);
 int virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapshot,
+                                virDomainDef *existingDomainDef,
                                 virDomainSnapshotLocation default_snapshot,
                                 bool uniform_internal_snapshot);

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 48fa19cebd..797972f4d4 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1675,7 +1675,7 @@ qemuSnapshotCreateAlignDisks(virDomainObj *vm,
                        VIR_DOMAIN_SNAPSHOT_LOCATION_NONE :
                        VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL);
     }
-    if (virDomainSnapshotAlignDisks(def, align_location, true) < 0)
+    if (virDomainSnapshotAlignDisks(def, NULL, align_location, true) < 0)
         return -1;

     return 0;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 44f06530b5..dde7bf1b8e 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -8736,7 +8736,7 @@ testDomainSnapshotAlignDisks(virDomainObj *vm,
                       VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
     }

-    return virDomainSnapshotAlignDisks(def, align_location, true);
+    return virDomainSnapshotAlignDisks(def, NULL, align_location, true);
 }

 static virDomainSnapshotPtr
-- 
2.31.1