From 4290b4834c4ffb9633a9851a9b932a147fcac928 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Mon, 22 Mar 2021 12:49:05 +0300 Subject: [PATCH 1/2] migration/block-dirty-bitmap: make incoming disabled bitmaps busy Incoming enabled bitmaps are busy, because we do bdrv_dirty_bitmap_create_successor() for them. But disabled bitmaps being migrated are not marked busy, and user can remove them during the incoming migration. Then we may crash in cancel_incoming_locked() when try to remove the bitmap that was already removed by user, like this: #0 qemu_mutex_lock_impl (mutex=0x5593d88c50d1, file=0x559680554b20 "../block/dirty-bitmap.c", line=64) at ../util/qemu-thread-posix.c:77 #1 bdrv_dirty_bitmaps_lock (bs=0x5593d88c0ee9) at ../block/dirty-bitmap.c:64 #2 bdrv_release_dirty_bitmap (bitmap=0x5596810e9570) at ../block/dirty-bitmap.c:362 #3 cancel_incoming_locked (s=0x559680be8208 ) at ../migration/block-dirty-bitmap.c:918 #4 dirty_bitmap_load (f=0x559681d02b10, opaque=0x559680be81e0 , version_id=1) at ../migration/block-dirty-bitmap.c:1194 #5 vmstate_load (f=0x559681d02b10, se=0x559680fb5810) at ../migration/savevm.c:908 #6 qemu_loadvm_section_part_end (f=0x559681d02b10, mis=0x559680fb4a30) at ../migration/savevm.c:2473 #7 qemu_loadvm_state_main (f=0x559681d02b10, mis=0x559680fb4a30) at ../migration/savevm.c:2626 #8 postcopy_ram_listen_thread (opaque=0x0) at ../migration/savevm.c:1871 #9 qemu_thread_start (args=0x5596817ccd10) at ../util/qemu-thread-posix.c:521 #10 start_thread () at /lib64/libpthread.so.0 #11 clone () at /lib64/libc.so.6 Note bs pointer taken from bitmap: it's definitely bad aligned. That's because we are in use after free, bitmap is already freed. So, let's make disabled bitmaps (being migrated) busy during incoming migration. Signed-off-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Stefan Hajnoczi Message-Id: <20210322094906.5079-2-vsementsov@virtuozzo.com> --- 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 975093610a..35f5ef688d 100644 --- a/migration/block-dirty-bitmap.c +++ b/migration/block-dirty-bitmap.c @@ -839,6 +839,8 @@ static int dirty_bitmap_load_start(QEMUFile *f, DBMLoadState *s) error_report_err(local_err); return -EINVAL; } + } else { + bdrv_dirty_bitmap_set_busy(s->bitmap, true); } b = g_new(LoadBitmapState, 1); @@ -914,6 +916,8 @@ static void cancel_incoming_locked(DBMLoadState *s) assert(!s->before_vm_start_handled || !b->migrated); if (bdrv_dirty_bitmap_has_successor(b->bitmap)) { bdrv_reclaim_dirty_bitmap(b->bitmap, &error_abort); + } else { + bdrv_dirty_bitmap_set_busy(b->bitmap, false); } bdrv_release_dirty_bitmap(b->bitmap); } @@ -951,6 +955,8 @@ static void dirty_bitmap_load_complete(QEMUFile *f, DBMLoadState *s) if (bdrv_dirty_bitmap_has_successor(s->bitmap)) { bdrv_reclaim_dirty_bitmap(s->bitmap, &error_abort); + } else { + bdrv_dirty_bitmap_set_busy(s->bitmap, false); } for (item = s->bitmaps; item; item = g_slist_next(item)) { From 3460fd7f3959d1fa7bcc255796844aa261c805a4 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Mon, 22 Mar 2021 12:49:06 +0300 Subject: [PATCH 2/2] migrate-bitmaps-postcopy-test: check that we can't remove in-flight bitmaps Check that we can't remove bitmaps being migrated on destination vm. The new check proves that previous commit helps. Signed-off-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Stefan Hajnoczi Message-Id: <20210322094906.5079-3-vsementsov@virtuozzo.com> --- tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test index d046ebeb94..584062b412 100755 --- a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test +++ b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test @@ -224,6 +224,16 @@ class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase): self.start_postcopy() self.vm_b_events += self.vm_b.get_qmp_events() + + # While being here, let's check that we can't remove in-flight bitmaps. + for vm in (self.vm_a, self.vm_b): + for i in range(0, nb_bitmaps): + result = vm.qmp('block-dirty-bitmap-remove', node='drive0', + name=f'bitmap{i}') + self.assert_qmp(result, 'error/desc', + f"Bitmap 'bitmap{i}' is currently in use by " + "another operation and cannot be used") + self.vm_b.shutdown() # recreate vm_b, so there is no incoming option, which prevents # loading bitmaps from disk