97168e
From 34eae2d7ef928a7e0e10cc30fe76839c005998eb Mon Sep 17 00:00:00 2001
0c40bc
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
0c40bc
Date: Wed, 13 Apr 2022 12:33:29 +0100
97168e
Subject: [PATCH 07/11] migration: Read state once
0c40bc
0c40bc
RH-Author: Dr. David Alan Gilbert <dgilbert@redhat.com>
97168e
RH-MergeRequest: 249: migration: Read state once
97168e
RH-Bugzilla: 2074205
0c40bc
RH-Acked-by: Peter Xu <peterx@redhat.com>
97168e
RH-Acked-by: Laszlo Ersek <lersek@redhat.com>
97168e
RH-Acked-by: Jon Maloy <jmaloy@redhat.com>
0c40bc
RH-Acked-by: quintela1 <quintela@redhat.com>
97168e
RH-Commit: [1/1] 9aa47b492a646fce4e66ebd9b7d7a85286d16051
0c40bc
0c40bc
The 'status' field for the migration is updated normally using
0c40bc
an atomic operation from the migration thread.
0c40bc
Most readers of it aren't that careful, and in most cases it doesn't
0c40bc
matter.
0c40bc
0c40bc
In query_migrate->fill_source_migration_info the 'state'
0c40bc
is read twice; the first time to decide which state fields to fill in,
0c40bc
and then secondly to copy the state to the status field; that can end up
0c40bc
with a status that's inconsistent; e.g. setting up the fields
0c40bc
for 'setup' and then having an 'active' status.  In that case
0c40bc
libvirt gets upset by the lack of ram info.
0c40bc
The symptom is:
0c40bc
   libvirt.libvirtError: internal error: migration was active, but no RAM info was set
0c40bc
0c40bc
Read the state exactly once in fill_source_migration_info.
0c40bc
0c40bc
This is a possible fix for:
0c40bc
https://bugzilla.redhat.com/show_bug.cgi?id=2074205
0c40bc
0c40bc
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
0c40bc
Message-Id: <20220413113329.103696-1-dgilbert@redhat.com>
0c40bc
Reviewed-by: Juan Quintela <quintela@redhat.com>
0c40bc
Reviewed-by: Peter Xu <peterx@redhat.com>
0c40bc
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
0c40bc
(cherry picked from commit 552de79bfdd5e9e53847eb3c6d6e4cd898a4370e)
0c40bc
---
0c40bc
 migration/migration.c | 5 +++--
0c40bc
 1 file changed, 3 insertions(+), 2 deletions(-)
0c40bc
0c40bc
diff --git a/migration/migration.c b/migration/migration.c
0c40bc
index 51e6726dac..d8b24a2c91 100644
0c40bc
--- a/migration/migration.c
0c40bc
+++ b/migration/migration.c
0c40bc
@@ -1071,6 +1071,7 @@ static void populate_disk_info(MigrationInfo *info)
0c40bc
 static void fill_source_migration_info(MigrationInfo *info)
0c40bc
 {
0c40bc
     MigrationState *s = migrate_get_current();
0c40bc
+    int state = qatomic_read(&s->state);
0c40bc
     GSList *cur_blocker = migration_blockers;
0c40bc
 
0c40bc
     info->blocked_reasons = NULL;
0c40bc
@@ -1090,7 +1091,7 @@ static void fill_source_migration_info(MigrationInfo *info)
0c40bc
     }
0c40bc
     info->has_blocked_reasons = info->blocked_reasons != NULL;
0c40bc
 
0c40bc
-    switch (s->state) {
0c40bc
+    switch (state) {
0c40bc
     case MIGRATION_STATUS_NONE:
0c40bc
         /* no migration has happened ever */
0c40bc
         /* do not overwrite destination migration status */
0c40bc
@@ -1135,7 +1136,7 @@ static void fill_source_migration_info(MigrationInfo *info)
0c40bc
         info->has_status = true;
0c40bc
         break;
0c40bc
     }
0c40bc
-    info->status = s->state;
0c40bc
+    info->status = state;
0c40bc
 }
0c40bc
 
0c40bc
 typedef enum WriteTrackingSupport {
0c40bc
-- 
0c40bc
2.37.3
0c40bc