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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list