[libvirt] [PATCHv4 00/51] another round of snapshot patches

[Daniel P. Berrange]

On Fri, Sep 02, 2011 at 05:57:15PM +0800, Daniel Veillard wrote:
> On Thu, Sep 01, 2011 at 10:24:37PM -0600, Eric Blake wrote:
> > I think I've addressed most findings from round 3 - by implementing
> > the ability to redefine a snapshot, it becomes possible to restore
> > snapshot hierarchy when recreating a transient domain by the same
> > name.  New goodies in this round: several bug fixes, add virsh
> > snapshot-edit, drop undefine --snapshots-full (you can only remove
> > snapshot metadata on undefine).  I tested as I went, but this went
> > through so many rebases that there may be some nasties that snuck
> > in; but I wanted to get this posted now.  I also know that I'm
> > missing at least one major feature requested in the v3 review:
> > namely, transient domains _should_ auto-remove snapshot metadata
> > files when they halt, but right now aren't doing that.
> > 
> > v3 was at:
> > https://www.redhat.com/archives/libvir-list/2011-August/msg01132.html
> > 
> > Also available here:
> > 
> > git fetch git://repo.or.cz/libvirt/ericb.git snapshot
> 
>   thanks, very convenient !
> though I had to use
>  git fetch git://repo.or.cz/libvirt/ericb.git +snapshot:snapshot
> to actually get a snapshot branch locally...
> 
> Review:
>  1 ACK
>  2 ACK
>  3,4,5,6: New flags in API ACK, it would be good to have regression tests
>           tracking all the events sent in the various cases...
>  7, 8 ACK
>  9      : New flag in API, sensible, ACK
>  10 doesn't change default behaviour, looks fine, ACK
>  11 ACK
>  12 nasty, thanks for providing a new clean iterator, ACK
>  13 ACK
>  14 good, another iterator, ACK
>  15 implementation of 9/ for qemu, ACK
>  16 ACK
>  17 ACK
>  18 ACK
>  19 new API flags, ACK
>  20 ACK
>  21 virsh extensions, ACK
>  22 ACK
>  23 ACK
>  24 ACK
>  25 ACK unfortunately the half baked state of 0.9.4 is gonna remain
>         for a while
>  26 ACK
>  27 I'm not so sure about that, as the caching is infinite. Some module
>     rely on inotify already, and best would be to add an utility for
>     inotify use and then use it on the dirs of $PATH, then upon change
>     discard the cacher path
>     I would push for now but add a TODO to fix that problem
>  28 ACK
>  29 Isn't there a way to save the domain snapshot on shared storage when
>     available to try to avoid the problem ? It wouldn't work all the
>     time but might be simpler than rolling out a v4. or consider the
>     snapshot data as extra domain resource that could be migrated on
>     the fly like we can do for disk images in some cases.
>  30 ACK
>  31 ACK
>  32 argh ... ACK
>  33 the new rng need to be added to libvirt.spec.in file list,
>     once done ACK
>  34 ACK
>  35, 36 ACK
>  37 ACK
>  38 ACK
>  39 ACK to new API flag
>  40 ACK
>  41 ACK
>  42 ACK
>  43 ACK
>  44 nice improvement, hopefully can't lead to regressions, and also
>     end up cleaning up the code in a few places, ACK
>  45 useful for scripting, ACK
>  46 ACK
>  47 ACK
>  48 ACK
>  49 ACK, mechnical mostly
>  50 ACK
>  51 ACK
> 
> Elapsed time: 3h 20mn
> 
> now the 100hours question is how are we gonna test all this in a
> reasonable fashion and outside of your environment :-)
> I think we should push, but need a testing plan because I don't think
> we can reasonably expect people to test this in time for 0.9.5,

The number of patches & complexity of bugs being fixed here
clearly demonstrates that having indivduals test this is not
feasible. Thus I have been working on creating some TCK tests
which exercise the snapshot code, and in particular try to
hit the bugs Eric has been fixing & check the expected output.
I posted a couple of tests last week which I'll improve and
there are more to be written...

Regards,
Daniel