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

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?

> + > + if (ret == -1) > + goto cleanup; > + > + if (uri_out) { > + if (virTypedParamsReplaceString(&params, &nparams, > + VIR_MIGRATE_PARAM_URI, uri_out) < 0) { > + orig_err = virSaveLastError(); > + goto finish; > + } > + } else { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("domainMigratePrepare3 did not set uri")); > + goto finish; > + } > + > + VIR_DEBUG("Perform3 uri=%s", NULLSTR(uri_out)); > + ret = libxlDomainMigrationPerform(driver, vm, NULL, NULL, > + uri_out, NULL, flags); > + > + if (ret < 0) > + orig_err = virSaveLastError(); > + > + cancelled = (ret < 0); > + > + finish: > + VIR_DEBUG("Finish3 ret=%d", ret); > + if (virTypedParamsGetString(params, nparams, > + VIR_MIGRATE_PARAM_DEST_NAME, NULL) <= 0 && > + virTypedParamsReplaceString(&params, &nparams, > + VIR_MIGRATE_PARAM_DEST_NAME, > + vm->def->name) < 0) { > + ddomain = NULL; > + } else { > + virObjectUnlock(vm); > + ddomain = dconn->driver->domainMigrateFinish3Params > + (dconn, params, nparams, NULL, 0, NULL, NULL, > + destflags, cancelled); > + virObjectLock(vm); > + } > + > + cancelled = (ddomain == NULL); > + > + /* If Finish3Params set an error, and we don't have an earlier > + * one we need to preserve it in case confirm3 overwrites > + */ > + if (!orig_err) > + orig_err = virSaveLastError(); > + > + VIR_DEBUG("Confirm3 cancelled=%d vm=%p", cancelled, vm); > + ret = libxlDomainMigrationConfirm(driver, vm, flags, cancelled); > + > + if (ret < 0) > + VIR_WARN("Guest %s probably left in 'paused' state on source", > + vm->def->name); > + > + cleanup: > + if (ddomain) { > + virObjectUnref(ddomain); > + ret = 0; > + } else { > + ret = -1; > + } > + > + if (orig_err) { > + virSetError(orig_err); > + virFreeError(orig_err); > + } > + > + VIR_FREE(dom_xml); > + VIR_FREE(uri_out); > + virTypedParamsFree(params, nparams); > + return ret; > +} > + > +static int virConnectCredType[] = { > + VIR_CRED_AUTHNAME, > + VIR_CRED_PASSPHRASE, > +}; > + > +static virConnectAuth virConnectAuthConfig = { > + .credtype = virConnectCredType, > + .ncredtype = ARRAY_CARDINALITY(virConnectCredType), > +}; > + > +static void > +libxlMigrationConnectionClosed(virConnectPtr conn, > + int reason, > + void *opaque) > +{ > + virDomainObjPtr vm = opaque; > + > + VIR_DEBUG("conn=%p, reason=%d, vm=%s", conn, reason, vm->def->name); > + virDomainObjBroadcast(vm); I would expect to see a corresponding virDomainObjWait for this call to virDomainObjBroadcast. But with the current migration code, I don't think the connect close callback is needed. Is there anything to do when the connection to destination libvirtd closes?

> +} > + > +/* On P2P mode there is only the Perform3 phase and we need to handle > + * the connection with the destination libvirtd and perform the migration. > + * Here we first tackle the first part of it, and libxlDoMigrationP2P handles > + * the migration process with an established virConnectPtr to the destination. > + */ > +int > +libxlDomainMigrationPerformP2P(libxlDriverPrivatePtr driver, > + virDomainObjPtr vm, > + virConnectPtr sconn, > + const char *xmlin, > + const char *dconnuri, > + const char *uri_str ATTRIBUTE_UNUSED, > + const char *dname, > + unsigned int flags) > +{ > + int ret = -1; > + int keepAliveInterval = 5; > + int keepAliveCount = 5; > + bool useParams; > + virConnectPtr dconn = NULL; > + virErrorPtr orig_err = NULL; > + > + virObjectUnlock(vm); > + dconn = virConnectOpenAuth(dconnuri, &virConnectAuthConfig, 0); > + virObjectLock(vm); > + > + if (dconn == NULL) { > + virReportError(VIR_ERR_OPERATION_FAILED, > + _("Failed to connect to remote libvirt URI %s: %s"), > + dconnuri, virGetLastErrorMessage()); > + return ret; > + } > + > + if (virConnectSetKeepAlive(dconn, keepAliveInterval, > + keepAliveCount) < 0) > + goto cleanup; > + > + if (virConnectRegisterCloseCallback(dconn, libxlMigrationConnectionClosed, > + vm, NULL) < 0) { > + goto cleanup; > + } > + > + virObjectUnlock(vm); > + useParams = VIR_DRV_SUPPORTS_FEATURE(dconn->driver, dconn, > + VIR_DRV_FEATURE_MIGRATION_PARAMS); > + virObjectLock(vm); > + > + if (!useParams) { > + virReportError(VIR_ERR_OPERATION_FAILED, "%s", > + _("Destination libvirt does not support migration with extensible parameters")); > + goto cleanup; > + } > + > + ret = libxlDoMigrateP2P(driver, vm, sconn, xmlin, dconn, dconnuri, > + dname, uri_str, flags); > + > + cleanup: > + orig_err = virSaveLastError(); > + virObjectUnlock(vm); > + virConnectUnregisterCloseCallback(dconn, libxlMigrationConnectionClosed); > + virObjectUnref(dconn); > + virObjectLock(vm); > + if (orig_err) { > + virSetError(orig_err); > + virFreeError(orig_err); > + } > + return ret; > +} > + > int > libxlDomainMigrationPerform(libxlDriverPrivatePtr driver, > virDomainObjPtr vm, > diff --git a/src/libxl/libxl_migration.h b/src/libxl/libxl_migration.h > index 20b45d8..0f83bb4 100644 > --- a/src/libxl/libxl_migration.h > +++ b/src/libxl/libxl_migration.h > @@ -28,6 +28,7 @@ > > # define LIBXL_MIGRATION_FLAGS \ > (VIR_MIGRATE_LIVE | \ > + VIR_MIGRATE_PEER2PEER | \ > VIR_MIGRATE_UNDEFINE_SOURCE | \ > VIR_MIGRATE_PAUSED) > > @@ -56,6 +57,16 @@ libxlDomainMigrationPrepare(virConnectPtr dconn, > unsigned int flags); > > int > +libxlDomainMigrationPerformP2P(libxlDriverPrivatePtr driver, > + virDomainObjPtr vm, > + virConnectPtr sconn, > + const char *dom_xml, > + const char *dconnuri, > + const char *uri_str, > + const char *dname, > + unsigned int flags); > + > +int > libxlDomainMigrationPerform(libxlDriverPrivatePtr driver, > virDomainObjPtr vm, > const char *dom_xml,