The bdrv_create_dirty_bitmap() function (which is also called by
bdrv_dirty_bitmap_create_successor()) uses bdrv_getlength(bs). This is
a wrapper around a coroutine, and when not called in coroutine context
would use bdrv_poll_co(). Such a call would trigger an assert() if the
correct AioContext hasn't been acquired before, because polling would
try to release the AioContext.

The issue does not happen for migration, because the call happens
from process_incoming_migration_co(), i.e. in coroutine context. So
the bdrv_getlength() wrapper will just call bdrv_co_getlength()
directly without polling.

The issue would happen for snapshots, but won't in practice, because
saving a snapshot with a block dirty bitmap is currently not possible.
The reason is that dirty_bitmap_save_iterate() returns whether it has
completed the bulk phase, which only happens in postcopy, so
qemu_savevm_state_iterate() will always return 0, meaning the call
to iterate will be repeated over and over again without ever reaching
the completion phase.

Still, this would make the code more robust for the future.

Signed-off-by: Fiona Ebner <f.eb...@proxmox.com>
---

We ran into this issue downstream, because we have a custom snapshot
mechanism which does support dirty bitmaps and does not run in
coroutine context during load.

 migration/block-dirty-bitmap.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 032fc5f405..e1ae3b7316 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -805,8 +805,11 @@ static int dirty_bitmap_load_start(QEMUFile *f, 
DBMLoadState *s)
                      "destination", bdrv_dirty_bitmap_name(s->bitmap));
         return -EINVAL;
     } else {
+        AioContext *ctx = bdrv_get_aio_context(s->bs);
+        aio_context_acquire(ctx);
         s->bitmap = bdrv_create_dirty_bitmap(s->bs, granularity,
                                              s->bitmap_name, &local_err);
+        aio_context_release(ctx);
         if (!s->bitmap) {
             error_report_err(local_err);
             return -EINVAL;
@@ -833,7 +836,10 @@ static int dirty_bitmap_load_start(QEMUFile *f, 
DBMLoadState *s)
 
     bdrv_disable_dirty_bitmap(s->bitmap);
     if (flags & DIRTY_BITMAP_MIG_START_FLAG_ENABLED) {
+        AioContext *ctx = bdrv_get_aio_context(s->bs);
+        aio_context_acquire(ctx);
         bdrv_dirty_bitmap_create_successor(s->bitmap, &local_err);
+        aio_context_release(ctx);
         if (local_err) {
             error_report_err(local_err);
             return -EINVAL;
-- 
2.39.2



Reply via email to