Re: [libvirt] [PATCH] migration: support all toURI and proto combos

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