[libvirt] [PATCH 0/3 TCK] Some tests for snapshot management

[Eric Blake]

On 08/26/2011 08:16 AM, Daniel P. Berrange wrote:
> All these tests fail with current GIT, but are intended to work
> with Eric's snapshot series applied.
>
> The first test in this list, however, does not pass.
>
> Eric's current tree forbids calling 'Destroy' on a transient
> guest which has snapshots present. IMHO this is wrong, because
> the guest may itself exit at any time, which leaves us in the
> same state as is 'Destroy' had been called wrt snapshots.

Hmm, interesting point.

With managed save, we don't have that problem - the only way to have a 
managed save image is with a persistent def, so we can safely forbid 
undefine of a domain with managed save state.

>
> IMHO, we should be allowed to call 'virDomainDestroy' on a
> transient guest with snapshots, and then later 'virDomainCreateXML'
> to re-create the guest and still have access to the snapshots

But leaving the snapshot metadata files behind is problematic.  If you 
create a new domain with the same name but a different uuid, then those 
existing metadata files _will_ cause problems with the domain definition.

>
> In other words, just because a transient guest is not currently
> present, does not mean we should forget about its snapshots as
> a management app can re-create it again at any time.

How about this compromise?

I already documented in my RFC the desire to add a new flag, 
virDomainSnapshotCreateXML(, VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE), which 
will allow a management app to recreate a metadata file.  I haven't yet 
implemented it, but think that it will solve this problem, as follows:

Libvirt should not ever forbid destroy of a domain with snapshots. 
However, when a transient guest shuts down, then part of the cleanup 
that libvirt does for that guest is to remove all existing snapshot 
metadata (the snapshots themselves continue to exist, but the libvirt 
hierarchy of creation date, domain xml, and parent relationships is gone).

On destroy, either the domain is persistent (so the snapshots will still 
be useful from the inactive guest), or the domain is transient (so the 
management app has to be aware of the ramifications).  But the 
management app _already_ has to track domain xml independently if it 
plans on recreating a new transient guest from the same disk images, so 
it is not too much of a stretch to assume that the management app can 
also track snapshot xml.  At which point, if the management app wants to 
recreate a new transient app with the same snapshot metadata, then it 
simply calls virDomainSnapshotCreateXML(, 
VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) for each saved snapshot that it 
wants to revive in libvirt's tracking.

Thus, snapshot metadata is never left behind to confuse the creation of 
a new guest (persistent domains fail to undefine until the snapshots are 
cleaned up, and transient domains are cleaned up automatically when the 
last reference to the domain disappears), but can easily be restored and 
thus tracked externally.

However, this means that your first test would need tweaking - the test 
itself would have to create the snapshot, capture the snapshots xml, 
destroy the domain, then create a new domain with the same name and 
uuid, restore the snapshot, then revert to the snapshot, all to prove 
that redefining a transient domain does not lose the actual snapshot 
data, just the metadata that tracked the snapshot.

There's also some FIXMEs in libvirt about the notion of extracting the 
name of all internal snapshots directly from qcow2 files, and exposing 
those to the user with minimal information.  qcow2 lacks parent and 
domain xml information, so the regenerated snapshot xml would be weak 
and reverting to it would require the use of 
VIR_DOMAIN_SNAPSHOT_REVERT_FORCE; but making it easier to expose all 
known internal snapshot names, even when libvirt does not have the 
metadata behind the origin of those internal snapshots, might be useful 
via a new flag to virDomainSnapshotListNames.

>
> Forbidding virDomainUndefine with snapshots is more reasonable,

Good, I'll keep it that way, since it is consistent with managed save 
images.

> but I think its still possible to argue that it should be allowed
> and that a later virDomainDefine with the same name+uuid should
> resurrect any previous snapshots.

If resurrecting previous snapshots is important, then it should be done 
with VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE.

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