On 02/15/2017 11:17 AM, 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> --- v3: * virStream{Send,Finish} and VIR_ALLOC_N already report errors, hence removing the virReportError calls from its error paths. * Revert top-level migration APIs to initial v1 approach. 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. * Adjust debug message to "poll returned 0" as opposed to "poll got timeout" as the latter might not always be the case. --- src/libxl/libxl_driver.c | 58 +++++++++- src/libxl/libxl_migration.c | 275 +++++++++++++++++++++++++++++++++++++++++--- src/libxl/libxl_migration.h | 9 ++ 3 files changed, 325 insertions(+), 17 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 74cb05a..e5b8f48 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -5934,6 +5934,61 @@ libxlDomainMigrateBegin3Params(virDomainPtr domain, }

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) +{ + libxlDriverPrivatePtr driver = dconn->privateData; + virDomainDefPtr def = NULL; + const char *dom_xml = NULL; + const char *dname = NULL; + const char *uri_in = NULL; + +#ifdef LIBXL_HAVE_NO_SUSPEND_RESUME + virReportUnsupportedError(); + return -1; +#endif + + virCheckFlags(LIBXL_MIGRATION_FLAGS, -1); + if (virTypedParamsValidate(params, nparams, LIBXL_MIGRATION_PARAMETERS) < 0) + goto error; + + if (virTypedParamsGetString(params, nparams, + VIR_MIGRATE_PARAM_DEST_XML, + &dom_xml) < 0 || + virTypedParamsGetString(params, nparams, + VIR_MIGRATE_PARAM_DEST_NAME, + &dname) < 0 || + virTypedParamsGetString(params, nparams, + VIR_MIGRATE_PARAM_URI, + &uri_in) < 0) + + goto error; + + if (!(def = libxlDomainMigrationPrepareDef(driver, dom_xml, dname))) + goto error; + + if (virDomainMigratePrepareTunnel3ParamsEnsureACL(dconn, def) < 0) + goto error; + + if (libxlDomainMigrationPrepareTunnel3(dconn, st, &def, cookiein, + cookieinlen, flags) < 0) + goto error; + + return 0; + + error: + virDomainDefFree(def); + return -1; +} + +static int libxlDomainMigratePrepare3Params(virConnectPtr dconn, virTypedParameterPtr params, int nparams, @@ -6033,7 +6088,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; @@ -6518,6 +6573,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..6bd8983 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,143 @@ 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) + 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 returned 0"); + 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) { + 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; This is here since v1 and went unnoticed (in make syntax-check too), but the indentation is off here. It should be 4 spaces to the left. But hopefully that could be adjusted upon commit if no more comments are brought up?

+ } else { + /* EOF; transferred all data */ + break; + } + } + } + + ignore_value(virStreamFinish(data->st)); + + 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 +985,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 +1015,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 +1086,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/16/2017 11:47 AM, John Ferlan wrote:

FYI: Couple of Coverity issues resulted from these patches.

...

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)

Coverity notes that '@mig' will be leaked in this case when "if (!cookiein || !cookieinlen) {" is true in libxlMigrationEatCookie and failures from libxlDomainMigrationPrepareAny don't free it.

/nods. This diff should do then? It covers both cases I think. diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index ba1ca5c..6a7d458 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -637,6 +637,7 @@ libxlDomainMigrationPrepareTunnel3(virConnectPtr dconn, } done: + libxlMigrationCookieFree(mig); if (vm) virObjectUnlock(vm);

...

+ 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; +}

[...]

...

+ +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"));

