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

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.

Overall, if you see a way to refactor and improve the code so that it's more readable and maintainable, just go ahead, but be careful about this kind of changes in behavior.

Jirka