On 17.09.2015 17:32, John Ferlan wrote:
On 09/10/2015 09:20 AM, Nikolay Shirokovskiy wrote:
> Finally on this step we get what we were aimed for - toURI{1, 2} (and
> migration{*} APIs too) now can work thru V3_PARAMS protocol. Execution path
> goes thru unchanged virDomainMigrateUnmanaged adapter function which is called
> by all target places.
>
> Note that we keep the fact that direct migration never works
> thru V3_PARAMS proto. We can't change this aspect without
> further investigation.
>
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
> ---
> src/libvirt-domain.c | 53 ++++++++++++++-----------------------------------
> 1 files changed, 15 insertions(+), 38 deletions(-)
>
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
>
> - if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
> + if ((flags & VIR_MIGRATE_PEER2PEER) &&
> + VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
> + VIR_DRV_FEATURE_MIGRATION_PARAMS)) {
> + VIR_DEBUG("Using migration protocol 3 with extensible
parameters");
> + if (!domain->conn->driver->domainMigratePerform3Params) {
> + virReportUnsupportedError();
> + return -1;
> + }
> + return domain->conn->driver->domainMigratePerform3Params
> + (domain, dconnuri, params, nparams,
> + NULL, 0, NULL, NULL, flags);
> + } else if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver,
domain->conn,
> VIR_DRV_FEATURE_MIGRATION_V3)) {
It almost seems things could be written as:
if (flags & VIR_MIGRATE_PEER2PEER) {
if (virDomainMigrateCheckNotLocal(dconnuri) < 0)
....
if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
....)
else if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver,
...
else
something's wrong
} else {
VIR_DEBUG("Using migration protocol 2");
...
}
IOW: I'm trying to isolate the dconnuri a bit more. Up to this point it
seems the assumption calling this routine was that if we support
VIR_DRV_FEATURE_MIGRATION_V3, then this would only be called if
VIR_MIGRATE_PEER2PEER was true, correct?
I can't agree. This way we get a code dup as direct and p2p switch
for protocol version are no different except for v3_params. And even
this little difference could get away in the future as the reason to
keep it is unclear.
We'd better check that dconnuri is not NULL for p2p somewhere earlier
to make things consistent. In different patch i guess.