On Thu, Sep 17, 2015 at 01:11:59PM +0100, Daniel P. Berrange wrote: > On Thu, Sep 10, 2015 at 04:20:12PM +0300, Nikolay Shirokovskiy wrote: > > Current implementation of 'toURI' migration interfaces does not support all > > combinations of interface versions and protocol versions. For example 'toURI2' > > with p2p flag will not migrate if driver supports only v3params proto. > > > > This is not convinient as drivers that starts to support migration have to > > manually support older versions of protocol. I guess this should be done in > > one place, namely here. > > > > Another issue is that there are a lot of code duplication in implementation of > > toURI interfaces and it is not obvious from code how are they related. > > > > This implementation uses extensible parameters as intermediate parameters > > representation. This is possible as interfaces are done backward compatible in > > terms of parameters and later versions supports all parameters of former > > versions. > > = Changes from version1 > > > > Patch is splitted into a set. Quite a big one as a result of the following strategy: > > > > 1. each change in behaviour even subtle one deserves a separate patch. One > > patch changes one aspect in behaviour. > > > > 2. separate pure refactoring steps into patches too as rather simple refactor > > steps could introduce many line changes. Mark such patches with 'refactor:' > > > > Now every patch is easy to grasp I think. > > > > The resulted cumulative patch is slightly different from first in behaviour but > > I'm not going to describe the differece here as original patch was not reviewed > > in details by anyone anyway ) > > > > src/libvirt-domain.c | 520 +++++++++++++++++++++----------------------------- > > 1 files changed, 216 insertions(+), 304 deletions(-) > > Just a quick note to say that I haven't forgotten about this patch > series. I'm looking to review it today/tomorrow I hope. So I've finally run through this. The patches look fine, and I also compared the final state, with the current code to understand the final logic we now have. That all still looks good and has the nice new features you mention. There's the few points John points out in review, but nothing too critical I've seen so far. Given we have a history of screwing up migration though, I'd like one of the other migration experts, such as Jiri, to take a look too before we push it. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list