Re: [Xen-devel] [PATCH v2] libxl: add p2p migration

[Jim Fehlig <jfehlig@xxxxxxxx>]

On 12/03/2015 12:18 PM, Joao Martins wrote:
>
> On 12/02/2015 11:27 PM, Jim Fehlig wrote:
>> On 11/10/2015 08:32 AM, Joao Martins wrote:
>>> Introduce support for VIR_MIGRATE_PEER2PEER in libxl driver
>>> for supporting migration in Openstack. Most of the changes
>>> occur at the source and no modifications at the receiver.
>>>
>>> In P2P mode there is only the Perform phase so we must handle
>>> the connection with the destination and actually perform the
>>> migration. libxlDomainPerformP2P implements the connection to
>>> the destination and let libxlDoMigrateP2P implements the actual
>>> migration logic with virConnectPtr. In this function we do
>>> the migration steps in the destination similar to
>>> virDomainMigrateVersion3Full. We appropriately save the last
>>> error reported in each of the phases to provide proper
>>> reporting. We don't yet support VIR_MIGRATE_TUNNELED and
>>> we always use V3 with extensible params, making the
>>> implementation simpler.
>>>
>>> It is worth noting that the receiver didn't have any changes,
>>> and because it's still the v3 sequence thus it is possible to
>>> migrate from a P2P to non-P2P host.
>>>
>>> Signed-off-by: Joao Martins <joao.m.martins@xxxxxxxxxx>
>>> ---
>>> Changes since v1:
>>>  - Move Begin step to libxlDoMigrateP2P to have all 4 steps
>>>  together.
>>>  - Remove if before VIR_FREE(dom_xml)
>>> ---
>>>  src/libxl/libxl_driver.c    |  13 ++-
>>>  src/libxl/libxl_migration.c | 220 
>>> ++++++++++++++++++++++++++++++++++++++++++++
>>>  src/libxl/libxl_migration.h |  11 +++
>>>  3 files changed, 241 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>>> index fcdcbdb..da98265 100644
>>> --- a/src/libxl/libxl_driver.c
>>> +++ b/src/libxl/libxl_driver.c
>>> @@ -4713,6 +4713,7 @@ libxlConnectSupportsFeature(virConnectPtr conn, int 
>>> feature)
>>>      switch (feature) {
>>>      case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
>>>      case VIR_DRV_FEATURE_MIGRATION_PARAMS:
>>> +    case VIR_DRV_FEATURE_MIGRATION_P2P:
>>>          return 1;
>>>      default:
>>>          return 0;
>>> @@ -5039,9 +5040,15 @@ libxlDomainMigratePerform3Params(virDomainPtr dom,
>>>      if (virDomainMigratePerform3ParamsEnsureACL(dom->conn, vm->def) < 0)
>>>          goto cleanup;
>>>  
>>> -    if (libxlDomainMigrationPerform(driver, vm, dom_xml, dconnuri,
>>> -                                    uri, dname, flags) < 0)
>>> -        goto cleanup;
>>> +    if (flags & VIR_MIGRATE_PEER2PEER) {
>>> +        if (libxlDomainMigrationPerformP2P(driver, vm, dom->conn, dom_xml,
>>> +                                           dconnuri, uri, dname, flags) < 
>>> 0)
>>> +            goto cleanup;
>>> +    } else {
>>> +        if (libxlDomainMigrationPerform(driver, vm, dom_xml, dconnuri,
>>> +                                        uri, dname, flags) < 0)
>>> +            goto cleanup;
>>> +    }
>>>  
>>>      ret = 0;
>>>  
>>> diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
>>> index 0d23e5f..a1c7b55 100644
>>> --- a/src/libxl/libxl_migration.c
>>> +++ b/src/libxl/libxl_migration.c
>>> @@ -42,6 +42,7 @@
>>>  #include "libxl_conf.h"
>>>  #include "libxl_migration.h"
>>>  #include "locking/domain_lock.h"
>>> +#include "virtypedparam.h"
>>>  
>>>  #define VIR_FROM_THIS VIR_FROM_LIBXL
>>>  
>>> @@ -456,6 +457,225 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
>>>      return ret;
>>>  }
>>>  
>>> +/* This function is a simplification of virDomainMigrateVersion3Full
>>> + * excluding tunnel support and restricting it to migration v3
>>> + * with params since it was the first to be introduced in libxl.
>>> + */
>>> +static int
>>> +libxlDoMigrateP2P(libxlDriverPrivatePtr driver,
>>> +                  virDomainObjPtr vm,
>>> +                  virConnectPtr sconn,
>>> +                  const char *xmlin,
>>> +                  virConnectPtr dconn,
>>> +                  const char *dconnuri ATTRIBUTE_UNUSED,
>>> +                  const char *dname,
>>> +                  const char *uri,
>>> +                  unsigned int flags)
>>> +{
>>> +    virDomainPtr ddomain = NULL;
>>> +    virTypedParameterPtr params = NULL;
>>> +    int nparams = 0;
>>> +    int maxparams = 0;
>>> +    char *uri_out = NULL;
>>> +    char *dom_xml = NULL;
>>> +    unsigned long destflags;
>>> +    bool cancelled = true;
>>> +    virErrorPtr orig_err = NULL;
>>> +    int ret = -1;
>>> +
>>> +    dom_xml = libxlDomainMigrationBegin(sconn, vm, xmlin);
>>> +    if (!dom_xml)
>>> +        goto cleanup;
>>> +
>>> +    if (virTypedParamsAddString(&params, &nparams, &maxparams,
>>> +                                VIR_MIGRATE_PARAM_DEST_XML, dom_xml) < 0)
>>> +        goto cleanup;
>>> +
>>> +    if (dname &&
>>> +        virTypedParamsAddString(&params, &nparams, &maxparams,
>>> +                                VIR_MIGRATE_PARAM_DEST_NAME, dname) < 0)
>>> +        goto cleanup;
>>> +
>>> +    if (uri &&
>>> +        virTypedParamsAddString(&params, &nparams, &maxparams,
>>> +                                VIR_MIGRATE_PARAM_URI, uri) < 0)
>>> +        goto cleanup;
>>> +
>>> +    /* We don't require the destination to have P2P support
>>> +     * as it looks to be normal migration from the receiver perpective.
>>> +     */
>>> +    destflags = flags & ~(VIR_MIGRATE_PEER2PEER);
>>> +
>>> +    VIR_DEBUG("Prepare3");
>>> +    virObjectUnlock(vm);
>>> +    ret = dconn->driver->domainMigratePrepare3Params
>>> +        (dconn, params, nparams, NULL, 0, NULL, NULL, &uri_out, destflags);
>>> +    virObjectLock(vm);
>> Is it necessary to unlock and lock the vm while calling prepare and finish on
>> the destination libvirtd?
>>
> They can be removed, but then we leave the virDomainObj locked for a "long" 
> time
> when no operations are being done on his context. Plus no API calls could be
> done there in the meantime (and migration can take a rather long time).

Ah, right. Dropping the lock is fine, but IMO we'll need a modify job to protect
the domain from modification by another thread. I think the existing migration
code needs improvement wrt job handling too :-/.

Regards,
Jim


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel