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@xxxxxxxxxxxxx> >> --- >> 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. -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list