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

[Eric Blake]

On 09/02/2011 03:57 AM, 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

I'll commit things in phases, broken out by BZs that each phase fixes.

I have now pushed patch 1 (BZ 674537).

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

These tests will have to be in the TCK, unless we were to also teach 
src/tests/test_driver.c the same events.  Improving tests:///default to 
cover events is certainly doable in the future, but not in time for 0.9.5.

>   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

If we expected the location of qemu-img along a PATH search to change, 
then I see your point.  But I see nothing wrong with caching that 
qemu-img lives in /usr/bin (or wherever else it was found) - that is 
unlikely to change, even if you upgrade the package that provided 
qemu-img in the meantime.

Using inotify cache expulsion is more useful for things like caching 
'qemu -help' output - it's one thing to cache where qemu lives, it's 
another to also cache what that version of qemu supports, and the cache 
must be invalidated if the file at the cached location changes due to a 
package upgrade.  But in this case, we aren't caching qemu-img capabilities.

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

Certainly lots of room for improvement along this front, but probably 
most of it will be post-0.9.5.  Right now, we're just getting the basics 
in place.

>   30 ACK
>   31 ACK
>   32 argh ... ACK
>   33 the new rng need to be added to libvirt.spec.in file list,
>      once done ACK

Good catch.  I also had to rebase on top of Osier's change to domain.rng 
(git wasn't very nice, claiming a conflict of several thousand lines 
even though it was really just a single addition).

>
> Elapsed time: 3h 20mn

Not bad, considering it took me several weeks to reach 51 pending patches.

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

I'll have everything pushed in the next 8 hours or so, in batches tied 
to BZ as I mentioned above.  I'll have limited connectivity next week 
(I'll be traveling), so hopefully I've tested well enough that I'm not 
leaving people in a lurch by pushing a large series then disappearing 
for a few days.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org