On 09.09.2015 17:13, Daniel P. Berrange wrote: > On Tue, Sep 08, 2015 at 02:14:32PM +0200, Jiri Denemark wrote: >> On Tue, Sep 08, 2015 at 13:43:09 +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. >> >> In general, this patch is pretty large and hard to review, I suggest >> splitting it into a series of several shorter patches. They all need to >> compile and work, but it shouldn't be too hard in this case. >> >>> 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. >> >> Well, the thing is all the code you're changing runs on a *client* and >> then the appropriate API calls are sent as RPC to a daemon. So just >> checking what APIs are implemented by the *client's* remote driver to >> choose what API will be called on the daemon is wrong. The client can be >> new enough to support all migration APIs while the daemon may be pretty >> old supporting only a few of them. Thus, for backward compatibility, the >> client has to either check what API is supported by the daemon (via >> VIR_DRV_SUPPORTS_FEATURE(..., VIR_DRV_FEATURE_MIGRATION_...)) or it has >> to be conservative and call the oldest API which supports the parameters >> passed by the application/user. >> >> Moreover, various versions of virDomainMigrateToURI are not really about >> the migration protocol to be used. They differ only in the set of >> parameters, the actual migration protocol will be decided by the daemon >> itself after making a connection to the destination daemon. That said, >> it should be fairly easy to implement all the entry points in a driver >> while still allowing only protocol version 3. > > It seems to be that the only real critical thing is that all drivers > support both V3 and V3Params driver callbacks. We could easily achieve > that by creating a src/migration-helpers.c that provides the stub > impls of V3 callbacks which simply pack args into a virTypedParams > array and then invoke the corresponding V3Params callbacks. Those > helpers can be just referenced from the virHypervisorDriver struct. > > So I don't think we need to change src/libvirt-host.c for this particular > problem > > See the same recommendation I just made here wrt libxl driver which > has the same need as vz > > https://www.redhat.com/archives/libvir-list/2015-September/msg00372.html > > Regards, > Daniel > Well I'm just about to finish the splitting of combo patch into more understanable set. Although helpers could be a solution this patchset has another value that it reduces a tangle in this aspect and code dups in libvirt-domain.c. Hope you'll check it. -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list