[libvirt] [PATCHv3 13/43] snapshot: prevent stranding snapshot data on domain destruction
Eric Blake
eblake at redhat.com
Wed Aug 24 16:56:25 UTC 2011
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
>> virDomainSnapshotDelete.
>>
>> 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
>
> https://www.redhat.com/archives/libvir-list/2011-July/msg01548.html
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 at redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
More information about the libvir-list
mailing list