[libvirt PATCH 47/80] qemu: Make qemuMigrationCheckPhase failure fatal

[Jiri Denemark]

The check can reveal a serious bug in our migration code and we should
not silently ignore it.

Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
---
 src/qemu/qemu_migration.c | 58 ++++++++++++++++++++++++---------------
 1 file changed, 36 insertions(+), 22 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 5b6073b963..1c5dd9b391 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -147,9 +147,10 @@ qemuMigrationCheckPhase(virDomainObj *vm,
 
     if (phase < QEMU_MIGRATION_PHASE_POSTCOPY_FAILED &&
         phase < priv->job.phase) {
-        VIR_ERROR(_("migration protocol going backwards %s => %s"),
-                  qemuMigrationJobPhaseTypeToString(priv->job.phase),
-                  qemuMigrationJobPhaseTypeToString(phase));
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("migration protocol going backwards %s => %s"),
+                       qemuMigrationJobPhaseTypeToString(priv->job.phase),
+                       qemuMigrationJobPhaseTypeToString(phase));
         return -1;
     }
 
@@ -157,22 +158,23 @@ qemuMigrationCheckPhase(virDomainObj *vm,
 }
 
 
-static void ATTRIBUTE_NONNULL(1)
+static int G_GNUC_WARN_UNUSED_RESULT
 qemuMigrationJobSetPhase(virDomainObj *vm,
                          qemuMigrationJobPhase phase)
 {
     if (qemuMigrationCheckPhase(vm, phase) < 0)
-        return;
+        return -1;
 
     qemuDomainObjSetJobPhase(vm, phase);
+    return 0;
 }
 
 
-static void ATTRIBUTE_NONNULL(1)
+static int G_GNUC_WARN_UNUSED_RESULT
 qemuMigrationJobStartPhase(virDomainObj *vm,
                            qemuMigrationJobPhase phase)
 {
-    qemuMigrationJobSetPhase(vm, phase);
+    return qemuMigrationJobSetPhase(vm, phase);
 }
 
 
@@ -2596,8 +2598,9 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver,
      * Otherwise we will start the async job later in the perform phase losing
      * change protection.
      */
-    if (priv->job.asyncJob == VIR_ASYNC_JOB_MIGRATION_OUT)
-        qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_BEGIN3);
+    if (priv->job.asyncJob == VIR_ASYNC_JOB_MIGRATION_OUT &&
+        qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_BEGIN3) < 0)
+        return NULL;
 
     if (!qemuMigrationSrcIsAllowed(driver, vm, true, flags))
         return NULL;
@@ -3148,7 +3151,9 @@ qemuMigrationDstPrepareAny(virQEMUDriver *driver,
     if (qemuMigrationJobStart(driver, vm, VIR_ASYNC_JOB_MIGRATION_IN,
                               flags) < 0)
         goto cleanup;
-    qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PREPARE);
+
+    if (qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PREPARE) < 0)
+        goto stopjob;
 
     /* Domain starts inactive, even if the domain XML had an id field. */
     vm->def->id = -1;
