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,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to