On 17.09.2015 17:39, Daniel P. Berrange wrote: > 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. Thanx! I'll incorporate yours and Jonhs suggestions in next version so Jiri will see the "final" version. > > Regards, > Daniel > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list