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

Nikolay Shirokovskiy nshirokovskiy at virtuozzo.com
Thu Oct 15 14:15:21 UTC 2015

In short, may be factor out common p2p code that now resides in libvirt-domain.c
and reuse it for vz? Looks like vz driver is not a good place
for such common and kinda complicated code.

On 11.09.2015 14:06, Nikolay Shirokovskiy wrote:
> 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