From: "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com>
To: qemu-devel@nongnu.org, quintela@redhat.com, peterx@redhat.com,
leobras@redhat.com
Cc: pkrempa@redhat.com
Subject: [PATCH] migration: Read state once
Date: Wed, 13 Apr 2022 12:33:29 +0100 [thread overview]
Message-ID: <20220413113329.103696-1-dgilbert@redhat.com> (raw)
From: "Dr. David Alan Gilbert" <dgilbert@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 <dgilbert@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 {
--
2.35.1
next reply other threads:[~2022-04-13 11:35 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-13 11:33 Dr. David Alan Gilbert (git) [this message]
2022-04-19 12:47 ` [PATCH] migration: Read state once Juan Quintela
2022-04-19 13:04 ` Peter Xu
2022-04-21 14:13 ` Dr. David Alan Gilbert
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220413113329.103696-1-dgilbert@redhat.com \
--to=dgilbert@redhat.com \
--cc=leobras@redhat.com \
--cc=peterx@redhat.com \
--cc=pkrempa@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=quintela@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).