Re: [libvirt] [PATCH v3 2/2] libxl: add tunnelled migration support

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.

> + 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:"...