[libvirt] [PATCH v5 2/5] vz: add migration backbone code

Nikolay Shirokovskiy nshirokovskiy at parallels.com
Fri Sep 11 11:06:46 UTC 2015

On 11.09.2015 12:28, Daniel P. Berrange wrote:
> On Fri, Sep 11, 2015 at 12:05:54PM +0300, Nikolay Shirokovskiy wrote:
>>> As mentioned in the previous review, I really want to see VZ
>>> provide the full set of drive callbacks required by the V3
>>> migration protocol, not just those you happen to need to have
>>> today. ie add Begin, Finish & Confirm.  This in turn makes it
>>> quite easy to support non-P2P mode which is desirable because
>>> in P2P mode there is no practical way todo interactive auth
>>> for the destination connection, but in non-P2P mode the client
>>> app opens the destination connection, so can do auth if needed.
>> Ok, I just wanted this to be a matter of a different patchset.
>>> I believe that the following changes to this patch should do
>>> all that is required. NB, I have only compile tested this,
>>> not actually run it, since I don't have an active VZ install,
>>> only the SDK install.
>> For me the patch you suggested is overkill in part of p2p.                                                          
>> vzDomainMigratePerform3P2PParams follows virDomainMigrateVersion3Full too                                           
>> close, while vz p2p migration is much simplier that qemu or some abstract case.
> As you can see from the fact that we have many versions of the
> migration protocol, what appears obviously sufficient when first
> implmented, has frequently turned out to be insufficient. So while
> you only need 2 calls today, I would not bet on that always being
> sufficient in the future. Implementing all phases today provides
> better future proofing, should we need to make use of them in the
> future, as it allows the potential to implement fixes / workarounds
> on the dest, without relying on also lock-step upgrading the host
> to add the callers you didn't make to start with.
> It would also be desirable in the future, to provide the means to
> share the code impl for P2P migration across all the hypervisor
> drivers. This would again imply supporting all stages of the v3
> migration protocol. If we don't do this now we will create problems
> in the future if we wish to change to shared code, because any
> shared impl would be calling functions that were not implemented
> on older versions of VZ.

Well I'm not against implementing the full set of protocol functions.  This
will be done anyway as a part of supporting direct managed migrating scheme as
you recommend. So it is not a point of dispute.

The point of dispute is what p2p migration should look like. Well I understand
the argument of ease of fixes/workaronds. But if there is even a disire in
future share p2p code why not to do it from the start?

Look vzDomainMigratePerform3P2PParams is identical to
virDomainMigrateVersion3Full except for that last one supports plain version
3 protocol.  Can we reuse virDomainMigrateVersion3Full? Thus at least for vz
p2p and direct managed will be identical from procedural POV, the only
difference is who executes the migration procedure - client or daemon. So the
last statement will be not only a declaration but implemented in code.

>> 1. Confirm phase is noop and this even won't change in future as vz migration                                       
>>    is practically unmanaged direct one. So why call it in p2p at all?                                               
>> 2. Finish phase function is to return migrated domain only(in vz case). Thus it                                     
>>    have meaning only for direct managed case due its API definition.                                                
>> 3. Calling begin phase in p2p is excessive too. We don't use generated dxml, so                                     
>>    this phase is only a stub for direct managed case.                                                               
>> Even if we support changing domain xml in future it is more practical                                               
>> to use specific functions and not general API which cases a lot                                                     
>> of preparation work and has rather uncomfortable way of passing                                                     
>> parameters.                                                                                                         
>> I'd better just add a small patch to existent vzDomainMigratePerform3Params which                                   
>> branches on how to get miguri and session_uuid.                                                                     
>> As to supporting all API versions. I resent a second version of the patch                                           
>> that should cover the case of toURI API so that implementing only                                                   
>> version3 params protocol functions would be enough. But the patch                                                   
>> does not cover the case of migration{N} API.

Looks like you misunderstood me. Here i just wanted you to
review https://www.redhat.com/archives/libvir-list/2015-September/msg00397.html patch.
and estimate it as an alternative to helpers methods you suggested
in https://www.redhat.com/archives/libvir-list/2015-September/msg00372.html.

> This patch I have provided covers the normal virDomainMigrate  API, in
> addition to the virDomainMigrateToURI method. As I mentioned, supporting
> virDomainMigrate is desirable, because it allows the calling application
> to perform arbitrary authentication with the destination libvirtd, which
> is not possible when using P2P migration. Supporting the non-P2P migration
> mode requires that we provide all the callbacks defined for version 3,
> and once we do this, it is sensible to support all the callbacks in the
> P2P case too, to ensure we have identical functional behaviour in both
> cases.

> So in summary, I want to see the full V3 migration protocol implemented
> in VZ before this is acceptable for merge. This patch I supplied would
> be sufficient to achieve this.
> Regards,
> Daniel

More information about the libvir-list mailing list