Fix preservation of errors across migration steps

When doing migration, if an error occurs in Perform, it must not
be overwritten during Finish/Confirm steps. If an error occurs
in Finish, it must not be overwritten in Confirm.

Previous commit a9d12c2444e43a0d3e5135eb15b4b62a7c011427 added
code to qemudDomainMigrateFinish2 to preserve the error. This
is not the right place, because it is not applicable in non-p2p
migration. The src/libvirt.c virDomainMigrateV2/3 methods need
code to preserve errors for non-p2p migration, while the
doPeer2PeerMigrate2 and doPeer2PeerMigrate3 methods contain
code to preverse errors for p2p migration.

Remove the bogus error preservation from qemudDomainMigrateFinish2
and qemudDomainMigrateFinish3.

Fix virDomainMigrateV3 and doPeer2PeerMigrate3 so that they
preserve any error hit during the Finish3 step, before invoking
Confirm3.

Finally if qemuMigrationFinish fails to resume the CPUs, it must
preserve the error before tearing down the VM, so that VM cleanup
doesn't overwrite it.

* src/libvirt.c: Preserve error before invoking Confirm3
* src/qemu/qemu_driver.c: Remove bogus error preservation
  code in qemudDomainMigrateFinish2/qemudDomainMigrateFinish3
* src/qemu/qemu_migration.c: Preserve error before invoking Confirm3
  and after resume fails in qemuMigrationFinish.

diff --git a/src/libvirt.c b/src/libvirt.c
index 6aa952bea9b7dc41b8f2f120f5ccc94f69996662..8f9b6cebda0e44d666540637e90d8cfb723a749f 100644 (file)
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -3842,6 +3842,12 @@ finish:
      */
     cancelled = ret == 0 && ddomain == NULL ? 1 : 0;
 
+    /* If finish3 set an error, and we don't have an earlier
+     * one we need to preserve it in case confirm3 overwrites
+     */
+    if (!orig_err)
+        orig_err = virSaveLastError();
+
     /*
      * If cancelled, then src VM will be restarted, else
      * it will be killed

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index dd3bc6c3daac1925cbd1a19328175992a9117ed6..df6e1c2bef5ca99d0527b57ba04c0ed2c9621be8 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6002,7 +6002,6 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn,
     struct qemud_driver *driver = dconn->privateData;
     virDomainObjPtr vm;
     virDomainPtr dom = NULL;
-    virErrorPtr orig_err;
 
     virCheckFlags(VIR_MIGRATE_LIVE |
                   VIR_MIGRATE_PEER2PEER |
@@ -6013,9 +6012,6 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn,
                   VIR_MIGRATE_NON_SHARED_DISK |
                   VIR_MIGRATE_NON_SHARED_INC, NULL);
 
-    /* Migration failed. Save the current error so nothing squashes it */
-    orig_err = virSaveLastError();
-
     qemuDriverLock(driver);
     vm = virDomainFindByName(&driver->domains, dname);
     if (!vm) {
@@ -6033,10 +6029,6 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn,
                               flags, retcode, false);
 
 cleanup:
-    if (orig_err) {
-        virSetError(orig_err);
-        virFreeError(orig_err);
-    }
     qemuDriverUnlock(driver);
     return dom;
 }
