Re: [libvirt] [PATCHv4 00/51] another round of snapshot patches
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/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/