Coverity notes that failures would cause a leak for @tc (I assume here and of course if virThreadCreate fails. Perhaps the 'best' way to handle that would be to set tc = NULL after ThreadCreate success and just call libxlMigrationStopTunnel in "out:"...

But libxlMigrationStopTunnel is always called (hence should be freeing @tc), whether libxlMigrationStartTunnel failed or not. How about this? @@ -907,8 +908,9 @@ libxlMigrationStartTunnel(libxlDriverPrivatePtr driver, virDomainObjPtr vm, unsigned long flags, virStreamPtr st, - struct libxlTunnelControl *tc) + struct libxlTunnelControl **tnl) { + struct libxlTunnelControl *tc = *tnl; libxlTunnelMigrationThread *arg = NULL; int ret = -1; @@ -1045,7 +1047,7 @@ libxlDoMigrateP2P(libxlDriverPrivatePtr driver, VIR_DEBUG("Perform3 uri=%s", NULLSTR(uri_out)); if (flags & VIR_MIGRATE_TUNNELLED) - ret = libxlMigrationStartTunnel(driver, vm, flags, st, tc); + ret = libxlMigrationStartTunnel(driver, vm, flags, st, &tc); else ret = libxlDomainMigrationPerform(driver, vm, NULL, NULL, uri_out, NULL, flags);

On 02/16/2017 02:57 PM, John Ferlan wrote:

On 02/16/2017 09:26 AM, Joao Martins wrote:

...

On 02/16/2017 11:47 AM, John Ferlan wrote:

...

FYI: Couple of Coverity issues resulted from these patches.

...

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)

Coverity notes that '@mig' will be leaked in this case when "if (!cookiein || !cookieinlen) {" is true in libxlMigrationEatCookie and failures from libxlDomainMigrationPrepareAny don't free it.

/nods. This diff should do then? It covers both cases I think.

What about:

args->migcookie = mig;

and then successful virThreadCreate(&thread, false, libxlDoMigrateReceive, args) where 'args' is unref'd, but the cookie isn't free'd.

Certainly you wouldn't want to free the cookie and then let the thread use it! \facepalm - you're right.

So I think the libxlMigrationCookieFree needs to be done in the error: label and in the thread cleanup. Also after you assign @mig to migcookie, set 'mig = NULL'; Yeap, let me send that in a follow-up patch shortly. The thread cleanup you mean libxlDoMigrateReceive right? There I think we could simply move unref args to cleanup label (which would free the cookie). But probably should go into a separate patch as it seems to be an issue to all migration paths.

...

diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index ba1ca5c..6a7d458 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -637,6 +637,7 @@ libxlDomainMigrationPrepareTunnel3(virConnectPtr dconn, }

done: + libxlMigrationCookieFree(mig); if (vm) virObjectUnlock(vm);

...

...

+ 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; +}

[...]

...

+ +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"));

Coverity notes that failures would cause a leak for @tc (I assume here and of course if virThreadCreate fails. Perhaps the 'best' way to handle that would be to set tc = NULL after ThreadCreate success and just call libxlMigrationStopTunnel in "out:"...

But libxlMigrationStopTunnel is always called (hence should be freeing @tc), whether libxlMigrationStartTunnel failed or not. How about this?

@@ -907,8 +908,9 @@ libxlMigrationStartTunnel(libxlDriverPrivatePtr driver, virDomainObjPtr vm, unsigned long flags, virStreamPtr st, - struct libxlTunnelControl *tc) + struct libxlTunnelControl **tnl) { + struct libxlTunnelControl *tc = *tnl;

This doesn't do much... since @tc would be allocated below and *tnl wouldn't be updated... Thus, the following is better:

if (VIR_ALLOC(tc) < 0) goto out; *tnl = tc;

OK.

...

libxlTunnelMigrationThread *arg = NULL; int ret = -1;

@@ -1045,7 +1047,7 @@ libxlDoMigrateP2P(libxlDriverPrivatePtr driver,

VIR_DEBUG("Perform3 uri=%s", NULLSTR(uri_out)); if (flags & VIR_MIGRATE_TUNNELLED) - ret = libxlMigrationStartTunnel(driver, vm, flags, st, tc); + ret = libxlMigrationStartTunnel(driver, vm, flags, st, &tc); else ret = libxlDomainMigrationPerform(driver, vm, NULL, NULL, uri_out, NULL, flags);