[PATCH v2 3/4] migration: Add global_migration

Peter Xu posted 4 patches 1 month ago
There is a newer version of this series
[PATCH v2 3/4] migration: Add global_migration
Posted by Peter Xu 1 month ago
Add a variable that is only used in exported / global migration helpers,
reflecting whether migration is available to the outside world.

Note that we haven't yet started using this variable, but hopefully that is
still better already because now we have an explicit place to say who owns
the initial migration refcount.  In this case, it's the global_migration
pointer (rather than current_migration) that owns it.  Then in shutdown()
we clear that pointer right after the unref() would make sense.

We'll start to use the variable in the next patch to provide thread safety
to all migration exported helpers.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/migration.c | 44 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 41 insertions(+), 3 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index a82297db0f..c4adad7972 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -96,7 +96,36 @@ enum mig_rp_message_type {
    migrations at once.  For now we don't need to add
    dynamic creation of migration */
 
-static MigrationState *current_migration;
+/*
+ * We have two pointers for the global migration objects.  Both of them are
+ * initialized early during QEMU starts, but they have different lifecycles.
+ *
+ * - current_migration
+ *
+ *   This variable reflects the whole lifecycle of the migration object
+ *   (which each QEMU can only have one).  It is valid until the migration
+ *   object is destroyed.
+ *
+ *   This is the object that internal migration so far use.  For example,
+ *   internal helper migrate_get_current() references it.
+ *
+ *   When all migration code can always pass over a MigrationState* around,
+ *   this variable can logically be dropped.  But we're not yet there.
+ *
+ * - global_migration
+ *
+ *   This is valid only until the migration object is still valid to the
+ *   outside-migration world (until migration_shutdown()).
+ *
+ *   This should normally be always set, cleared or accessed by the main
+ *   thread only, rather than the migration thread.
+ *
+ *   All the exported functions (in include/migration) should reference the
+ *   exported migration object only to avoid race conditions, as
+ *   current_migration can be freed concurrently by migration thread when
+ *   the migration thread holds the last refcount.
+ */
+static MigrationState *current_migration, *global_migration;
 static MigrationIncomingState *current_incoming;
 
 static GSList *migration_blockers[MIG_MODE__MAX];
@@ -232,7 +261,9 @@ static int migration_stop_vm(MigrationState *s, RunState state)
 
 void migration_object_init(void)
 {
-    MIGRATION_OBJ(object_new(TYPE_MIGRATION));
+    /* The global variable holds the refcount */
+    global_migration = MIGRATION_OBJ(object_new(TYPE_MIGRATION));
+
     /* This should be set when initialize the object */
     assert(current_migration);
 
@@ -333,7 +364,14 @@ void migration_shutdown(void)
      * stop the migration using this structure
      */
     migration_cancel(NULL);
-    object_unref(OBJECT(current_migration));
+
+    /*
+     * This marks that migration object is not visible anymore to
+     * outside-migration world.  Migration might still hold a refcount if
+     * it's still in progress.
+     */
+    object_unref(OBJECT(global_migration));
+    global_migration = NULL;
 
     /*
      * Cancel outgoing migration of dirty bitmaps. It should
-- 
2.45.0