[Xen-devel] [PATCH 4/5] libxl: fix job handling across migration phases on dst

[Jim Fehlig <jfehlig@xxxxxxxx>]

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 <jfehlig@xxxxxxxx>
---
 src/libxl/libxl_driver.c    |  7 ----
 src/libxl/libxl_migration.c | 65 +++++++++++++++++++++++--------------
 2 files changed, 40 insertions(+), 32 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 5a5e792957..73c2ff3546 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -6020,15 +6020,8 @@ libxlDomainMigrateFinish3Params(virConnectPtr dconn,
         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;
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 191973edeb..54b01a3169 100644
--- a/src/libxl/libxl_migration.c
+++ b/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(virConnectPtr dconn,
         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(virConnectPtr dconn,
      * 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(virConnectPtr dconn,
     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(virConnectPtr dconn,
         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(virConnectPtr dconn,
     /* 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(virConnectPtr dconn,
             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(virConnectPtr dconn,
         }
 
         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(virConnectPtr dconn,
                                  &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(virConnectPtr dconn,
     }
 
     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(virConnectPtr dconn,
             virDomainObjListRemove(driver->domains, vm);
     }
 
+    /* EndJob for corresponding BeginJob in prepare phase */
+    libxlDomainObjEndJob(driver, vm);
     virObjectEventStateQueue(driver->domainEventState, event);
     virObjectUnref(cfg);
     return dom;
-- 
2.18.0


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel