[libvirt] [PATCH v2 09/12] migration: merge all proto branches into single function

[Nikolay Shirokovskiy]

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 at 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.