[libvirt] [PATCH v2 0/3] libxl: tunnelled migration support

Hey! Presented herewith is take 2 from tunnelled migration addressing all previous comments. Changelog in individual patches (patch 1 and 2 are small refactorings suggested in v1) Despite being functional changes mostly I had a quick round of testing too. Thanks, Joao Bob Liu (1): libxl: add tunnelled migration support Joao Martins (2): libxl: refactor libxlDomainMigrationPrepare libxl: streamline top-level migrate functions src/libxl/libxl_driver.c | 79 ++++++++-- src/libxl/libxl_migration.c | 372 +++++++++++++++++++++++++++++++++++++------- src/libxl/libxl_migration.h | 9 ++ 3 files changed, 393 insertions(+), 67 deletions(-) -- 2.1.4

The newly introduced function libxlDomainMigrationPrepareAny will be shared between P2P and tunnelled variations. Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- New in v2 --- src/libxl/libxl_migration.c | 92 +++++++++++++++++++++++++++------------------ 1 file changed, 56 insertions(+), 36 deletions(-) diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index a471d2a..7beabd2 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -483,41 +483,26 @@ libxlDomainMigrationPrepareDef(libxlDriverPrivatePtr driver, return def; } -int -libxlDomainMigrationPrepare(virConnectPtr dconn, - virDomainDefPtr *def, - const char *uri_in, - char **uri_out, - const char *cookiein, - int cookieinlen, - unsigned int flags) +static int +libxlDomainMigrationPrepareAny(virConnectPtr dconn, + virDomainDefPtr *def, + const char *cookiein, + int cookieinlen, + libxlMigrationCookiePtr *mig, + char **xmlout, + bool *taint_hook) { libxlDriverPrivatePtr driver = dconn->privateData; libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); - libxlMigrationCookiePtr mig = NULL; - virDomainObjPtr vm = NULL; - char *hostname = NULL; - char *xmlout = NULL; - unsigned short port; - char portstr[100]; - virURIPtr uri = NULL; - virNetSocketPtr *socks = NULL; - size_t nsocks = 0; - int nsocks_listen = 0; - libxlMigrationDstArgs *args = NULL; - bool taint_hook = false; - libxlDomainObjPrivatePtr priv = NULL; - size_t i; - int ret = -1; - if (libxlMigrationEatCookie(cookiein, cookieinlen, &mig) < 0) - goto error; + if (libxlMigrationEatCookie(cookiein, cookieinlen, mig) < 0) + return -1; - if (mig->xenMigStreamVer > LIBXL_SAVE_VERSION) { + if ((*mig)->xenMigStreamVer > LIBXL_SAVE_VERSION) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("Xen migration stream version '%d' is not supported on this host"), - mig->xenMigStreamVer); - goto error; + (*mig)->xenMigStreamVer); + return -1; } /* Let migration hook filter domain XML */ @@ -528,29 +513,29 @@ libxlDomainMigrationPrepare(virConnectPtr dconn, if (!(xml = virDomainDefFormat(*def, cfg->caps, VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_MIGRATABLE))) - goto error; + return -1; hookret = virHookCall(VIR_HOOK_DRIVER_LIBXL, (*def)->name, VIR_HOOK_LIBXL_OP_MIGRATE, VIR_HOOK_SUBOP_BEGIN, - NULL, xml, &xmlout); + NULL, xml, xmlout); VIR_FREE(xml); if (hookret < 0) { - goto error; + return -1; } else if (hookret == 0) { - if (virStringIsEmpty(xmlout)) { + if (virStringIsEmpty(*xmlout)) { VIR_DEBUG("Migrate hook filter returned nothing; using the" " original XML"); } else { virDomainDefPtr newdef; - VIR_DEBUG("Using hook-filtered domain XML: %s", xmlout); - newdef = virDomainDefParseString(xmlout, cfg->caps, driver->xmlopt, + VIR_DEBUG("Using hook-filtered domain XML: %s", *xmlout); + newdef = virDomainDefParseString(*xmlout, cfg->caps, driver->xmlopt, NULL, VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE); if (!newdef) - goto error; + return -1; /* TODO At some stage we will want to have some check of what the user * did in the hook. */ @@ -560,17 +545,52 @@ libxlDomainMigrationPrepare(virConnectPtr dconn, /* We should taint the domain here. However, @vm and therefore * privateData too are still NULL, so just notice the fact and * taint it later. */ - taint_hook = true; + *taint_hook = true; } } } + return 0; +} + +int +libxlDomainMigrationPrepare(virConnectPtr dconn, + virDomainDefPtr *def, + const char *uri_in, + char **uri_out, + const char *cookiein, + int cookieinlen, + unsigned int flags) +{ + libxlDriverPrivatePtr driver = dconn->privateData; + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); + libxlMigrationCookiePtr mig = NULL; + virDomainObjPtr vm = NULL; + char *hostname = NULL; + char *xmlout = NULL; + unsigned short port; + char portstr[100]; + virURIPtr uri = NULL; + virNetSocketPtr *socks = NULL; + size_t nsocks = 0; + int nsocks_listen = 0; + libxlMigrationDstArgs *args = NULL; + bool taint_hook = false; + libxlDomainObjPrivatePtr priv = NULL; + size_t i; + int ret = -1; + + if (libxlDomainMigrationPrepareAny(dconn, def, cookiein, cookieinlen, + &mig, &xmlout, &taint_hook) < 0) + goto error; + if (!(vm = virDomainObjListAdd(driver->domains, *def, driver->xmlopt, VIR_DOMAIN_OBJ_LIST_ADD_LIVE | VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, NULL))) goto error; + *def = NULL; priv = vm->privateData; -- 2.1.4

This allows us to reuse a single function for both tunnelled and non-tunnelled variants. Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- New in v2 --- src/libxl/libxl_driver.c | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 3a69720..7bc8adf 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -5930,21 +5930,22 @@ libxlDomainMigrateBegin3Params(virDomainPtr domain, } static int -libxlDomainMigratePrepare3Params(virConnectPtr dconn, - virTypedParameterPtr params, - int nparams, - const char *cookiein, - int cookieinlen, - char **cookieout ATTRIBUTE_UNUSED, - int *cookieoutlen ATTRIBUTE_UNUSED, - char **uri_out, - unsigned int flags) +libxlDomainMigratePrepareCommon(virConnectPtr dconn, + virTypedParameterPtr params, + int nparams, + const char *cookiein, + int cookieinlen, + char **cookieout ATTRIBUTE_UNUSED, + int *cookieoutlen ATTRIBUTE_UNUSED, + unsigned int flags, + void *data) { libxlDriverPrivatePtr driver = dconn->privateData; virDomainDefPtr def = NULL; const char *dom_xml = NULL; const char *dname = NULL; const char *uri_in = NULL; + char **uri_out = data; #ifdef LIBXL_HAVE_NO_SUSPEND_RESUME virReportUnsupportedError(); @@ -5985,6 +5986,23 @@ libxlDomainMigratePrepare3Params(virConnectPtr dconn, } static int +libxlDomainMigratePrepare3Params(virConnectPtr dconn, + virTypedParameterPtr params, + int nparams, + const char *cookiein, + int cookieinlen, + char **cookieout ATTRIBUTE_UNUSED, + int *cookieoutlen ATTRIBUTE_UNUSED, + char **uri_out, + unsigned int flags) +{ + return libxlDomainMigratePrepareCommon(dconn, params, nparams, + cookiein, cookieinlen, + cookieout, cookieoutlen, + flags, uri_out); +} + +static int libxlDomainMigratePerform3Params(virDomainPtr dom, const char *dconnuri, virTypedParameterPtr params, -- 2.1.4

From: Bob Liu <bob.liu@oracle.com> Tunnelled migration doesn't require any extra network connections beside the libvirt daemon. It's capable of strong encryption and the default option of openstack-nova. This patch adds the tunnelled migration(Tunnel3params) support to libxl. On the source side, the data flow is: * libxlDoMigrateSend() -> pipe libxlTunnel3MigrationFunc() polls pipe * out and then write to dest stream. While on the destination side: * Stream -> pipe -> 'recvfd of libxlDomainStartRestore' The usage is the same as p2p migration, execpt adding one extra '--tunnelled' to the libvirt p2p migration command. Signed-off-by: Bob Liu <bob.liu@oracle.com> Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- v2 (comments from Jim): * Adjust common preparetunnel function reflecting v1 suggestions - Using common top-level Prepare function by adding is_tunnel variable - Using libxl_migration PrepareAny added in patch 1 - virHook support in PrepareTunnel3 * Move virThreadPtr from libxlTunnelMigrationThread into libxlTunnelControl * Rename function local variable from tmThreadPtr to arg * Remove redundant assignment "tmThreadPtr = &tc->tmThread;" always being not null. --- src/libxl/libxl_driver.c | 49 ++++++-- src/libxl/libxl_migration.c | 280 +++++++++++++++++++++++++++++++++++++++++--- src/libxl/libxl_migration.h | 9 ++ 3 files changed, 313 insertions(+), 25 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 7bc8adf..b34f120 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -5938,14 +5938,14 @@ libxlDomainMigratePrepareCommon(virConnectPtr dconn, char **cookieout ATTRIBUTE_UNUSED, int *cookieoutlen ATTRIBUTE_UNUSED, unsigned int flags, - void *data) + void *data, + bool is_tunnel) { libxlDriverPrivatePtr driver = dconn->privateData; virDomainDefPtr def = NULL; const char *dom_xml = NULL; const char *dname = NULL; const char *uri_in = NULL; - char **uri_out = data; #ifdef LIBXL_HAVE_NO_SUSPEND_RESUME virReportUnsupportedError(); @@ -5971,12 +5971,25 @@ libxlDomainMigratePrepareCommon(virConnectPtr dconn, if (!(def = libxlDomainMigrationPrepareDef(driver, dom_xml, dname))) goto error; - if (virDomainMigratePrepare3ParamsEnsureACL(dconn, def) < 0) - goto error; + if (is_tunnel) { + virStreamPtr st = data; - if (libxlDomainMigrationPrepare(dconn, &def, uri_in, uri_out, - cookiein, cookieinlen, flags) < 0) - goto error; + if (virDomainMigratePrepareTunnel3ParamsEnsureACL(dconn, def) < 0) + goto error; + + if (libxlDomainMigrationPrepareTunnel3(dconn, st, &def, cookiein, + cookieinlen, flags) < 0) + goto error; + } else { + char **uri_out = data; + + if (virDomainMigratePrepare3ParamsEnsureACL(dconn, def) < 0) + goto error; + + if (libxlDomainMigrationPrepare(dconn, &def, uri_in, uri_out, + cookiein, cookieinlen, flags) < 0) + goto error; + } return 0; @@ -5999,7 +6012,24 @@ libxlDomainMigratePrepare3Params(virConnectPtr dconn, return libxlDomainMigratePrepareCommon(dconn, params, nparams, cookiein, cookieinlen, cookieout, cookieoutlen, - flags, uri_out); + flags, uri_out, false); +} + +static int +libxlDomainMigratePrepareTunnel3Params(virConnectPtr dconn, + virStreamPtr st, + virTypedParameterPtr params, + int nparams, + const char *cookiein, + int cookieinlen, + char **cookieout ATTRIBUTE_UNUSED, + int *cookieoutlen ATTRIBUTE_UNUSED, + unsigned int flags) +{ + return libxlDomainMigratePrepareCommon(dconn, params, nparams, + cookiein, cookieinlen, + cookieout, cookieoutlen, + flags, st, true); } static int @@ -6047,7 +6077,7 @@ libxlDomainMigratePerform3Params(virDomainPtr dom, if (virDomainMigratePerform3ParamsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; - if (flags & VIR_MIGRATE_PEER2PEER) { + if ((flags & (VIR_MIGRATE_TUNNELLED | VIR_MIGRATE_PEER2PEER))) { if (libxlDomainMigrationPerformP2P(driver, vm, dom->conn, dom_xml, dconnuri, uri, dname, flags) < 0) goto cleanup; @@ -6532,6 +6562,7 @@ static virHypervisorDriver libxlHypervisorDriver = { .nodeDeviceReset = libxlNodeDeviceReset, /* 1.2.3 */ .domainMigrateBegin3Params = libxlDomainMigrateBegin3Params, /* 1.2.6 */ .domainMigratePrepare3Params = libxlDomainMigratePrepare3Params, /* 1.2.6 */ + .domainMigratePrepareTunnel3Params = libxlDomainMigratePrepareTunnel3Params, /* 3.1.0 */ .domainMigratePerform3Params = libxlDomainMigratePerform3Params, /* 1.2.6 */ .domainMigrateFinish3Params = libxlDomainMigrateFinish3Params, /* 1.2.6 */ .domainMigrateConfirm3Params = libxlDomainMigrateConfirm3Params, /* 1.2.6 */ diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 7beabd2..3aa0797 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -44,6 +44,7 @@ #include "libxl_migration.h" #include "locking/domain_lock.h" #include "virtypedparam.h" +#include "fdstream.h" #define VIR_FROM_THIS VIR_FROM_LIBXL @@ -554,6 +555,95 @@ libxlDomainMigrationPrepareAny(virConnectPtr dconn, } int +libxlDomainMigrationPrepareTunnel3(virConnectPtr dconn, + virStreamPtr st, + virDomainDefPtr *def, + const char *cookiein, + int cookieinlen, + unsigned int flags) +{ + libxlMigrationCookiePtr mig = NULL; + libxlDriverPrivatePtr driver = dconn->privateData; + virDomainObjPtr vm = NULL; + libxlMigrationDstArgs *args = NULL; + virThread thread; + bool taint_hook = false; + libxlDomainObjPrivatePtr priv = NULL; + char *xmlout = NULL; + int dataFD[2] = { -1, -1 }; + int ret = -1; + + if (libxlDomainMigrationPrepareAny(dconn, def, cookiein, cookieinlen, + &mig, &xmlout, &taint_hook) < 0) + goto error; + + if (!(vm = virDomainObjListAdd(driver->domains, *def, + driver->xmlopt, + VIR_DOMAIN_OBJ_LIST_ADD_LIVE | + VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, + NULL))) + goto error; + + *def = NULL; + priv = vm->privateData; + + if (taint_hook) { + /* Domain XML has been altered by a hook script. */ + priv->hookRun = true; + } + + /* + * The data flow of tunnel3 migration in the dest side: + * stream -> pipe -> recvfd of libxlDomainStartRestore + */ + if (pipe(dataFD) < 0) + goto error; + + /* Stream data will be written to pipeIn */ + if (virFDStreamOpen(st, dataFD[1]) < 0) + goto error; + dataFD[1] = -1; /* 'st' owns the FD now & will close it */ + + if (libxlMigrationDstArgsInitialize() < 0) + goto error; + + if (!(args = virObjectNew(libxlMigrationDstArgsClass))) + goto error; + + args->conn = dconn; + args->vm = vm; + args->flags = flags; + args->migcookie = mig; + /* Receive from pipeOut */ + args->recvfd = dataFD[0]; + args->nsocks = 0; + if (virThreadCreate(&thread, false, libxlDoMigrateReceive, args) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("Failed to create thread for receiving migration data")); + goto error; + } + + ret = 0; + goto done; + + error: + VIR_FORCE_CLOSE(dataFD[1]); + VIR_FORCE_CLOSE(dataFD[0]); + virObjectUnref(args); + /* Remove virDomainObj from domain list */ + if (vm) { + virDomainObjListRemove(driver->domains, vm); + vm = NULL; + } + + done: + if (vm) + virObjectUnlock(vm); + + return ret; +} + +int libxlDomainMigrationPrepare(virConnectPtr dconn, virDomainDefPtr *def, const char *uri_in, @@ -734,9 +824,148 @@ 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. +typedef struct _libxlTunnelMigrationThread libxlTunnelMigrationThread; +struct _libxlTunnelMigrationThread { + virStreamPtr st; + int srcFD; +}; +#define TUNNEL_SEND_BUF_SIZE 65536 + +/* + * The data flow of tunnel3 migration in the src side: + * libxlDoMigrateSend() -> pipe + * libxlTunnel3MigrationFunc() polls pipe out and then write to dest stream. + */ +static void libxlTunnel3MigrationFunc(void *arg) +{ + libxlTunnelMigrationThread *data = (libxlTunnelMigrationThread *)arg; + char *buffer = NULL; + struct pollfd fds[1]; + int timeout = -1; + + if (VIR_ALLOC_N(buffer, TUNNEL_SEND_BUF_SIZE) < 0) { + virReportError(errno, "%s", _("poll failed in migration tunnel")); + return; + } + + fds[0].fd = data->srcFD; + for (;;) { + int ret; + + fds[0].events = POLLIN; + fds[0].revents = 0; + ret = poll(fds, ARRAY_CARDINALITY(fds), timeout); + if (ret < 0) { + if (errno == EAGAIN || errno == EINTR) + continue; + virReportError(errno, "%s", + _("poll failed in libxlTunnel3MigrationFunc")); + goto cleanup; + } + + if (ret == 0) { + VIR_DEBUG("poll got timeout"); + break; + } + + if (fds[0].revents & (POLLIN | POLLERR | POLLHUP)) { + int nbytes; + + nbytes = read(data->srcFD, buffer, TUNNEL_SEND_BUF_SIZE); + if (nbytes > 0) { + /* Write to dest stream */ + if (virStreamSend(data->st, buffer, nbytes) < 0) { + virReportError(errno, "%s", + _("tunnelled migration failed to send to virStream")); + virStreamAbort(data->st); + goto cleanup; + } + } else if (nbytes < 0) { + virReportError(errno, "%s", + _("tunnelled migration failed to read from xen side")); + virStreamAbort(data->st); + goto cleanup; + } else { + /* EOF; transferred all data */ + break; + } + } + } + + if (virStreamFinish(data->st) < 0) + virReportError(errno, "%s", + _("tunnelled migration failed to finish stream")); + cleanup: + VIR_FREE(buffer); + + return; +} + +struct libxlTunnelControl { + libxlTunnelMigrationThread tmThread; + virThread thread; + int dataFD[2]; +}; + +static int +libxlMigrationStartTunnel(libxlDriverPrivatePtr driver, + virDomainObjPtr vm, + unsigned long flags, + virStreamPtr st, + struct libxlTunnelControl *tc) +{ + libxlTunnelMigrationThread *arg = NULL; + int ret = -1; + + if (VIR_ALLOC(tc) < 0) + goto out; + + tc->dataFD[0] = -1; + tc->dataFD[1] = -1; + if (pipe(tc->dataFD) < 0) { + virReportError(errno, "%s", _("Unable to make pipes")); + goto out; + } + + arg = &tc->tmThread; + /* Read from pipe */ + arg->srcFD = tc->dataFD[0]; + /* Write to dest stream */ + arg->st = st; + if (virThreadCreate(&tc->thread, true, + libxlTunnel3MigrationFunc, arg) < 0) { + virReportError(errno, "%s", + _("Unable to create tunnel migration thread")); + goto out; + } + + virObjectUnlock(vm); + /* Send data to pipe */ + ret = libxlDoMigrateSend(driver, vm, flags, tc->dataFD[1]); + virObjectLock(vm); + + out: + /* libxlMigrationStopTunnel will be called in libxlDoMigrateP2P to free + * all resources for us. */ + return ret; +} + +static void libxlMigrationStopTunnel(struct libxlTunnelControl *tc) +{ + if (!tc) + return; + + virThreadCancel(&tc->thread); + virThreadJoin(&tc->thread); + + VIR_FORCE_CLOSE(tc->dataFD[0]); + VIR_FORCE_CLOSE(tc->dataFD[1]); + VIR_FREE(tc); +} + +/* This function is a simplification of virDomainMigrateVersion3Full and + * restricting it to migration v3 with params since it was the first to be + * introduced in libxl. */ static int libxlDoMigrateP2P(libxlDriverPrivatePtr driver, @@ -761,6 +990,9 @@ libxlDoMigrateP2P(libxlDriverPrivatePtr driver, bool cancelled = true; virErrorPtr orig_err = NULL; int ret = -1; + /* For tunnel migration */ + virStreamPtr st = NULL; + struct libxlTunnelControl *tc = NULL; dom_xml = libxlDomainMigrationBegin(sconn, vm, xmlin, &cookieout, &cookieoutlen); @@ -788,29 +1020,40 @@ libxlDoMigrateP2P(libxlDriverPrivatePtr driver, VIR_DEBUG("Prepare3"); virObjectUnlock(vm); - ret = dconn->driver->domainMigratePrepare3Params - (dconn, params, nparams, cookieout, cookieoutlen, NULL, NULL, &uri_out, destflags); + if (flags & VIR_MIGRATE_TUNNELLED) { + if (!(st = virStreamNew(dconn, 0))) + goto cleanup; + ret = dconn->driver->domainMigratePrepareTunnel3Params + (dconn, st, params, nparams, cookieout, cookieoutlen, NULL, NULL, destflags); + } else { + ret = dconn->driver->domainMigratePrepare3Params + (dconn, params, nparams, cookieout, cookieoutlen, NULL, NULL, &uri_out, destflags); + } virObjectLock(vm); if (ret == -1) goto cleanup; - if (uri_out) { - if (virTypedParamsReplaceString(¶ms, &nparams, - VIR_MIGRATE_PARAM_URI, uri_out) < 0) { - orig_err = virSaveLastError(); + if (!(flags & VIR_MIGRATE_TUNNELLED)) { + if (uri_out) { + if (virTypedParamsReplaceString(¶ms, &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; } - } 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 (flags & VIR_MIGRATE_TUNNELLED) + ret = libxlMigrationStartTunnel(driver, vm, flags, st, tc); + else + ret = libxlDomainMigrationPerform(driver, vm, NULL, NULL, + uri_out, NULL, flags); if (ret < 0) orig_err = virSaveLastError(); @@ -848,6 +1091,11 @@ libxlDoMigrateP2P(libxlDriverPrivatePtr driver, vm->def->name); cleanup: + if (flags & VIR_MIGRATE_TUNNELLED) { + libxlMigrationStopTunnel(tc); + virObjectUnref(st); + } + if (ddomain) { virObjectUnref(ddomain); ret = 0; diff --git a/src/libxl/libxl_migration.h b/src/libxl/libxl_migration.h index 8a074a0..fcea558 100644 --- a/src/libxl/libxl_migration.h +++ b/src/libxl/libxl_migration.h @@ -29,6 +29,7 @@ # define LIBXL_MIGRATION_FLAGS \ (VIR_MIGRATE_LIVE | \ VIR_MIGRATE_PEER2PEER | \ + VIR_MIGRATE_TUNNELLED | \ VIR_MIGRATE_PERSIST_DEST | \ VIR_MIGRATE_UNDEFINE_SOURCE | \ VIR_MIGRATE_PAUSED) @@ -53,6 +54,14 @@ libxlDomainMigrationPrepareDef(libxlDriverPrivatePtr driver, const char *dname); int +libxlDomainMigrationPrepareTunnel3(virConnectPtr dconn, + virStreamPtr st, + virDomainDefPtr *def, + const char *cookiein, + int cookieinlen, + unsigned int flags); + +int libxlDomainMigrationPrepare(virConnectPtr dconn, virDomainDefPtr *def, const char *uri_in, -- 2.1.4

On 02/07/2017 05:35 PM, Joao Martins wrote:
From: Bob Liu <bob.liu@oracle.com>
Tunnelled migration doesn't require any extra network connections beside the libvirt daemon. It's capable of strong encryption and the default option of openstack-nova.
This patch adds the tunnelled migration(Tunnel3params) support to libxl. On the source side, the data flow is:
* libxlDoMigrateSend() -> pipe libxlTunnel3MigrationFunc() polls pipe * out and then write to dest stream.
While on the destination side: * Stream -> pipe -> 'recvfd of libxlDomainStartRestore'
The usage is the same as p2p migration, execpt adding one extra '--tunnelled' to the libvirt p2p migration command.
Signed-off-by: Bob Liu <bob.liu@oracle.com> Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- v2 (comments from Jim): * Adjust common preparetunnel function reflecting v1 suggestions - Using common top-level Prepare function by adding is_tunnel variable - Using libxl_migration PrepareAny added in patch 1 - virHook support in PrepareTunnel3 * Move virThreadPtr from libxlTunnelMigrationThread into libxlTunnelControl * Rename function local variable from tmThreadPtr to arg * Remove redundant assignment "tmThreadPtr = &tc->tmThread;" always being not null. --- src/libxl/libxl_driver.c | 49 ++++++-- src/libxl/libxl_migration.c | 280 +++++++++++++++++++++++++++++++++++++++++--- src/libxl/libxl_migration.h | 9 ++ 3 files changed, 313 insertions(+), 25 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 7bc8adf..b34f120 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -5938,14 +5938,14 @@ libxlDomainMigratePrepareCommon(virConnectPtr dconn, char **cookieout ATTRIBUTE_UNUSED, int *cookieoutlen ATTRIBUTE_UNUSED, unsigned int flags, - void *data) + void *data, + bool is_tunnel) { libxlDriverPrivatePtr driver = dconn->privateData; virDomainDefPtr def = NULL; const char *dom_xml = NULL; const char *dname = NULL; const char *uri_in = NULL; - char **uri_out = data;
#ifdef LIBXL_HAVE_NO_SUSPEND_RESUME virReportUnsupportedError(); @@ -5971,12 +5971,25 @@ libxlDomainMigratePrepareCommon(virConnectPtr dconn, if (!(def = libxlDomainMigrationPrepareDef(driver, dom_xml, dname))) goto error;
- if (virDomainMigratePrepare3ParamsEnsureACL(dconn, def) < 0) - goto error; + if (is_tunnel) { + virStreamPtr st = data;
- if (libxlDomainMigrationPrepare(dconn, &def, uri_in, uri_out, - cookiein, cookieinlen, flags) < 0) - goto error; + if (virDomainMigratePrepareTunnel3ParamsEnsureACL(dconn, def) < 0) + goto error; + + if (libxlDomainMigrationPrepareTunnel3(dconn, st, &def, cookiein, + cookieinlen, flags) < 0) + goto error; + } else { + char **uri_out = data; + + if (virDomainMigratePrepare3ParamsEnsureACL(dconn, def) < 0) + goto error; + + if (libxlDomainMigrationPrepare(dconn, &def, uri_in, uri_out, + cookiein, cookieinlen, flags) < 0) + goto error; + }
Same comment here about the ACL checks and 'make check. With both patches applied ./libxl/libxl_driver.c:5981 Mismatch check 'virDomainMigratePrepareTunnel3ParamsEnsureACL' for function 'libxlDomainMigratePrepareCommon' ./libxl/libxl_driver.c:5990 Mismatch check 'virDomainMigratePrepare3ParamsEnsureACL' for function 'libxlDomainMigratePrepareCommon'
return 0;
@@ -5999,7 +6012,24 @@ libxlDomainMigratePrepare3Params(virConnectPtr dconn, return libxlDomainMigratePrepareCommon(dconn, params, nparams, cookiein, cookieinlen, cookieout, cookieoutlen, - flags, uri_out); + flags, uri_out, false); +} + +static int +libxlDomainMigratePrepareTunnel3Params(virConnectPtr dconn, + virStreamPtr st, + virTypedParameterPtr params, + int nparams, + const char *cookiein, + int cookieinlen, + char **cookieout ATTRIBUTE_UNUSED, + int *cookieoutlen ATTRIBUTE_UNUSED, + unsigned int flags) +{ + return libxlDomainMigratePrepareCommon(dconn, params, nparams, + cookiein, cookieinlen, + cookieout, cookieoutlen, + flags, st, true); }
static int @@ -6047,7 +6077,7 @@ libxlDomainMigratePerform3Params(virDomainPtr dom, if (virDomainMigratePerform3ParamsEnsureACL(dom->conn, vm->def) < 0) goto cleanup;
- if (flags & VIR_MIGRATE_PEER2PEER) { + if ((flags & (VIR_MIGRATE_TUNNELLED | VIR_MIGRATE_PEER2PEER))) { if (libxlDomainMigrationPerformP2P(driver, vm, dom->conn, dom_xml, dconnuri, uri, dname, flags) < 0) goto cleanup; @@ -6532,6 +6562,7 @@ static virHypervisorDriver libxlHypervisorDriver = { .nodeDeviceReset = libxlNodeDeviceReset, /* 1.2.3 */ .domainMigrateBegin3Params = libxlDomainMigrateBegin3Params, /* 1.2.6 */ .domainMigratePrepare3Params = libxlDomainMigratePrepare3Params, /* 1.2.6 */ + .domainMigratePrepareTunnel3Params = libxlDomainMigratePrepareTunnel3Params, /* 3.1.0 */ .domainMigratePerform3Params = libxlDomainMigratePerform3Params, /* 1.2.6 */ .domainMigrateFinish3Params = libxlDomainMigrateFinish3Params, /* 1.2.6 */ .domainMigrateConfirm3Params = libxlDomainMigrateConfirm3Params, /* 1.2.6 */ diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 7beabd2..3aa0797 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -44,6 +44,7 @@ #include "libxl_migration.h" #include "locking/domain_lock.h" #include "virtypedparam.h" +#include "fdstream.h"
#define VIR_FROM_THIS VIR_FROM_LIBXL
@@ -554,6 +555,95 @@ libxlDomainMigrationPrepareAny(virConnectPtr dconn, }
int +libxlDomainMigrationPrepareTunnel3(virConnectPtr dconn, + virStreamPtr st, + virDomainDefPtr *def, + const char *cookiein, + int cookieinlen, + unsigned int flags) +{ + libxlMigrationCookiePtr mig = NULL; + libxlDriverPrivatePtr driver = dconn->privateData; + virDomainObjPtr vm = NULL; + libxlMigrationDstArgs *args = NULL; + virThread thread; + bool taint_hook = false; + libxlDomainObjPrivatePtr priv = NULL; + char *xmlout = NULL; + int dataFD[2] = { -1, -1 }; + int ret = -1; + + if (libxlDomainMigrationPrepareAny(dconn, def, cookiein, cookieinlen, + &mig, &xmlout, &taint_hook) < 0) + goto error; + + if (!(vm = virDomainObjListAdd(driver->domains, *def, + driver->xmlopt, + VIR_DOMAIN_OBJ_LIST_ADD_LIVE | + VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, + NULL))) + goto error; + + *def = NULL; + priv = vm->privateData; + + if (taint_hook) { + /* Domain XML has been altered by a hook script. */ + priv->hookRun = true; + } + + /* + * The data flow of tunnel3 migration in the dest side: + * stream -> pipe -> recvfd of libxlDomainStartRestore + */ + if (pipe(dataFD) < 0) + goto error; + + /* Stream data will be written to pipeIn */ + if (virFDStreamOpen(st, dataFD[1]) < 0) + goto error; + dataFD[1] = -1; /* 'st' owns the FD now & will close it */ + + if (libxlMigrationDstArgsInitialize() < 0) + goto error; + + if (!(args = virObjectNew(libxlMigrationDstArgsClass))) + goto error; + + args->conn = dconn; + args->vm = vm; + args->flags = flags; + args->migcookie = mig; + /* Receive from pipeOut */ + args->recvfd = dataFD[0]; + args->nsocks = 0; + if (virThreadCreate(&thread, false, libxlDoMigrateReceive, args) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("Failed to create thread for receiving migration data")); + goto error; + } + + ret = 0; + goto done; + + error: + VIR_FORCE_CLOSE(dataFD[1]); + VIR_FORCE_CLOSE(dataFD[0]); + virObjectUnref(args); + /* Remove virDomainObj from domain list */ + if (vm) { + virDomainObjListRemove(driver->domains, vm); + vm = NULL; + } + + done: + if (vm) + virObjectUnlock(vm); + + return ret; +} + +int libxlDomainMigrationPrepare(virConnectPtr dconn, virDomainDefPtr *def, const char *uri_in, @@ -734,9 +824,148 @@ 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. +typedef struct _libxlTunnelMigrationThread libxlTunnelMigrationThread; +struct _libxlTunnelMigrationThread { + virStreamPtr st; + int srcFD; +}; +#define TUNNEL_SEND_BUF_SIZE 65536 + +/* + * The data flow of tunnel3 migration in the src side: + * libxlDoMigrateSend() -> pipe + * libxlTunnel3MigrationFunc() polls pipe out and then write to dest stream. + */ +static void libxlTunnel3MigrationFunc(void *arg) +{ + libxlTunnelMigrationThread *data = (libxlTunnelMigrationThread *)arg; + char *buffer = NULL; + struct pollfd fds[1]; + int timeout = -1; + + if (VIR_ALLOC_N(buffer, TUNNEL_SEND_BUF_SIZE) < 0) { + virReportError(errno, "%s", _("poll failed in migration tunnel"));
Error reporting is handled by VIR_ALLOC_N.
+ return; + } + + fds[0].fd = data->srcFD; + for (;;) { + int ret; + + fds[0].events = POLLIN; + fds[0].revents = 0; + ret = poll(fds, ARRAY_CARDINALITY(fds), timeout); + if (ret < 0) { + if (errno == EAGAIN || errno == EINTR) + continue; + virReportError(errno, "%s", + _("poll failed in libxlTunnel3MigrationFunc")); + goto cleanup; + } + + if (ret == 0) { + VIR_DEBUG("poll got timeout"); + break; + } + + if (fds[0].revents & (POLLIN | POLLERR | POLLHUP)) { + int nbytes; + + nbytes = read(data->srcFD, buffer, TUNNEL_SEND_BUF_SIZE); + if (nbytes > 0) { + /* Write to dest stream */ + if (virStreamSend(data->st, buffer, nbytes) < 0) { + virReportError(errno, "%s", + _("tunnelled migration failed to send to virStream"));
virStreamSend also reports (a more helpful?) error.
+ virStreamAbort(data->st); + goto cleanup; + } + } else if (nbytes < 0) { + virReportError(errno, "%s", + _("tunnelled migration failed to read from xen side")); + virStreamAbort(data->st); + goto cleanup; + } else { + /* EOF; transferred all data */ + break; + } + } + } + + if (virStreamFinish(data->st) < 0) + virReportError(errno, "%s", + _("tunnelled migration failed to finish stream"));
AFAICT virStreamFinish also reports error. Besides the nits, looks good. I'll find some time tomorrow to test the patches. Thanks! Regards, Jim
+ cleanup: + VIR_FREE(buffer); + + return; +} + +struct libxlTunnelControl { + libxlTunnelMigrationThread tmThread; + virThread thread; + int dataFD[2]; +}; + +static int +libxlMigrationStartTunnel(libxlDriverPrivatePtr driver, + virDomainObjPtr vm, + unsigned long flags, + virStreamPtr st, + struct libxlTunnelControl *tc) +{ + libxlTunnelMigrationThread *arg = NULL; + int ret = -1; + + if (VIR_ALLOC(tc) < 0) + goto out; + + tc->dataFD[0] = -1; + tc->dataFD[1] = -1; + if (pipe(tc->dataFD) < 0) { + virReportError(errno, "%s", _("Unable to make pipes")); + goto out; + } + + arg = &tc->tmThread; + /* Read from pipe */ + arg->srcFD = tc->dataFD[0]; + /* Write to dest stream */ + arg->st = st; + if (virThreadCreate(&tc->thread, true, + libxlTunnel3MigrationFunc, arg) < 0) { + virReportError(errno, "%s", + _("Unable to create tunnel migration thread")); + goto out; + } + + virObjectUnlock(vm); + /* Send data to pipe */ + ret = libxlDoMigrateSend(driver, vm, flags, tc->dataFD[1]); + virObjectLock(vm); + + out: + /* libxlMigrationStopTunnel will be called in libxlDoMigrateP2P to free + * all resources for us. */ + return ret; +} + +static void libxlMigrationStopTunnel(struct libxlTunnelControl *tc) +{ + if (!tc) + return; + + virThreadCancel(&tc->thread); + virThreadJoin(&tc->thread); + + VIR_FORCE_CLOSE(tc->dataFD[0]); + VIR_FORCE_CLOSE(tc->dataFD[1]); + VIR_FREE(tc); +} + +/* This function is a simplification of virDomainMigrateVersion3Full and + * restricting it to migration v3 with params since it was the first to be + * introduced in libxl. */ static int libxlDoMigrateP2P(libxlDriverPrivatePtr driver, @@ -761,6 +990,9 @@ libxlDoMigrateP2P(libxlDriverPrivatePtr driver, bool cancelled = true; virErrorPtr orig_err = NULL; int ret = -1; + /* For tunnel migration */ + virStreamPtr st = NULL; + struct libxlTunnelControl *tc = NULL;
dom_xml = libxlDomainMigrationBegin(sconn, vm, xmlin, &cookieout, &cookieoutlen); @@ -788,29 +1020,40 @@ libxlDoMigrateP2P(libxlDriverPrivatePtr driver,
VIR_DEBUG("Prepare3"); virObjectUnlock(vm); - ret = dconn->driver->domainMigratePrepare3Params - (dconn, params, nparams, cookieout, cookieoutlen, NULL, NULL, &uri_out, destflags); + if (flags & VIR_MIGRATE_TUNNELLED) { + if (!(st = virStreamNew(dconn, 0))) + goto cleanup; + ret = dconn->driver->domainMigratePrepareTunnel3Params + (dconn, st, params, nparams, cookieout, cookieoutlen, NULL, NULL, destflags); + } else { + ret = dconn->driver->domainMigratePrepare3Params + (dconn, params, nparams, cookieout, cookieoutlen, NULL, NULL, &uri_out, destflags); + } virObjectLock(vm);
if (ret == -1) goto cleanup;
- if (uri_out) { - if (virTypedParamsReplaceString(¶ms, &nparams, - VIR_MIGRATE_PARAM_URI, uri_out) < 0) { - orig_err = virSaveLastError(); + if (!(flags & VIR_MIGRATE_TUNNELLED)) { + if (uri_out) { + if (virTypedParamsReplaceString(¶ms, &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; } - } 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 (flags & VIR_MIGRATE_TUNNELLED) + ret = libxlMigrationStartTunnel(driver, vm, flags, st, tc); + else + ret = libxlDomainMigrationPerform(driver, vm, NULL, NULL, + uri_out, NULL, flags); if (ret < 0) orig_err = virSaveLastError();
@@ -848,6 +1091,11 @@ libxlDoMigrateP2P(libxlDriverPrivatePtr driver, vm->def->name);
cleanup: + if (flags & VIR_MIGRATE_TUNNELLED) { + libxlMigrationStopTunnel(tc); + virObjectUnref(st); + } + if (ddomain) { virObjectUnref(ddomain); ret = 0; diff --git a/src/libxl/libxl_migration.h b/src/libxl/libxl_migration.h index 8a074a0..fcea558 100644 --- a/src/libxl/libxl_migration.h +++ b/src/libxl/libxl_migration.h @@ -29,6 +29,7 @@ # define LIBXL_MIGRATION_FLAGS \ (VIR_MIGRATE_LIVE | \ VIR_MIGRATE_PEER2PEER | \ + VIR_MIGRATE_TUNNELLED | \ VIR_MIGRATE_PERSIST_DEST | \ VIR_MIGRATE_UNDEFINE_SOURCE | \ VIR_MIGRATE_PAUSED) @@ -53,6 +54,14 @@ libxlDomainMigrationPrepareDef(libxlDriverPrivatePtr driver, const char *dname);
int +libxlDomainMigrationPrepareTunnel3(virConnectPtr dconn, + virStreamPtr st, + virDomainDefPtr *def, + const char *cookiein, + int cookieinlen, + unsigned int flags); + +int libxlDomainMigrationPrepare(virConnectPtr dconn, virDomainDefPtr *def, const char *uri_in,

On 02/14/2017 03:34 AM, Jim Fehlig wrote:
On 02/07/2017 05:35 PM, Joao Martins wrote:
From: Bob Liu <bob.liu@oracle.com>
Tunnelled migration doesn't require any extra network connections beside the libvirt daemon. It's capable of strong encryption and the default option of openstack-nova.
This patch adds the tunnelled migration(Tunnel3params) support to libxl. On the source side, the data flow is:
* libxlDoMigrateSend() -> pipe libxlTunnel3MigrationFunc() polls pipe * out and then write to dest stream.
While on the destination side: * Stream -> pipe -> 'recvfd of libxlDomainStartRestore'
The usage is the same as p2p migration, execpt adding one extra '--tunnelled' to the libvirt p2p migration command.
Signed-off-by: Bob Liu <bob.liu@oracle.com> Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- v2 (comments from Jim): * Adjust common preparetunnel function reflecting v1 suggestions - Using common top-level Prepare function by adding is_tunnel variable - Using libxl_migration PrepareAny added in patch 1 - virHook support in PrepareTunnel3 * Move virThreadPtr from libxlTunnelMigrationThread into libxlTunnelControl * Rename function local variable from tmThreadPtr to arg * Remove redundant assignment "tmThreadPtr = &tc->tmThread;" always being not null. --- src/libxl/libxl_driver.c | 49 ++++++-- src/libxl/libxl_migration.c | 280 +++++++++++++++++++++++++++++++++++++++++--- src/libxl/libxl_migration.h | 9 ++ 3 files changed, 313 insertions(+), 25 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 7bc8adf..b34f120 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -5938,14 +5938,14 @@ libxlDomainMigratePrepareCommon(virConnectPtr dconn, char **cookieout ATTRIBUTE_UNUSED, int *cookieoutlen ATTRIBUTE_UNUSED, unsigned int flags, - void *data) + void *data, + bool is_tunnel) { libxlDriverPrivatePtr driver = dconn->privateData; virDomainDefPtr def = NULL; const char *dom_xml = NULL; const char *dname = NULL; const char *uri_in = NULL; - char **uri_out = data;
#ifdef LIBXL_HAVE_NO_SUSPEND_RESUME virReportUnsupportedError(); @@ -5971,12 +5971,25 @@ libxlDomainMigratePrepareCommon(virConnectPtr dconn, if (!(def = libxlDomainMigrationPrepareDef(driver, dom_xml, dname))) goto error;
- if (virDomainMigratePrepare3ParamsEnsureACL(dconn, def) < 0) - goto error; + if (is_tunnel) { + virStreamPtr st = data;
- if (libxlDomainMigrationPrepare(dconn, &def, uri_in, uri_out, - cookiein, cookieinlen, flags) < 0) - goto error; + if (virDomainMigratePrepareTunnel3ParamsEnsureACL(dconn, def) < 0) + goto error; + + if (libxlDomainMigrationPrepareTunnel3(dconn, st, &def, cookiein, + cookieinlen, flags) < 0) + goto error; + } else { + char **uri_out = data; + + if (virDomainMigratePrepare3ParamsEnsureACL(dconn, def) < 0) + goto error; + + if (libxlDomainMigrationPrepare(dconn, &def, uri_in, uri_out, + cookiein, cookieinlen, flags) < 0) + goto error; + }
Same comment here about the ACL checks and 'make check. With both patches applied
./libxl/libxl_driver.c:5981 Mismatch check 'virDomainMigratePrepareTunnel3ParamsEnsureACL' for function 'libxlDomainMigratePrepareCommon' ./libxl/libxl_driver.c:5990 Mismatch check 'virDomainMigratePrepare3ParamsEnsureACL' for function 'libxlDomainMigratePrepareCommon' Yeap, see my comment in the previous patch.
return 0;
@@ -5999,7 +6012,24 @@ libxlDomainMigratePrepare3Params(virConnectPtr dconn, return libxlDomainMigratePrepareCommon(dconn, params, nparams, cookiein, cookieinlen, cookieout, cookieoutlen, - flags, uri_out); + flags, uri_out, false); +} + +static int +libxlDomainMigratePrepareTunnel3Params(virConnectPtr dconn, + virStreamPtr st, + virTypedParameterPtr params, + int nparams, + const char *cookiein, + int cookieinlen, + char **cookieout ATTRIBUTE_UNUSED, + int *cookieoutlen ATTRIBUTE_UNUSED, + unsigned int flags) +{ + return libxlDomainMigratePrepareCommon(dconn, params, nparams, + cookiein, cookieinlen, + cookieout, cookieoutlen, + flags, st, true); }
static int @@ -6047,7 +6077,7 @@ libxlDomainMigratePerform3Params(virDomainPtr dom, if (virDomainMigratePerform3ParamsEnsureACL(dom->conn, vm->def) < 0) goto cleanup;
- if (flags & VIR_MIGRATE_PEER2PEER) { + if ((flags & (VIR_MIGRATE_TUNNELLED | VIR_MIGRATE_PEER2PEER))) { if (libxlDomainMigrationPerformP2P(driver, vm, dom->conn, dom_xml, dconnuri, uri, dname, flags) < 0) goto cleanup; @@ -6532,6 +6562,7 @@ static virHypervisorDriver libxlHypervisorDriver = { .nodeDeviceReset = libxlNodeDeviceReset, /* 1.2.3 */ .domainMigrateBegin3Params = libxlDomainMigrateBegin3Params, /* 1.2.6 */ .domainMigratePrepare3Params = libxlDomainMigratePrepare3Params, /* 1.2.6 */ + .domainMigratePrepareTunnel3Params = libxlDomainMigratePrepareTunnel3Params, /* 3.1.0 */ .domainMigratePerform3Params = libxlDomainMigratePerform3Params, /* 1.2.6 */ .domainMigrateFinish3Params = libxlDomainMigrateFinish3Params, /* 1.2.6 */ .domainMigrateConfirm3Params = libxlDomainMigrateConfirm3Params, /* 1.2.6 */ diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 7beabd2..3aa0797 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -44,6 +44,7 @@ #include "libxl_migration.h" #include "locking/domain_lock.h" #include "virtypedparam.h" +#include "fdstream.h"
#define VIR_FROM_THIS VIR_FROM_LIBXL
@@ -554,6 +555,95 @@ libxlDomainMigrationPrepareAny(virConnectPtr dconn, }
int +libxlDomainMigrationPrepareTunnel3(virConnectPtr dconn, + virStreamPtr st, + virDomainDefPtr *def, + const char *cookiein, + int cookieinlen, + unsigned int flags) +{ + libxlMigrationCookiePtr mig = NULL; + libxlDriverPrivatePtr driver = dconn->privateData; + virDomainObjPtr vm = NULL; + libxlMigrationDstArgs *args = NULL; + virThread thread; + bool taint_hook = false; + libxlDomainObjPrivatePtr priv = NULL; + char *xmlout = NULL; + int dataFD[2] = { -1, -1 }; + int ret = -1; + + if (libxlDomainMigrationPrepareAny(dconn, def, cookiein, cookieinlen, + &mig, &xmlout, &taint_hook) < 0) + goto error; + + if (!(vm = virDomainObjListAdd(driver->domains, *def, + driver->xmlopt, + VIR_DOMAIN_OBJ_LIST_ADD_LIVE | + VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, + NULL))) + goto error; + + *def = NULL; + priv = vm->privateData; + + if (taint_hook) { + /* Domain XML has been altered by a hook script. */ + priv->hookRun = true; + } + + /* + * The data flow of tunnel3 migration in the dest side: + * stream -> pipe -> recvfd of libxlDomainStartRestore + */ + if (pipe(dataFD) < 0) + goto error; + + /* Stream data will be written to pipeIn */ + if (virFDStreamOpen(st, dataFD[1]) < 0) + goto error; + dataFD[1] = -1; /* 'st' owns the FD now & will close it */ + + if (libxlMigrationDstArgsInitialize() < 0) + goto error; + + if (!(args = virObjectNew(libxlMigrationDstArgsClass))) + goto error; + + args->conn = dconn; + args->vm = vm; + args->flags = flags; + args->migcookie = mig; + /* Receive from pipeOut */ + args->recvfd = dataFD[0]; + args->nsocks = 0; + if (virThreadCreate(&thread, false, libxlDoMigrateReceive, args) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("Failed to create thread for receiving migration data")); + goto error; + } + + ret = 0; + goto done; + + error: + VIR_FORCE_CLOSE(dataFD[1]); + VIR_FORCE_CLOSE(dataFD[0]); + virObjectUnref(args); + /* Remove virDomainObj from domain list */ + if (vm) { + virDomainObjListRemove(driver->domains, vm); + vm = NULL; + } + + done: + if (vm) + virObjectUnlock(vm); + + return ret; +} + +int libxlDomainMigrationPrepare(virConnectPtr dconn, virDomainDefPtr *def, const char *uri_in, @@ -734,9 +824,148 @@ 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. +typedef struct _libxlTunnelMigrationThread libxlTunnelMigrationThread; +struct _libxlTunnelMigrationThread { + virStreamPtr st; + int srcFD; +}; +#define TUNNEL_SEND_BUF_SIZE 65536 + +/* + * The data flow of tunnel3 migration in the src side: + * libxlDoMigrateSend() -> pipe + * libxlTunnel3MigrationFunc() polls pipe out and then write to dest stream. + */ +static void libxlTunnel3MigrationFunc(void *arg) +{ + libxlTunnelMigrationThread *data = (libxlTunnelMigrationThread *)arg; + char *buffer = NULL; + struct pollfd fds[1]; + int timeout = -1; + + if (VIR_ALLOC_N(buffer, TUNNEL_SEND_BUF_SIZE) < 0) { + virReportError(errno, "%s", _("poll failed in migration tunnel"));
Error reporting is handled by VIR_ALLOC_N. True, remnant from previous patch. I've remove it.
+ return; + } + + fds[0].fd = data->srcFD; + for (;;) { + int ret; + + fds[0].events = POLLIN; + fds[0].revents = 0; + ret = poll(fds, ARRAY_CARDINALITY(fds), timeout); + if (ret < 0) { + if (errno == EAGAIN || errno == EINTR) + continue; + virReportError(errno, "%s", + _("poll failed in libxlTunnel3MigrationFunc")); + goto cleanup; + } + + if (ret == 0) { + VIR_DEBUG("poll got timeout"); + break; + } + + if (fds[0].revents & (POLLIN | POLLERR | POLLHUP)) { + int nbytes; + + nbytes = read(data->srcFD, buffer, TUNNEL_SEND_BUF_SIZE); + if (nbytes > 0) { + /* Write to dest stream */ + if (virStreamSend(data->st, buffer, nbytes) < 0) { + virReportError(errno, "%s", + _("tunnelled migration failed to send to virStream"));
virStreamSend also reports (a more helpful?) error. Indeed :) I've removed this too.
+ virStreamAbort(data->st); + goto cleanup; + } + } else if (nbytes < 0) { + virReportError(errno, "%s", + _("tunnelled migration failed to read from xen side")); + virStreamAbort(data->st); + goto cleanup; + } else { + /* EOF; transferred all data */ + break; + } + } + } + + if (virStreamFinish(data->st) < 0) + virReportError(errno, "%s", + _("tunnelled migration failed to finish stream"));
AFAICT virStreamFinish also reports error. OK, removed.
Besides the nits, looks good. I'll find some time tomorrow to test the patches. Thanks! Cool, Thanks! I'll wait for the comment on patch 2 before respinning.
+ cleanup: + VIR_FREE(buffer); + + return; +} + +struct libxlTunnelControl { + libxlTunnelMigrationThread tmThread; + virThread thread; + int dataFD[2]; +}; + +static int +libxlMigrationStartTunnel(libxlDriverPrivatePtr driver, + virDomainObjPtr vm, + unsigned long flags, + virStreamPtr st, + struct libxlTunnelControl *tc) +{ + libxlTunnelMigrationThread *arg = NULL; + int ret = -1; + + if (VIR_ALLOC(tc) < 0) + goto out; + + tc->dataFD[0] = -1; + tc->dataFD[1] = -1; + if (pipe(tc->dataFD) < 0) { + virReportError(errno, "%s", _("Unable to make pipes")); + goto out; + } + + arg = &tc->tmThread; + /* Read from pipe */ + arg->srcFD = tc->dataFD[0]; + /* Write to dest stream */ + arg->st = st; + if (virThreadCreate(&tc->thread, true, + libxlTunnel3MigrationFunc, arg) < 0) { + virReportError(errno, "%s", + _("Unable to create tunnel migration thread")); + goto out; + } + + virObjectUnlock(vm); + /* Send data to pipe */ + ret = libxlDoMigrateSend(driver, vm, flags, tc->dataFD[1]); + virObjectLock(vm); + + out: + /* libxlMigrationStopTunnel will be called in libxlDoMigrateP2P to free + * all resources for us. */ + return ret; +} + +static void libxlMigrationStopTunnel(struct libxlTunnelControl *tc) +{ + if (!tc) + return; + + virThreadCancel(&tc->thread); + virThreadJoin(&tc->thread); + + VIR_FORCE_CLOSE(tc->dataFD[0]); + VIR_FORCE_CLOSE(tc->dataFD[1]); + VIR_FREE(tc); +} + +/* This function is a simplification of virDomainMigrateVersion3Full and + * restricting it to migration v3 with params since it was the first to be + * introduced in libxl. */ static int libxlDoMigrateP2P(libxlDriverPrivatePtr driver, @@ -761,6 +990,9 @@ libxlDoMigrateP2P(libxlDriverPrivatePtr driver, bool cancelled = true; virErrorPtr orig_err = NULL; int ret = -1; + /* For tunnel migration */ + virStreamPtr st = NULL; + struct libxlTunnelControl *tc = NULL;
dom_xml = libxlDomainMigrationBegin(sconn, vm, xmlin, &cookieout, &cookieoutlen); @@ -788,29 +1020,40 @@ libxlDoMigrateP2P(libxlDriverPrivatePtr driver,
VIR_DEBUG("Prepare3"); virObjectUnlock(vm); - ret = dconn->driver->domainMigratePrepare3Params - (dconn, params, nparams, cookieout, cookieoutlen, NULL, NULL, &uri_out, destflags); + if (flags & VIR_MIGRATE_TUNNELLED) { + if (!(st = virStreamNew(dconn, 0))) + goto cleanup; + ret = dconn->driver->domainMigratePrepareTunnel3Params + (dconn, st, params, nparams, cookieout, cookieoutlen, NULL, NULL, destflags); + } else { + ret = dconn->driver->domainMigratePrepare3Params + (dconn, params, nparams, cookieout, cookieoutlen, NULL, NULL, &uri_out, destflags); + } virObjectLock(vm);
if (ret == -1) goto cleanup;
- if (uri_out) { - if (virTypedParamsReplaceString(¶ms, &nparams, - VIR_MIGRATE_PARAM_URI, uri_out) < 0) { - orig_err = virSaveLastError(); + if (!(flags & VIR_MIGRATE_TUNNELLED)) { + if (uri_out) { + if (virTypedParamsReplaceString(¶ms, &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; } - } 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 (flags & VIR_MIGRATE_TUNNELLED) + ret = libxlMigrationStartTunnel(driver, vm, flags, st, tc); + else + ret = libxlDomainMigrationPerform(driver, vm, NULL, NULL, + uri_out, NULL, flags); if (ret < 0) orig_err = virSaveLastError();
@@ -848,6 +1091,11 @@ libxlDoMigrateP2P(libxlDriverPrivatePtr driver, vm->def->name);
cleanup: + if (flags & VIR_MIGRATE_TUNNELLED) { + libxlMigrationStopTunnel(tc); + virObjectUnref(st); + } + if (ddomain) { virObjectUnref(ddomain); ret = 0; diff --git a/src/libxl/libxl_migration.h b/src/libxl/libxl_migration.h index 8a074a0..fcea558 100644 --- a/src/libxl/libxl_migration.h +++ b/src/libxl/libxl_migration.h @@ -29,6 +29,7 @@ # define LIBXL_MIGRATION_FLAGS \ (VIR_MIGRATE_LIVE | \ VIR_MIGRATE_PEER2PEER | \ + VIR_MIGRATE_TUNNELLED | \ VIR_MIGRATE_PERSIST_DEST | \ VIR_MIGRATE_UNDEFINE_SOURCE | \ VIR_MIGRATE_PAUSED) @@ -53,6 +54,14 @@ libxlDomainMigrationPrepareDef(libxlDriverPrivatePtr driver, const char *dname);
int +libxlDomainMigrationPrepareTunnel3(virConnectPtr dconn, + virStreamPtr st, + virDomainDefPtr *def, + const char *cookiein, + int cookieinlen, + unsigned int flags); + +int libxlDomainMigrationPrepare(virConnectPtr dconn, virDomainDefPtr *def, const char *uri_in,

On 02/07/2017 05:35 PM, Joao Martins wrote:
This allows us to reuse a single function for both tunnelled and non-tunnelled variants.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- New in v2 --- src/libxl/libxl_driver.c | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 3a69720..7bc8adf 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -5930,21 +5930,22 @@ libxlDomainMigrateBegin3Params(virDomainPtr domain, }
static int -libxlDomainMigratePrepare3Params(virConnectPtr dconn, - virTypedParameterPtr params, - int nparams, - const char *cookiein, - int cookieinlen, - char **cookieout ATTRIBUTE_UNUSED, - int *cookieoutlen ATTRIBUTE_UNUSED, - char **uri_out, - unsigned int flags) +libxlDomainMigratePrepareCommon(virConnectPtr dconn, + virTypedParameterPtr params, + int nparams, + const char *cookiein, + int cookieinlen, + char **cookieout ATTRIBUTE_UNUSED, + int *cookieoutlen ATTRIBUTE_UNUSED, + unsigned int flags, + void *data) { libxlDriverPrivatePtr driver = dconn->privateData; virDomainDefPtr def = NULL; const char *dom_xml = NULL; const char *dname = NULL; const char *uri_in = NULL; + char **uri_out = data;
#ifdef LIBXL_HAVE_NO_SUSPEND_RESUME virReportUnsupportedError(); @@ -5985,6 +5986,23 @@ libxlDomainMigratePrepare3Params(virConnectPtr dconn, }
static int +libxlDomainMigratePrepare3Params(virConnectPtr dconn, + virTypedParameterPtr params, + int nparams, + const char *cookiein, + int cookieinlen, + char **cookieout ATTRIBUTE_UNUSED, + int *cookieoutlen ATTRIBUTE_UNUSED, + char **uri_out, + unsigned int flags) +{ + return libxlDomainMigratePrepareCommon(dconn, params, nparams, + cookiein, cookieinlen, + cookieout, cookieoutlen, + flags, uri_out); +}
It appears the ACL check must be done in libxlDomainMigratePrepare3Params to satisfy 'make check' ./libxl/libxl_driver.c:5978 Mismatch check 'virDomainMigratePrepare3ParamsEnsureACL' for function 'libxlDomainMigratePrepareCommon' Regards, Jim
+ +static int libxlDomainMigratePerform3Params(virDomainPtr dom, const char *dconnuri, virTypedParameterPtr params,

On 02/14/2017 03:13 AM, Jim Fehlig wrote:
On 02/07/2017 05:35 PM, Joao Martins wrote:
This allows us to reuse a single function for both tunnelled and non-tunnelled variants.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- New in v2 --- src/libxl/libxl_driver.c | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 3a69720..7bc8adf 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -5930,21 +5930,22 @@ libxlDomainMigrateBegin3Params(virDomainPtr domain, }
static int -libxlDomainMigratePrepare3Params(virConnectPtr dconn, - virTypedParameterPtr params, - int nparams, - const char *cookiein, - int cookieinlen, - char **cookieout ATTRIBUTE_UNUSED, - int *cookieoutlen ATTRIBUTE_UNUSED, - char **uri_out, - unsigned int flags) +libxlDomainMigratePrepareCommon(virConnectPtr dconn, + virTypedParameterPtr params, + int nparams, + const char *cookiein, + int cookieinlen, + char **cookieout ATTRIBUTE_UNUSED, + int *cookieoutlen ATTRIBUTE_UNUSED, + unsigned int flags, + void *data) { libxlDriverPrivatePtr driver = dconn->privateData; virDomainDefPtr def = NULL; const char *dom_xml = NULL; const char *dname = NULL; const char *uri_in = NULL; + char **uri_out = data;
#ifdef LIBXL_HAVE_NO_SUSPEND_RESUME virReportUnsupportedError(); @@ -5985,6 +5986,23 @@ libxlDomainMigratePrepare3Params(virConnectPtr dconn, }
static int +libxlDomainMigratePrepare3Params(virConnectPtr dconn, + virTypedParameterPtr params, + int nparams, + const char *cookiein, + int cookieinlen, + char **cookieout ATTRIBUTE_UNUSED, + int *cookieoutlen ATTRIBUTE_UNUSED, + char **uri_out, + unsigned int flags) +{ + return libxlDomainMigratePrepareCommon(dconn, params, nparams, + cookiein, cookieinlen, + cookieout, cookieoutlen, + flags, uri_out); +}
It appears the ACL check must be done in libxlDomainMigratePrepare3Params to satisfy 'make check'
./libxl/libxl_driver.c:5978 Mismatch check 'virDomainMigratePrepare3ParamsEnsureACL' for function 'libxlDomainMigratePrepareCommon'
Sorry, I naively ran solely the syntax checks when rebasing this series. The ACL checks require dconn and def, and consequently the call to fetch def: def = libxlDomainMigrationPrepareDef(driver, dom_xml, dname); Requires dom_xml and dname being fetched from params. Given all these dependencies having this common function for both prepare functions doesn't seem to be doing much, as the top-level functions would end up being very similar after these dependencies. Which makes me wonder if we should dropped this (i.e. remove this PrepareCommon function) instead and go like initial v1 (same comment would be applicable for the PrepareTunnel3Params in patch 3). What do you think? Joao

On 02/14/2017 04:44 AM, Joao Martins wrote:
On 02/14/2017 03:13 AM, Jim Fehlig wrote:
On 02/07/2017 05:35 PM, Joao Martins wrote:
This allows us to reuse a single function for both tunnelled and non-tunnelled variants.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- New in v2 --- src/libxl/libxl_driver.c | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 3a69720..7bc8adf 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -5930,21 +5930,22 @@ libxlDomainMigrateBegin3Params(virDomainPtr domain, }
static int -libxlDomainMigratePrepare3Params(virConnectPtr dconn, - virTypedParameterPtr params, - int nparams, - const char *cookiein, - int cookieinlen, - char **cookieout ATTRIBUTE_UNUSED, - int *cookieoutlen ATTRIBUTE_UNUSED, - char **uri_out, - unsigned int flags) +libxlDomainMigratePrepareCommon(virConnectPtr dconn, + virTypedParameterPtr params, + int nparams, + const char *cookiein, + int cookieinlen, + char **cookieout ATTRIBUTE_UNUSED, + int *cookieoutlen ATTRIBUTE_UNUSED, + unsigned int flags, + void *data) { libxlDriverPrivatePtr driver = dconn->privateData; virDomainDefPtr def = NULL; const char *dom_xml = NULL; const char *dname = NULL; const char *uri_in = NULL; + char **uri_out = data;
#ifdef LIBXL_HAVE_NO_SUSPEND_RESUME virReportUnsupportedError(); @@ -5985,6 +5986,23 @@ libxlDomainMigratePrepare3Params(virConnectPtr dconn, }
static int +libxlDomainMigratePrepare3Params(virConnectPtr dconn, + virTypedParameterPtr params, + int nparams, + const char *cookiein, + int cookieinlen, + char **cookieout ATTRIBUTE_UNUSED, + int *cookieoutlen ATTRIBUTE_UNUSED, + char **uri_out, + unsigned int flags) +{ + return libxlDomainMigratePrepareCommon(dconn, params, nparams, + cookiein, cookieinlen, + cookieout, cookieoutlen, + flags, uri_out); +}
It appears the ACL check must be done in libxlDomainMigratePrepare3Params to satisfy 'make check'
./libxl/libxl_driver.c:5978 Mismatch check 'virDomainMigratePrepare3ParamsEnsureACL' for function 'libxlDomainMigratePrepareCommon'
Sorry, I naively ran solely the syntax checks when rebasing this series.
The ACL checks require dconn and def, and consequently the call to fetch def:
def = libxlDomainMigrationPrepareDef(driver, dom_xml, dname);
Requires dom_xml and dname being fetched from params.
:-(
Given all these dependencies having this common function for both prepare functions doesn't seem to be doing much, as the top-level functions would end up being very similar after these dependencies. Which makes me wonder if we should dropped this (i.e. remove this PrepareCommon function) instead and go like initial v1 (same comment would be applicable for the PrepareTunnel3Params in patch 3). What do you think?
Agree. Drop this and go with v1 approach of separate functions. Regards, Jim

On 02/08/2017 12:35 AM, Joao Martins wrote:
Hey!
Presented herewith is take 2 from tunnelled migration addressing all previous comments. Changelog in individual patches (patch 1 and 2 are small refactorings suggested in v1) Despite being functional changes mostly I had a quick round of testing too. Argh, My apologies. It seems I chained reply all patches by mistake - I can respin if folks prefer.
Joao
participants (2)
-
Jim Fehlig
-
Joao Martins