[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