From 5919ec1446dd41a19efa9230c5927d084fe11332 Mon Sep 17 00:00:00 2001 From: Fiona Ebner Date: Fri, 28 Jul 2023 11:44:57 +0200 Subject: [PATCH] add patch fixing resume for snapshot and hibernate with drive with iothread and a dirty bitmap Not difficult to run into, just have a drive with iothread, take a PBS backup and then take a snapshot or hibernate. Resuming will fail with > qemu: qemu_mutex_unlock_impl: Operation not permitted because of not acquiring the correct AioContext first. Migration is not affected, because it runs in coroutine context. Reported in the community forum: https://forum.proxmox.com/threads/129899/ Signed-off-by: Fiona Ebner --- ...dirty-bitmap-fix-loading-bitmap-when.patch | 48 +++++++++++++++++++ ...dirty-bitmap-migrate-other-bitmaps-e.patch | 2 +- ...apshots-hold-the-BQL-during-setup-ca.patch | 6 +-- debian/patches/series | 1 + 4 files changed, 53 insertions(+), 4 deletions(-) create mode 100644 debian/patches/extra/0010-migration-block-dirty-bitmap-fix-loading-bitmap-when.patch diff --git a/debian/patches/extra/0010-migration-block-dirty-bitmap-fix-loading-bitmap-when.patch b/debian/patches/extra/0010-migration-block-dirty-bitmap-fix-loading-bitmap-when.patch new file mode 100644 index 0000000..bb01ced --- /dev/null +++ b/debian/patches/extra/0010-migration-block-dirty-bitmap-fix-loading-bitmap-when.patch @@ -0,0 +1,48 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Fiona Ebner +Date: Fri, 28 Jul 2023 10:47:48 +0200 +Subject: [PATCH] migration/block-dirty-bitmap: fix loading bitmap when there + is an iothread + +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 thus uses bdrv_poll_co(). Polling +tries to release the AioContext which will trigger an assert() if it +hasn't been acquired before. + +The issue does not happen for migration, because there we are in a +coroutine already, so the wrapper will just call bdrv_co_getlength() +directly without polling. + +Signed-off-by: Fiona Ebner +--- + 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 fe73aa94b1..7eaf498439 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; diff --git a/debian/patches/pve/0035-migration-block-dirty-bitmap-migrate-other-bitmaps-e.patch b/debian/patches/pve/0035-migration-block-dirty-bitmap-migrate-other-bitmaps-e.patch index 0e3f38d..bd721fc 100644 --- a/debian/patches/pve/0035-migration-block-dirty-bitmap-migrate-other-bitmaps-e.patch +++ b/debian/patches/pve/0035-migration-block-dirty-bitmap-migrate-other-bitmaps-e.patch @@ -19,7 +19,7 @@ Signed-off-by: Thomas Lamprecht 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c -index fe73aa94b1..a6440929fa 100644 +index 7eaf498439..509f3df0a6 100644 --- a/migration/block-dirty-bitmap.c +++ b/migration/block-dirty-bitmap.c @@ -539,7 +539,7 @@ static int add_bitmaps_to_list(DBMSaveState *s, BlockDriverState *bs, diff --git a/debian/patches/pve/0044-migration-for-snapshots-hold-the-BQL-during-setup-ca.patch b/debian/patches/pve/0044-migration-for-snapshots-hold-the-BQL-during-setup-ca.patch index cbc39cc..04ef6cb 100644 --- a/debian/patches/pve/0044-migration-for-snapshots-hold-the-BQL-during-setup-ca.patch +++ b/debian/patches/pve/0044-migration-for-snapshots-hold-the-BQL-during-setup-ca.patch @@ -67,10 +67,10 @@ index a8dfd8fefd..fa9b0b0f10 100644 * must_precopy: * - must be migrated in precopy or in stopped state diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c -index a6440929fa..69fab3275c 100644 +index 509f3df0a6..42dc4a8d61 100644 --- a/migration/block-dirty-bitmap.c +++ b/migration/block-dirty-bitmap.c -@@ -1214,10 +1214,17 @@ static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque) +@@ -1220,10 +1220,17 @@ static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque) { DBMSaveState *s = &((DBMState *)opaque)->save; SaveBitmapState *dbms = NULL; @@ -90,7 +90,7 @@ index a6440929fa..69fab3275c 100644 return -1; } -@@ -1225,7 +1232,9 @@ static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque) +@@ -1231,7 +1238,9 @@ static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque) send_bitmap_start(f, s, dbms); } qemu_put_bitmap_flags(f, DIRTY_BITMAP_MIG_FLAG_EOS); diff --git a/debian/patches/series b/debian/patches/series index c9c96d7..a4dd4c2 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -7,6 +7,7 @@ extra/0006-lsi53c895a-disable-reentrancy-detection-for-script-R.patch extra/0007-bcm2835_property-disable-reentrancy-detection-for-io.patch extra/0008-raven-disable-reentrancy-detection-for-iomem.patch extra/0009-apic-disable-reentrancy-detection-for-apic-msi.patch +extra/0010-migration-block-dirty-bitmap-fix-loading-bitmap-when.patch bitmap-mirror/0001-drive-mirror-add-support-for-sync-bitmap-mode-never.patch bitmap-mirror/0002-drive-mirror-add-support-for-conditional-and-always-.patch bitmap-mirror/0003-mirror-add-check-for-bitmap-mode-without-bitmap.patch -- 2.39.5