@@ -3668,7 +3673,8 @@ qemuMigrationSrcConfirmPhase(virQEMUDriver *driver,
     else
         phase = QEMU_MIGRATION_PHASE_CONFIRM3_CANCELLED;
 
-    qemuMigrationJobSetPhase(vm, phase);
+    if (qemuMigrationJobSetPhase(vm, phase) < 0)
+        return -1;
 
     if (!(mig = qemuMigrationCookieParse(driver, vm->def, priv->origname, priv,
                                          cookiein, cookieinlen,
@@ -3753,7 +3759,9 @@ qemuMigrationSrcConfirm(virQEMUDriver *driver,
     else
         phase = QEMU_MIGRATION_PHASE_CONFIRM3;
 
-    qemuMigrationJobStartPhase(vm, phase);
+    if (qemuMigrationJobStartPhase(vm, phase) < 0)
+        goto cleanup;
+
     virCloseCallbacksUnset(driver->closeCallbacks, vm,
                            qemuMigrationSrcCleanup);
 
@@ -4920,7 +4928,7 @@ qemuMigrationSrcPerformPeer2Peer2(virQEMUDriver *driver,
      * until the migration is complete.
      */
     VIR_DEBUG("Perform %p", sconn);
-    qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM2);
+    ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM2));
     if (flags & VIR_MIGRATE_TUNNELLED)
         ret = qemuMigrationSrcPerformTunnel(driver, vm, st, NULL,
                                             NULL, 0, NULL, NULL,
@@ -5164,7 +5172,7 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriver *driver,
      * confirm migration completion.
      */
     VIR_DEBUG("Perform3 %p uri=%s", sconn, NULLSTR(uri));
-    qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM3);
+    ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM3));
     VIR_FREE(cookiein);
     cookiein = g_steal_pointer(&cookieout);
     cookieinlen = cookieoutlen;
@@ -5189,7 +5197,7 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriver *driver,
     if (ret < 0) {
         virErrorPreserveLast(&orig_err);
     } else {
-        qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM3_DONE);
+        ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM3_DONE));
     }
 
     /* If Perform returns < 0, then we need to cancel the VM
@@ -5565,7 +5573,9 @@ qemuMigrationSrcPerformJob(virQEMUDriver *driver,
                                                migParams, flags, dname, resource,
                                                &v3proto);
     } else {
-        qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM2);
+        if (qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM2) < 0)
+            goto endjob;
+
         ret = qemuMigrationSrcPerformNative(driver, vm, persist_xml, uri, cookiein, cookieinlen,
                                             cookieout, cookieoutlen,
                                             flags, resource, NULL, NULL, 0, NULL,
@@ -5657,7 +5667,9 @@ qemuMigrationSrcPerformPhase(virQEMUDriver *driver,
         return ret;
     }
 
-    qemuMigrationJobStartPhase(vm, QEMU_MIGRATION_PHASE_PERFORM3);
+    if (qemuMigrationJobStartPhase(vm, QEMU_MIGRATION_PHASE_PERFORM3) < 0)
+        goto endjob;
+
     virCloseCallbacksUnset(driver->closeCallbacks, vm,
                            qemuMigrationSrcCleanup);
 
@@ -5671,7 +5683,7 @@ qemuMigrationSrcPerformPhase(virQEMUDriver *driver,
         goto endjob;
     }
 
-    qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM3_DONE);
+    ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM3_DONE));
 
     if (virCloseCallbacksSet(driver->closeCallbacks, vm, conn,
                              qemuMigrationSrcCleanup) < 0)
@@ -6238,9 +6250,10 @@ qemuMigrationDstFinish(virQEMUDriver *driver,
 
     ignore_value(virTimeMillisNow(&timeReceived));
 
-    qemuMigrationJobStartPhase(vm,
-                               v3proto ? QEMU_MIGRATION_PHASE_FINISH3
-                                       : QEMU_MIGRATION_PHASE_FINISH2);
+    if (qemuMigrationJobStartPhase(vm,
+                                   v3proto ? QEMU_MIGRATION_PHASE_FINISH3
+                                           : QEMU_MIGRATION_PHASE_FINISH2) < 0)
+        goto cleanup;
 
     qemuDomainCleanupRemove(vm, qemuMigrationDstPrepareCleanup);
     g_clear_pointer(&priv->job.completed, virDomainJobDataFree);
@@ -6314,7 +6327,8 @@ qemuMigrationProcessUnattended(virQEMUDriver *driver,
     else
         phase = QEMU_MIGRATION_PHASE_CONFIRM3;
 
-    qemuMigrationJobStartPhase(vm, phase);
+    if (qemuMigrationJobStartPhase(vm, phase) < 0)
+        return;
 
     if (job == VIR_ASYNC_JOB_MIGRATION_IN)
         qemuMigrationDstComplete(driver, vm, true, job, &priv->job);
-- 
2.35.1