@@ -6255,7 +6247,6 @@ qemuDomainMigrateFinish3(virConnectPtr dconn,
 {
     struct qemud_driver *driver = dconn->privateData;
     virDomainObjPtr vm;
-    virErrorPtr orig_err;
     int ret = -1;
 
     virCheckFlags(VIR_MIGRATE_LIVE |
@@ -6267,9 +6258,6 @@ qemuDomainMigrateFinish3(virConnectPtr dconn,
                   VIR_MIGRATE_NON_SHARED_DISK |
                   VIR_MIGRATE_NON_SHARED_INC, -1);
 
-    /* Migration failed. Save the current error so nothing squashes it */
-    orig_err = virSaveLastError();
-
     qemuDriverLock(driver);
     vm = virDomainFindByName(&driver->domains, dname);
     if (!vm) {
@@ -6286,10 +6274,6 @@ qemuDomainMigrateFinish3(virConnectPtr dconn,
     ret = 0;
 
 cleanup:
-    if (orig_err) {
-        virSetError(orig_err);
-        virFreeError(orig_err);
-    }
     qemuDriverUnlock(driver);
     return ret;
 }
@@ -6304,7 +6288,6 @@ qemuDomainMigrateConfirm3(virDomainPtr domain,
     struct qemud_driver *driver = domain->conn->privateData;
     virDomainObjPtr vm;
     int ret = -1;
-    virErrorPtr orig_err;
 
     virCheckFlags(VIR_MIGRATE_LIVE |
                   VIR_MIGRATE_PEER2PEER |
@@ -6332,8 +6315,6 @@ qemuDomainMigrateConfirm3(virDomainPtr domain,
                                cookiein, cookieinlen,
                                flags, cancelled);
 
-    orig_err = virSaveLastError();
-
     if (qemuDomainObjEndJob(vm) == 0) {
         vm = NULL;
     } else if (!virDomainObjIsActive(vm) &&
@@ -6344,11 +6325,6 @@ qemuDomainMigrateConfirm3(virDomainPtr domain,
         vm = NULL;
     }
 
-    if (orig_err) {
-        virSetError(orig_err);
-        virFreeError(orig_err);
-    }
-
 cleanup:
     if (vm)
         virDomainObjUnlock(vm);

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 30df054c1c04cd547a73fc28bb6014d64524e299..660365568c3f0888f90a31d8c39f4b512701d045 100644 (file)
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1956,6 +1956,12 @@ finish:
      */
     cancelled = ret == 0 && ddomain == NULL ? 1 : 0;
 
+    /* If finish3 set an error, and we don't have an earlier
+     * one we need to preserve it in case confirm3 overwrites
+     */
+    if (!orig_err)
+        orig_err = virSaveLastError();
+
     /*
      * If cancelled, then src VM will be restarted, else
      * it will be killed
@@ -2249,6 +2255,7 @@ qemuMigrationFinish(struct qemud_driver *driver,
               "cookieout=%p, cookieoutlen=%p, flags=%lu, retcode=%d",
               driver, dconn, vm, NULLSTR(cookiein), cookieinlen,
               cookieout, cookieoutlen, flags, retcode);
+    virErrorPtr orig_err = NULL;
 
     priv = vm->privateData;
     if (priv->jobActive != QEMU_JOB_MIGRATION_IN) {
@@ -2314,6 +2321,14 @@ qemuMigrationFinish(struct qemud_driver *driver,
              */
             if (qemuProcessStartCPUs(driver, vm, dconn,
                                      VIR_DOMAIN_RUNNING_MIGRATED) < 0) {
+                if (virGetLastError() == NULL)
+                    qemuReportError(VIR_ERR_INTERNAL_ERROR,
+                                    "%s", _("resume operation failed"));
+                /* Need to save the current error, in case shutting
+                 * down the process overwrites it
+                 */
+                orig_err = virSaveLastError();
+
                 /*
                  * In v3 protocol, the source VM is still available to
                  * restart during confirm() step, so we kill it off
@@ -2334,9 +2349,6 @@ qemuMigrationFinish(struct qemud_driver *driver,
                         vm = NULL;
                     }
                 }
-                if (virGetLastError() == NULL)
-                    qemuReportError(VIR_ERR_INTERNAL_ERROR,
-                                    "%s", _("resume operation failed"));
                 goto endjob;
             }
         }
@@ -2384,6 +2396,10 @@ cleanup:
     if (event)
         qemuDomainEventQueue(driver, event);
     qemuMigrationCookieFree(mig);
+    if (orig_err) {
+        virSetError(orig_err);
+        virFreeError(orig_err);
+    }
     return dom;
 }