commit 0149464afc7100f048a2468e40e84b6a50aeb3a3 Author: Jim Fehlig Date: Wed Aug 29 11:11:00 2018 -0600 libxl: fix job handling across migration phases on dst The libxlDomainMigrationDst* functions are a bit flawed in their handling of modify jobs. A job begins when the destination host begins receiving the incoming VM and ends after the VM is started. The finish phase contains another BeginJob/EndJob sequence. This patch changes the logic to begin a job for the incoming VM in the prepare phase and end the job in the finish phase. Signed-off-by: Jim Fehlig ACKed-by: Michal Privoznik Index: libvirt-4.7.0/src/libxl/libxl_driver.c =================================================================== --- libvirt-4.7.0.orig/src/libxl/libxl_driver.c +++ libvirt-4.7.0/src/libxl/libxl_driver.c @@ -6020,15 +6020,8 @@ libxlDomainMigrateFinish3Params(virConne return NULL; } - if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) { - virDomainObjEndAPI(&vm); - return NULL; - } - ret = libxlDomainMigrationDstFinish(dconn, vm, flags, cancelled); - libxlDomainObjEndJob(driver, vm); - virDomainObjEndAPI(&vm); return ret; Index: libvirt-4.7.0/src/libxl/libxl_migration.c =================================================================== --- libvirt-4.7.0.orig/src/libxl/libxl_migration.c +++ libvirt-4.7.0/src/libxl/libxl_migration.c @@ -266,9 +266,6 @@ libxlDoMigrateDstReceive(void *opaque) size_t i; virObjectRef(vm); - virObjectLock(vm); - if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) - goto cleanup; /* * Always start the domain paused. If needed, unpause in the @@ -288,10 +285,6 @@ libxlDoMigrateDstReceive(void *opaque) args->nsocks = 0; VIR_FORCE_CLOSE(recvfd); virObjectUnref(args); - - libxlDomainObjEndJob(driver, vm); - - cleanup: virDomainObjEndAPI(&vm); } @@ -583,6 +576,13 @@ libxlDomainMigrationDstPrepareTunnel3(vi goto error; *def = NULL; + /* + * Unless an error is encountered in this function, the job will + * be terminated in the finish phase. + */ + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) + goto error; + priv = vm->privateData; if (taint_hook) { @@ -595,18 +595,18 @@ libxlDomainMigrationDstPrepareTunnel3(vi * stream -> pipe -> recvfd of libxlDomainStartRestore */ if (pipe(dataFD) < 0) - goto error; + goto endjob; /* Stream data will be written to pipeIn */ if (virFDStreamOpen(st, dataFD[1]) < 0) - goto error; + goto endjob; dataFD[1] = -1; /* 'st' owns the FD now & will close it */ if (libxlMigrationDstArgsInitialize() < 0) - goto error; + goto endjob; if (!(args = virObjectNew(libxlMigrationDstArgsClass))) - goto error; + goto endjob; args->conn = virObjectRef(dconn); args->vm = virObjectRef(vm); @@ -620,12 +620,15 @@ libxlDomainMigrationDstPrepareTunnel3(vi if (virThreadCreate(&thread, false, libxlDoMigrateDstReceive, args) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Failed to create thread for receiving migration data")); - goto error; + goto endjob; } ret = 0; goto done; + endjob: + libxlDomainObjEndJob(driver, vm); + error: libxlMigrationCookieFree(mig); VIR_FORCE_CLOSE(dataFD[1]); @@ -679,6 +682,13 @@ libxlDomainMigrationDstPrepare(virConnec goto error; *def = NULL; + /* + * Unless an error is encountered in this function, the job will + * be terminated in the finish phase. + */ + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) + goto error; + priv = vm->privateData; if (taint_hook) { @@ -689,27 +699,27 @@ libxlDomainMigrationDstPrepare(virConnec /* Create socket connection to receive migration data */ if (!uri_in) { if ((hostname = virGetHostname()) == NULL) - goto error; + goto endjob; if (STRPREFIX(hostname, "localhost")) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("hostname on destination resolved to localhost," " but migration requires an FQDN")); - goto error; + goto endjob; } if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0) - goto error; + goto endjob; priv->migrationPort = port; if (virAsprintf(uri_out, "tcp://%s:%d", hostname, port) < 0) - goto error; + goto endjob; } else { if (!(STRPREFIX(uri_in, "tcp://"))) { /* not full URI, add prefix tcp:// */ char *tmp; if (virAsprintf(&tmp, "tcp://%s", uri_in) < 0) - goto error; + goto endjob; uri = virURIParse(tmp); VIR_FREE(tmp); } else { @@ -720,20 +730,20 @@ libxlDomainMigrationDstPrepare(virConnec virReportError(VIR_ERR_INVALID_ARG, _("unable to parse URI: %s"), uri_in); - goto error; + goto endjob; } if (uri->server == NULL) { virReportError(VIR_ERR_INVALID_ARG, _("missing host in migration URI: %s"), uri_in); - goto error; + goto endjob; } hostname = uri->server; if (uri->port == 0) { if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0) - goto error; + goto endjob; priv->migrationPort = port; } else { @@ -741,7 +751,7 @@ libxlDomainMigrationDstPrepare(virConnec } if (virAsprintf(uri_out, "tcp://%s:%d", hostname, port) < 0) - goto error; + goto endjob; } snprintf(portstr, sizeof(portstr), "%d", port); @@ -751,14 +761,14 @@ libxlDomainMigrationDstPrepare(virConnec &socks, &nsocks) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Fail to create socket for incoming migration")); - goto error; + goto endjob; } if (libxlMigrationDstArgsInitialize() < 0) - goto error; + goto endjob; if (!(args = virObjectNew(libxlMigrationDstArgsClass))) - goto error; + goto endjob; args->conn = virObjectRef(dconn); args->vm = virObjectRef(vm); @@ -786,11 +796,14 @@ libxlDomainMigrationDstPrepare(virConnec } if (!nsocks_listen) - goto error; + goto endjob; ret = 0; goto done; + endjob: + libxlDomainObjEndJob(driver, vm); + error: for (i = 0; i < nsocks; i++) { virNetSocketClose(socks[i]); @@ -1354,6 +1367,8 @@ libxlDomainMigrationDstFinish(virConnect virDomainObjListRemove(driver->domains, vm); } + /* EndJob for corresponding BeginJob in prepare phase */ + libxlDomainObjEndJob(driver, vm); virObjectEventStateQueue(driver->domainEventState, event); virObjectUnref(cfg); return dom;