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

[Daniel P. Berrange]

On Fri, Aug 26, 2011 at 08:57:54AM -0600, Eric Blake wrote:
> 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.

Surely this simply means that the snapshot metadata files shold
be recording the orignal domain UUID, so that we can safely
ignore/discard them if the guest is re-defined with the same
name but new uuid ?

> >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.

This is all workable, but is it overkill compared to just validating
the original VM uuid against the new UUID when loading metdata ?


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|