From e39c66d3ce3e65170a1db1324eb1fb8e57d82ecb Mon Sep 17 00:00:00 2001
From: Jim Fehlig <jfehlig@suse.com>
Date: Tue, 28 Aug 2018 17:13:54 -0600
Subject: [PATCH] libxl: fix logic in P2P migration

libxlDoMigrateSrcP2P() performs all phases of the migration
protocol for peer-to-peer migration. Unfortunately the logic
was a bit flawed since it is possible to skip the confirm
phase after a successfull begin and prepare phase. Fix the
logic to always call the confirm phase after a successful begin
and perform. Skip the confirm phase if begin or perform fail.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/libxl/libxl_migration.c | 44 +++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 97f72d0390..e4f2895690 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -972,21 +972,13 @@ libxlDoMigrateSrcP2P(libxlDriverPrivatePtr driver,
     char *cookieout = NULL;
     int cookieoutlen;
     bool cancelled = true;
+    bool notify_source = true;
     virErrorPtr orig_err = NULL;
     int ret = -1;
     /* For tunnel migration */
     virStreamPtr st = NULL;
     struct libxlTunnelControl *tc = NULL;
 
-    dom_xml = libxlDomainMigrationSrcBegin(sconn, vm, xmlin,
-                                           &cookieout, &cookieoutlen);
-    if (!dom_xml)
-        goto cleanup;
-
-    if (virTypedParamsAddString(&params, &nparams, &maxparams,
-                                VIR_MIGRATE_PARAM_DEST_XML, dom_xml) < 0)
-        goto cleanup;
-
     if (dname &&
         virTypedParamsAddString(&params, &nparams, &maxparams,
                                 VIR_MIGRATE_PARAM_DEST_NAME, dname) < 0)
@@ -997,6 +989,19 @@ libxlDoMigrateSrcP2P(libxlDriverPrivatePtr driver,
                                 VIR_MIGRATE_PARAM_URI, uri) < 0)
         goto cleanup;
 
+    dom_xml = libxlDomainMigrationSrcBegin(sconn, vm, xmlin,
+                                           &cookieout, &cookieoutlen);
+    /*
+     * If dom_xml is non-NULL the begin phase has succeeded, and the
+     * confirm phase must be called to cleanup the migration operation.
+     */
+    if (!dom_xml)
+        goto cleanup;
+
+    if (virTypedParamsAddString(&params, &nparams, &maxparams,
+                                VIR_MIGRATE_PARAM_DEST_XML, dom_xml) < 0)
+        goto confirm;
+
     /* We don't require the destination to have P2P support
      * as it looks to be normal migration from the receiver perpective.
      */
@@ -1006,7 +1011,7 @@ libxlDoMigrateSrcP2P(libxlDriverPrivatePtr driver,
     virObjectUnlock(vm);
     if (flags & VIR_MIGRATE_TUNNELLED) {
         if (!(st = virStreamNew(dconn, 0)))
-            goto cleanup;
+            goto confirm;
         ret = dconn->driver->domainMigratePrepareTunnel3Params
             (dconn, st, params, nparams, cookieout, cookieoutlen, NULL, NULL, destflags);
     } else {
@@ -1016,7 +1021,7 @@ libxlDoMigrateSrcP2P(libxlDriverPrivatePtr driver,
     virObjectLock(vm);
 
     if (ret == -1)
-        goto cleanup;
+        goto confirm;
 
     if (!(flags & VIR_MIGRATE_TUNNELLED)) {
         if (uri_out) {
@@ -1038,8 +1043,10 @@ libxlDoMigrateSrcP2P(libxlDriverPrivatePtr driver,
     else
         ret = libxlDomainMigrationSrcPerform(driver, vm, NULL, NULL,
                                              uri_out, NULL, flags);
-    if (ret < 0)
+    if (ret < 0) {
+        notify_source = false;
         orig_err = virSaveLastError();
+    }
 
     cancelled = (ret < 0);
 
@@ -1067,12 +1074,15 @@ libxlDoMigrateSrcP2P(libxlDriverPrivatePtr driver,
     if (!orig_err)
         orig_err = virSaveLastError();
 
-    VIR_DEBUG("Confirm3 cancelled=%d vm=%p", cancelled, vm);
-    ret = libxlDomainMigrationSrcConfirm(driver, vm, flags, cancelled);
+ confirm:
+    if (notify_source) {
+        VIR_DEBUG("Confirm3 cancelled=%d vm=%p", cancelled, vm);
+        ret = libxlDomainMigrationSrcConfirm(driver, vm, flags, cancelled);
 
-    if (ret < 0)
-        VIR_WARN("Guest %s probably left in 'paused' state on source",
-                 vm->def->name);
+        if (ret < 0)
+            VIR_WARN("Guest %s probably left in 'paused' state on source",
+                     vm->def->name);
+    }
 
  cleanup:
     if (flags & VIR_MIGRATE_TUNNELLED) {