[PATCH 07/17] virDomainSnapshotAlignDisks: Move 'require_match' selection logic inside
Peter Krempa
pkrempa at redhat.com
Wed Jan 12 18:10:07 UTC 2022
'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 at redhat.com>
---
src/conf/snapshot_conf.c | 24 ++++++++++++++++--------
src/conf/snapshot_conf.h | 2 +-
src/qemu/qemu_snapshot.c | 5 +----
src/test/test_driver.c | 5 +----
4 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index f0ded7919c..43f984912e 100644
--- 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 e5d4d0fc45..505c9f785d 100644
--- 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 51df0ed8df..48fa19cebd 100644
--- 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 2ff56cf03f..44f06530b5 100644
--- 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
--
2.31.1
More information about the libvir-list
mailing list