"Dr. David Alan Gilbert (git)" <dgilb...@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilb...@redhat.com>
>
> The 'status' field for the migration is updated normally using
> an atomic operation from the migration thread.
> Most readers of it aren't that careful, and in most cases it doesn't
> matter.
>
> In query_migrate->fill_source_migration_info the 'state'
> is read twice; the first time to decide which state fields to fill in,
> and then secondly to copy the state to the status field; that can end up
> with a status that's inconsistent; e.g. setting up the fields
> for 'setup' and then having an 'active' status. In that case
> libvirt gets upset by the lack of ram info.
> The symptom is:
> libvirt.libvirtError: internal error: migration was active, but no RAM
> info was set
>
> Read the state exactly once in fill_source_migration_info.
>
> This is a possible fix for:
> https://bugzilla.redhat.com/show_bug.cgi?id=2074205
>
> Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
Reviewed-by: Juan Quintela <quint...@redhat.com>
> ---
> migration/migration.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 695f0f2900..811c584619 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1073,6 +1073,7 @@ static void populate_disk_info(MigrationInfo *info)
> static void fill_source_migration_info(MigrationInfo *info)
> {
> MigrationState *s = migrate_get_current();
> + int state = qatomic_read(&s->state);
> GSList *cur_blocker = migration_blockers;
>
> info->blocked_reasons = NULL;
> @@ -1092,7 +1093,7 @@ static void fill_source_migration_info(MigrationInfo
> *info)
> }
> info->has_blocked_reasons = info->blocked_reasons != NULL;
>
> - switch (s->state) {
> + switch (state) {
> case MIGRATION_STATUS_NONE:
> /* no migration has happened ever */
> /* do not overwrite destination migration status */
> @@ -1137,7 +1138,7 @@ static void fill_source_migration_info(MigrationInfo
> *info)
> info->has_status = true;
> break;
> }
> - info->status = s->state;
> + info->status = state;
> }
>
> typedef enum WriteTrackingSupport {
