virDomainSnapshotAlignDisks: Move 'require_match' selection logic inside

'require_match' set to true is only needed for internal snapshots taken
by hypervisors (qemu) which don't have a way to control which disks take
part in the snapshot (savevm).

To de-clutter callers we can change the argument to mean 'this code path
requires uniform snapshot for internal snapshots'.

Change the argument and fix the callers. For now all callers pass 'true'
but any new hypervisor or even usage in qemu is not going to share the
limitation.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index f0ded7919c00897983a0064a042f464271fbabe2..43f984912e95849e21825e565d709dd7ff3790f7 100644 (file)
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -474,7 +474,6 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDef *def,
                                   unsigned int flags)
 {
     virDomainSnapshotLocation align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
-    bool align_match = true;
     bool external = def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT ||
         virDomainSnapshotDefIsExternal(def);
 
@@ -533,12 +532,10 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDef *def,
     }
 
     if (def->parent.dom) {
-        if (external) {
+        if (external)
             align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
-            align_match = false;
-        }
-        if (virDomainSnapshotAlignDisks(def, align_location,
-                                        align_match) < 0)
+
+        if (virDomainSnapshotAlignDisks(def, align_location, true) < 0)
             return -1;
     }
 
@@ -626,7 +623,8 @@ virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDef *def)
  * virDomainSnapshotAlignDisks:
  * @snapdef: Snapshot definition to align
  * @default_snapshot: snapshot location to assign to disks which don't have any
- * @require_match: Require that all disks use the same snapshot mode
+ * @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
  * snapshot state defaults given by the domain, with a fallback to
@@ -634,6 +632,11 @@ virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDef *def)
  * definitions in @snapdef and there are no disks described in @snapdef but
  * missing from the domain definition.
  *
+ * 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
+ * internal snapshot can't be controlled.
+ *
  * Convert paths to disk targets for uniformity.
  *
  * On error -1 is returned and a libvirt error is reported.
@@ -641,11 +644,12 @@ virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDef *def)
 int
 virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapdef,
                             virDomainSnapshotLocation default_snapshot,
-                            bool require_match)
+                            bool uniform_internal_snapshot)
 {
     virDomainDef *domdef = snapdef->parent.dom;
     g_autoptr(GHashTable) map = virHashNew(NULL);
     g_autofree virDomainSnapshotDiskDef *olddisks = NULL;
+    bool require_match = false;
     size_t oldndisks;
     size_t i;
 
@@ -661,6 +665,10 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapdef,
         return -1;
     }
 
+    if (uniform_internal_snapshot &&
+        default_snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL)
+        require_match = true;
+
     /* Unlikely to have a guest without disks but technically possible.  */
     if (!domdef->ndisks)
         return 0;

diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index e5d4d0fc4592ac3f224da7b486ef9a79c71ab8e3..505c9f785de6eaaf6192c97746d7766f9a436375 100644 (file)
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -127,7 +127,7 @@ char *virDomainSnapshotDefFormat(const char *uuidstr,
                                  unsigned int flags);
 int virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapshot,
                                 virDomainSnapshotLocation default_snapshot,
-                                bool require_match);
+                                bool uniform_internal_snapshot);
 
 bool virDomainSnapshotDefIsExternal(virDomainSnapshotDef *def);
 bool virDomainSnapshotIsExternal(virDomainMomentObj *snap);

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 51df0ed8dfb248784f4da95b474946ab8a275e5b..48fa19cebd2ef5b414e68d8142269a80c43c8530 100644 (file)
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1631,7 +1631,6 @@ qemuSnapshotCreateAlignDisks(virDomainObj *vm,
     g_autofree char *xml = NULL;
     qemuDomainObjPrivate *priv = vm->privateData;
     virDomainSnapshotLocation align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
-    bool align_match = true;
 
     /* Easiest way to clone inactive portion of vm->def is via
      * conversion in and back out of xml.  */
@@ -1653,7 +1652,6 @@ qemuSnapshotCreateAlignDisks(virDomainObj *vm,
 
     if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) {
         align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
-        align_match = false;
         if (virDomainObjIsActive(vm))
             def->state = VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT;
         else
@@ -1662,7 +1660,6 @@ qemuSnapshotCreateAlignDisks(virDomainObj *vm,
     } else if (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
         def->state = virDomainObjGetState(vm, NULL);
         align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
-        align_match = false;
     } else {
         def->state = virDomainObjGetState(vm, NULL);
 
@@ -1678,7 +1675,7 @@ qemuSnapshotCreateAlignDisks(virDomainObj *vm,
                        VIR_DOMAIN_SNAPSHOT_LOCATION_NONE :
                        VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL);
     }
-    if (virDomainSnapshotAlignDisks(def, align_location, align_match) < 0)
+    if (virDomainSnapshotAlignDisks(def, align_location, true) < 0)
         return -1;
 
     return 0;

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 2ff56cf03f77ed7d2a6f5a947e775f159b0905d9..44f06530b5345117d85972855b13546d1e0a452d 100644 (file)
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -8718,11 +8718,9 @@ testDomainSnapshotAlignDisks(virDomainObj *vm,
                              unsigned int flags)
 {
     virDomainSnapshotLocation align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
-    bool align_match = true;
 
     if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) {
         align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
-        align_match = false;
         if (virDomainObjIsActive(vm))
             def->state = VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT;
         else
@@ -8731,7 +8729,6 @@ testDomainSnapshotAlignDisks(virDomainObj *vm,
     } else if (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
         def->state = virDomainObjGetState(vm, NULL);
         align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
-        align_match = false;
     } else {
         def->state = virDomainObjGetState(vm, NULL);
         def->memory = def->state == VIR_DOMAIN_SNAPSHOT_SHUTOFF ?
@@ -8739,7 +8736,7 @@ testDomainSnapshotAlignDisks(virDomainObj *vm,
                       VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
     }
 
-    return virDomainSnapshotAlignDisks(def, align_location, align_match);
+    return virDomainSnapshotAlignDisks(def, align_location, true);
 }
 
 static virDomainSnapshotPtr