5:55 p.m.
On 08/22/2011 02:41 PM, Eric Blake wrote:
My RFC for snapshot support [1] proposes several rules for when it
is
safe to delete or revert to an external snapshot, predicated on
the existence of new API flags. These will be incrementally added
in future patches, but until then, blindly mishandling a disk
snapshot risks corrupting internal state, so it is better to
outright reject the attempts until the other pieces are in place,
thus incrementally relaxing the restrictions added in this patch.
[1] https://www.redhat.com/archives/libvir-list/2011-August/msg00361.html
* src/qemu/qemu_driver.c (qemuDomainSnapshotCountExternal): New
function.
(qemuDomainSnapshotDelete): Use it to add safety valve.
(qemuDomainRevertToSnapshot): Add safety valve.
@@ -9293,6 +9314,16 @@ static int
qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
goto cleanup;
}
+ if (snap->def->state == VIR_DOMAIN_DISK_SNAPSHOT ||
+ (virDomainSnapshotForEachDescendant(&vm->snapshots, snap,
+ qemuDomainSnapshotCountExternal,
+&external)&& external)) {
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("deletion of external disk snapshots not supported
"
+ "yet"));
+ goto cleanup;
+ }
This was a little too strict - I couldn't even get rid of my metadata,
which should always be a safe operation. Conversely, undefine and
destroy with the options to delete *_SNAPSHOT_FULL should also be
complaining. Squash this in:
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index bf3c137..9f3102b 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -1777,6 +1777,19 @@ qemuDomainSnapshotDiscardAll(void *payload,
curr->err = err;
}
+/* Count how many snapshots in a set have external disk snapshots. */
+static void
+qemuDomainSnapshotCountExternal(void *payload,
+ const void *name ATTRIBUTE_UNUSED,
+ void *data)
+{
+ virDomainSnapshotObjPtr snap = payload;
+ int *count = data;
+
+ if (snap->def->state == VIR_DOMAIN_DISK_SNAPSHOT)
+ (*count)++;
+}
+
static int
qemuDomainDestroyFlags(virDomainPtr dom,
unsigned int flags)
@@ -1787,6 +1800,7 @@ qemuDomainDestroyFlags(virDomainPtr dom,
virDomainEventPtr event = NULL;
qemuDomainObjPrivatePtr priv;
int nsnapshots;
+ int external = 0;
virCheckFlags(VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA |
VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL, -1);
@@ -1814,6 +1828,16 @@ qemuDomainDestroyFlags(virDomainPtr dom,
goto cleanup;
}
+ if ((flags & VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL) &&
+ virHashForEach(vm->snapshots.objs,
qemuDomainSnapshotCountExternal,
+ &external) &&
+ external) {
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("deletion of %d external disk snapshots not "
+ "supported yet"), external);
+ goto cleanup;
+ }
+
rem.driver = driver;
rem.vm = vm;
rem.metadata_only = !(flags & VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL);
@@ -4943,6 +4967,7 @@ qemuDomainUndefineFlags(virDomainPtr dom,
char *name = NULL;
int ret = -1;
int nsnapshots;
+ int external = 0;
virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE |
VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA |
@@ -4972,6 +4997,16 @@ qemuDomainUndefineFlags(virDomainPtr dom,
goto cleanup;
}
+ if ((flags & VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL) &&
+ virHashForEach(vm->snapshots.objs,
qemuDomainSnapshotCountExternal,
+ &external) &&
+ external) {
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("deletion of %d external disk snapshots not "
+ "supported yet"), external);
+ goto cleanup;
+ }
+
rem.driver = driver;
rem.vm = vm;
rem.metadata_only = !(flags & VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL);
@@ -9502,19 +9537,6 @@ qemuDomainSnapshotReparentChildren(void *payload,
rep->driver->snapshotDir);
}
-/* Count how many snapshots in a set have external disk snapshots. */
-static void
-qemuDomainSnapshotCountExternal(void *payload,
- const void *name ATTRIBUTE_UNUSED,
- void *data)
-{
- virDomainSnapshotObjPtr snap = payload;
- int *count = data;
-
- if (snap->def->state == VIR_DOMAIN_DISK_SNAPSHOT)
- (*count)++;
-}
-
static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
unsigned int flags)
{
@@ -9549,10 +9571,11 @@ static int
qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
goto cleanup;
}
- if (snap->def->state == VIR_DOMAIN_DISK_SNAPSHOT ||
- (virDomainSnapshotForEachDescendant(&vm->snapshots, snap,
-
qemuDomainSnapshotCountExternal,
- &external) && external)) {
+ if (!(flags & VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY) &&
+ (snap->def->state == VIR_DOMAIN_DISK_SNAPSHOT ||
+ (virDomainSnapshotForEachDescendant(&vm->snapshots, snap,
+
qemuDomainSnapshotCountExternal,
+ &external) && external))) {
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("deletion of external disk snapshots not
supported "
"yet"));
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org