Re: [libvirt] [PATCH v2] libxl: add p2p migration

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(a)oracle.com&gt; >> --- >> 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).