From: "Denis V. Lunev" <[email protected]>
With dirty bitmaps on more than one node, the source switches nodes
during the postcopy bulk phase and sends the node name again. The
destination reads those chunks in the postcopy listen thread, which does
not hold the BQL, and resolves the name through bdrv_lookup_bs():
qemu-system-x86_64: ../block.c:6400: bdrv_lookup_bs: Assertion
`qemu_in_main_thread()' failed.
The bdrv_find_dirty_bitmap() call right after it needs the BQL or the
dirty bitmap lock as well. Taking the BQL there would deadlock: the
listen thread holds s->lock, and dirty_bitmap_mig_before_vm_start()
takes s->lock with the BQL held.
Every node and bitmap that a non-START chunk refers to was set up by a
START chunk, and those are loaded under the BQL during setup. Record the
node alias there, and resolve all other chunks against s->bitmaps
instead of the block graph.
Bitmaps on a single node never resend the node name after setup, which
is why migrate-bitmaps-postcopy-test does not catch this.
Fixes: f791bf7f93f2 ("assertions for block global state API")
Signed-off-by: Denis V. Lunev <[email protected]>
Cc: Peter Xu <[email protected]>
Cc: Fabiano Rosas <[email protected]>
Cc: Eric Blake <[email protected]>
Cc: Vladimir Sementsov-Ogievskiy <[email protected]>
Cc: [email protected]
Message-ID: <[email protected]>
Reviewed-by: Eric Blake <[email protected]>
Signed-off-by: Eric Blake <[email protected]>
(cherry picked from commit 4d11088a088d0915196e38b3e69df12d4f06fdb5)
Signed-off-by: Michael Tokarev <[email protected]>
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 702e761c0ed..0625b9ef2da 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -140,6 +140,7 @@ typedef struct LoadBitmapState {
BdrvDirtyBitmap *bitmap;
bool migrated;
bool enabled;
+ char node_alias[QEMU_COUNTED_STRING_SIZE];
} LoadBitmapState;
/* State of the dirty bitmap migration (DBM) during load process */
@@ -855,6 +856,7 @@ static int dirty_bitmap_load_start(QEMUFile *f,
DBMLoadState *s)
}
b = g_new(LoadBitmapState, 1);
+ g_strlcpy(b->node_alias, s->node_alias, sizeof(b->node_alias));
b->bs = s->bs;
b->bitmap = s->bitmap;
b->migrated = false;
@@ -1052,6 +1054,53 @@ static int dirty_bitmap_load_bits(QEMUFile *f,
DBMLoadState *s)
return 0;
}
+/*
+ * Only START chunks are guaranteed to be loaded under the BQL; the rest may
+ * come from the postcopy listen thread and must not touch the block graph.
+ */
+static BlockDriverState *dirty_bitmap_load_find_bs(DBMLoadState *s,
+ const char *device,
+ const char *node_name,
+ Error **errp)
+{
+ GSList *item;
+
+ if (s->flags & DIRTY_BITMAP_MIG_FLAG_START) {
+ return bdrv_lookup_bs(device, node_name, errp);
+ }
+
+ for (item = s->bitmaps; item; item = g_slist_next(item)) {
+ LoadBitmapState *b = item->data;
+
+ if (!strcmp(b->node_alias, s->node_alias)) {
+ return b->bs;
+ }
+ }
+
+ error_setg(errp, "no migrated bitmaps on node alias '%s'", s->node_alias);
+ return NULL;
+}
+
+static BdrvDirtyBitmap *dirty_bitmap_load_find_bitmap(DBMLoadState *s)
+{
+ GSList *item;
+
+ if (s->flags & DIRTY_BITMAP_MIG_FLAG_START) {
+ return bdrv_find_dirty_bitmap(s->bs, s->bitmap_name);
+ }
+
+ for (item = s->bitmaps; item; item = g_slist_next(item)) {
+ LoadBitmapState *b = item->data;
+
+ if (b->bs == s->bs &&
+ !strcmp(bdrv_dirty_bitmap_name(b->bitmap), s->bitmap_name)) {
+ return b->bitmap;
+ }
+ }
+
+ return NULL;
+}
+
static int dirty_bitmap_load_header(QEMUFile *f, DBMLoadState *s,
GHashTable *alias_map)
{
@@ -1080,11 +1129,12 @@ static int dirty_bitmap_load_header(QEMUFile *f,
DBMLoadState *s,
s->bs = NULL;
} else {
bitmap_alias_map = amin->subtree;
- s->bs = bdrv_lookup_bs(NULL, amin->string, &local_err);
+ s->bs = dirty_bitmap_load_find_bs(s, NULL, amin->string,
+ &local_err);
}
} else {
- s->bs = bdrv_lookup_bs(s->node_alias, s->node_alias,
- &local_err);
+ s->bs = dirty_bitmap_load_find_bs(s, s->node_alias,
+ s->node_alias, &local_err);
}
if (!s->bs) {
error_report_err(local_err);
@@ -1135,7 +1185,7 @@ static int dirty_bitmap_load_header(QEMUFile *f,
DBMLoadState *s,
if (!s->cancelled) {
g_strlcpy(s->bitmap_name, bitmap_name, sizeof(s->bitmap_name));
- s->bitmap = bdrv_find_dirty_bitmap(s->bs, s->bitmap_name);
+ s->bitmap = dirty_bitmap_load_find_bitmap(s);
/*
* bitmap may be NULL here, it wouldn't be an error if it is the
--
2.47.3
