[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv3 13/43] snapshot: prevent stranding snapshot data on domain destruction

On 08/24/2011 10:49 AM, Daniel P. Berrange wrote:
On Wed, Aug 24, 2011 at 09:22:30AM -0600, Eric Blake wrote:
Just as leaving managed save metadata behind can cause problems
when creating a new domain that happens to collide with the name
of the just-deleted domain, the same is true of leaving any
snapshot metadata behind.  For safety sake, extend the semantic
change of commit b26a9fa9 to also cover snapshot metadata as a
reason to reject losing the last reference to a domain (undefine
on an inactive, or shutdown/destroy on a transient).  The caller
must first take care of snapshots, possible via the existing

This also documents some new flags that hypervisors can choose to
support to shortcuts taking care of the metadata as part of the
shutdown process; however, nontrivial driver support for these flags
will be deferred to future patches.

+/* Counterparts to virDomainUndefineFlagsValues, but note that running
+ * domains have no managed save data, so no flag is provided for that.
+typedef enum {
+    /* VIR_DOMAIN_DESTROY_MANAGED_SAVE = (1<<  0), */ /* Reserved */
+    VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA = (1<<  1), /* If last use of domain,
+                                                         then also remove any
+                                                         snapshot metadata */

This one is always safe - the only things deleted are libvirt files, leaving all the qcow2 disks and external drives intact.

+    VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL     = (1<<  2), /* If last use of domain,
+                                                         then also remove any
+                                                         snapshot data */

But I am starting to be convinced by your assertion that this is dangerous - there is no way to cleanly report partial failure with a flag like this, which means that in the failure case, it did not add any convenience, but made things worse.

This feels a little bit wrong to me, for the same reasons we
discussed wrt deleteing disks on undefine, particularly once
we start storing snapshots in external files across arbitrary
storage, instead of just inside qcow2 images


How about a compromise? Are you willing to agree that deleting snapshot metadata is always safe (the user's disk images are still intact, whether qcow2 or external, and only the hidden files in /var/lib/libvirt are removed so that they cannot interfere with later creation of a new domain by the same name)? That is, should I rework this patch for just the metadata flag, or drop it entirely?

And there's still the O(n) vs. O(n^2) consideration; if we drop this patch (or even if we keep the metadata portion of this patch but drop the full deletion aspect), I have to wonder if there is any way we can improve existing virDomainSnapshotDelete (or create a new API) that allows deletion of multiple snapshots without inherent quadratic behavior caused by deleting one snapshot at a time forcing all other snapshots to be checked for reparenting.

Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]