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

On 02/07/2017 05:35 PM, Joao Martins wrote: > From: Bob Liu <bob.liu(a)oracle.com&gt; > > 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(a)oracle.com&gt; > Signed-off-by: Joao Martins <joao.m.martins(a)oracle.com&gt; > --- > 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!

> + 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(&params, &nparams, > - VIR_MIGRATE_PARAM_URI, uri_out) < 0) { > - orig_err = virSaveLastError(); > + if (!(flags & VIR_MIGRATE_TUNNELLED)) { > + 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; > } > - } 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, >