[v2,04/10] migration: Cleanup incoming migration setup state change

Commit Message

Peter Xu June 17, 2024, 6:15 p.m. UTC

Destination QEMU can setup incoming ports for two purposes: either a fresh
new incoming migration, in which QEMU will switch to SETUP for channel
establishment, or a paused postcopy migration, in which QEMU will stay in
POSTCOPY_PAUSED until kicking off the RECOVER phase.

Now the state machine worked on dest node for the latter, only because
migrate_set_state() implicitly will become a noop if the current state
check failed.  It wasn't clear at all.

Clean it up by providing a helper migration_incoming_state_setup() doing
proper checks over current status.  Postcopy-paused will be explicitly
checked now, and then we can bail out for unknown states.

Signed-off-by: Peter Xu <[e-mail]>
---
 migration/migration.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

Comments

Fabiano Rosas June 17, 2024, 7:41 p.m. UTC | #1

Peter Xu <[e-mail]> writes:

> Destination QEMU can setup incoming ports for two purposes: either a fresh
> new incoming migration, in which QEMU will switch to SETUP for channel
> establishment, or a paused postcopy migration, in which QEMU will stay in
> POSTCOPY_PAUSED until kicking off the RECOVER phase.
>
> Now the state machine worked on dest node for the latter, only because
> migrate_set_state() implicitly will become a noop if the current state
> check failed.  It wasn't clear at all.
>
> Clean it up by providing a helper migration_incoming_state_setup() doing
> proper checks over current status.  Postcopy-paused will be explicitly
> checked now, and then we can bail out for unknown states.
>
> Signed-off-by: Peter Xu <[e-mail]>

Reviewed-by: Fabiano Rosas <[e-mail]>

Patch

diff --git a/migration/migration.c b/migration/migration.c
index 75c9d80e8e..59442181a1 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -595,6 +595,29 @@  bool migrate_uri_parse(const char *uri, MigrationChannel **channel,
     return true;
 }
 
+static bool
+migration_incoming_state_setup(MigrationIncomingState *mis, Error **errp)
+{
+    MigrationStatus current = mis->state;
+
+    if (current == MIGRATION_STATUS_POSTCOPY_PAUSED) {
+        /*
+         * Incoming postcopy migration will stay in PAUSED state even if
+         * reconnection happened.
+         */
+        return true;
+    }
+
+    if (current != MIGRATION_STATUS_NONE) {
+        error_setg(errp, "Illegal migration incoming state: %s",
+                   MigrationStatus_str(current));
+        return false;
+    }
+
+    migrate_set_state(&mis->state, current, MIGRATION_STATUS_SETUP);
+    return true;
+}
+
 static void qemu_start_incoming_migration(const char *uri, bool has_channels,
                                           MigrationChannelList *channels,
                                           Error **errp)
@@ -633,8 +656,9 @@  static void qemu_start_incoming_migration(const char *uri, bool has_channels,
         return;
     }
 
-    migrate_set_state(&mis->state, MIGRATION_STATUS_NONE,
-                      MIGRATION_STATUS_SETUP);
+    if (!migration_incoming_state_setup(mis, errp)) {
+        return;
+    }
 
     if (addr->transport == MIGRATION_ADDRESS_TYPE_SOCKET) {
         SocketAddress *saddr = &addr->u.socket;