migration: refactor migration_completion

Current migration_completion function is a bit long. Refactor the long
implementation into different subfunctions:
- migration_completion_precopy: completion code related to precopy
- migration_completion_postcopy: completion code related to postcopy

Rename await_return_path_close_on_source to
close_return_path_on_source: It is renamed to match with
open_return_path_on_source.

This improves readability and is easier for future updates (e.g. add new
subfunctions when completion code related to new features are needed). No
functional changes intended.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Isaku Yamahata <isaku.yamahata@intel.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-ID: <20230804093053.5037-1-wei.w.wang@intel.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>

migration/migration.c

@@ -99,7 +99,7 @@ static int migration_maybe_pause(MigrationState *s,
                                  int *current_active_state,
                                  int new_state);
 static void migrate_fd_cancel(MigrationState *s);
-static int await_return_path_close_on_source(MigrationState *s);
+static int close_return_path_on_source(MigrationState *s);
 
 static bool migration_needs_multiple_sockets(void)
 {
@@ -1191,7 +1191,7 @@ static void migrate_fd_cleanup(MigrationState *s)
      * We already cleaned up to_dst_file, so errors from the return
      * path might be due to that, ignore them.
      */
-    await_return_path_close_on_source(s);
+    close_return_path_on_source(s);
 
     assert(!migration_is_active(s));
 
@@ -2049,8 +2049,7 @@ static int open_return_path_on_source(MigrationState *ms)
     return 0;
 }
 
-/* Returns 0 if the RP was ok, otherwise there was an error on the RP */
+static int close_return_path_on_source(MigrationState *ms)
-static int await_return_path_close_on_source(MigrationState *ms)
 {
     int ret;
 
@@ -2317,18 +2316,11 @@ static int migration_maybe_pause(MigrationState *s,
     return s->state == new_state ? 0 : -EINVAL;
 }
 
-/**
+static int migration_completion_precopy(MigrationState *s,
- * migration_completion: Used by migration_thread when there's not much left.
+                                        int *current_active_state)
- *   The caller 'breaks' the loop when this returns.
- *
- * @s: Current migration state
- */
-static void migration_completion(MigrationState *s)
 {
     int ret;
-    int current_active_state = s->state;
 
-    if (s->state == MIGRATION_STATUS_ACTIVE) {
     qemu_mutex_lock_iothread();
     s->downtime_start = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
     qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER, NULL);
@@ -2338,28 +2330,31 @@ static void migration_completion(MigrationState *s)
 
     ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
     trace_migration_completion_vm_stop(ret);
-        if (ret >= 0) {
+    if (ret < 0) {
-            ret = migration_maybe_pause(s, &current_active_state,
+        goto out_unlock;
-                                        MIGRATION_STATUS_DEVICE);
     }
-        if (ret >= 0) {
+
+    ret = migration_maybe_pause(s, current_active_state,
+                                MIGRATION_STATUS_DEVICE);
+    if (ret < 0) {
+        goto out_unlock;
+    }
+
     /*
-             * Inactivate disks except in COLO, and track that we
+     * Inactivate disks except in COLO, and track that we have done so in order
-             * have done so in order to remember to reactivate
+     * to remember to reactivate them if migration fails or is cancelled.
-             * them if migration fails or is cancelled.
      */
     s->block_inactive = !migrate_colo();
     migration_rate_set(RATE_LIMIT_DISABLED);
     ret = qemu_savevm_state_complete_precopy(s->to_dst_file, false,
                                              s->block_inactive);
-        }
+out_unlock:
-
     qemu_mutex_unlock_iothread();
-
+    return ret;
-        if (ret < 0) {
-            goto fail;
 }
-    } else if (s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE) {
+
+static void migration_completion_postcopy(MigrationState *s)
+{
     trace_migration_completion_postcopy_end();
 
     qemu_mutex_lock_iothread();
@@ -2367,40 +2362,19 @@ static void migration_completion(MigrationState *s)
     qemu_mutex_unlock_iothread();
 
     /*
-         * Shutdown the postcopy fast path thread.  This is only needed
+     * Shutdown the postcopy fast path thread.  This is only needed when dest
-         * when dest QEMU binary is old (7.1/7.2).  QEMU 8.0+ doesn't need
+     * QEMU binary is old (7.1/7.2).  QEMU 8.0+ doesn't need this.
-         * this.
      */
     if (migrate_postcopy_preempt() && s->preempt_pre_7_2) {
         postcopy_preempt_shutdown_file(s);
     }
 
     trace_migration_completion_postcopy_end_after_complete();
-    } else {
-        goto fail;
 }
 
-    if (await_return_path_close_on_source(s)) {
+static void migration_completion_failed(MigrationState *s,
-        goto fail;
+                                        int current_active_state)
-    }
+{
-
-    if (qemu_file_get_error(s->to_dst_file)) {
-        trace_migration_completion_file_err();
-        goto fail;
-    }
-
-    if (migrate_colo() && s->state == MIGRATION_STATUS_ACTIVE) {
-        /* COLO does not support postcopy */
-        migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE,
-                          MIGRATION_STATUS_COLO);
-    } else {
-        migrate_set_state(&s->state, current_active_state,
-                          MIGRATION_STATUS_COMPLETED);
-    }
-
-    return;
-
-fail:
     if (s->block_inactive && (s->state == MIGRATION_STATUS_ACTIVE ||
                               s->state == MIGRATION_STATUS_DEVICE)) {
         /*
@@ -2423,6 +2397,53 @@ fail:
                       MIGRATION_STATUS_FAILED);
 }
 
+/**
+ * migration_completion: Used by migration_thread when there's not much left.
+ *   The caller 'breaks' the loop when this returns.
+ *
+ * @s: Current migration state
+ */
+static void migration_completion(MigrationState *s)
+{
+    int ret = 0;
+    int current_active_state = s->state;
+
+    if (s->state == MIGRATION_STATUS_ACTIVE) {
+        ret = migration_completion_precopy(s, &current_active_state);
+    } else if (s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE) {
+        migration_completion_postcopy(s);
+    } else {
+        ret = -1;
+    }
+
+    if (ret < 0) {
+        goto fail;
+    }
+
+    if (close_return_path_on_source(s)) {
+        goto fail;
+    }
+
+    if (qemu_file_get_error(s->to_dst_file)) {
+        trace_migration_completion_file_err();
+        goto fail;
+    }
+
+    if (migrate_colo() && s->state == MIGRATION_STATUS_ACTIVE) {
+        /* COLO does not support postcopy */
+        migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE,
+                          MIGRATION_STATUS_COLO);
+    } else {
+        migrate_set_state(&s->state, current_active_state,
+                          MIGRATION_STATUS_COMPLETED);
+    }
+
+    return;
+
+fail:
+    migration_completion_failed(s, current_active_state);
+}
+
 /**
  * bg_migration_completion: Used by bg_migration_thread when after all the
  *   RAM has been saved. The caller 'breaks' the loop when this returns.
@@ -2563,7 +2584,7 @@ static MigThrError postcopy_pause(MigrationState *s)
          * path and just wait for the thread to finish. It will be
          * re-created when we resume.
          */
-        await_return_path_close_on_source(s);
+        close_return_path_on_source(s);
 
         migrate_set_state(&s->state, s->state,
                           MIGRATION_STATUS_POSTCOPY_PAUSED);