The following changes since commit 67c1115edd98f388ca89dd38322ea3fadf034523:

Merge remote-tracking branch 'remotes/kraxel/tags/ui-20210323-pull-request' into staging (2021-03-23 23:47:30 +0000)

for you to fetch changes up to 3460fd7f3959d1fa7bcc255796844aa261c805a4:

migrate-bitmaps-postcopy-test: check that we can't remove in-flight bitmaps (2021-03-24 13:41:19 +0000)

This dirty bitmap fix solves a crash that can be triggered in the destination

QEMU process during live migration.

Vladimir Sementsov-Ogievskiy (2):

migration/block-dirty-bitmap: make incoming disabled bitmaps busy

migrate-bitmaps-postcopy-test: check that we can't remove in-flight

bitmaps

migration/block-dirty-bitmap.c | 6 ++++++

tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test | 10 ++++++++++

2 files changed, 16 insertions(+)

2.30.2

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

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 <vsementsov@virtuozzo.com>

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

--- a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test

+++ b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test

@@ -XXX,XX +XXX,XX @@ 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

2.30.2

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

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 <dbm_state+40>)

at ../migration/block-dirty-bitmap.c:918

#4 dirty_bitmap_load (f=0x559681d02b10, opaque=0x559680be81e0

<dbm_state>, 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 <vsementsov@virtuozzo.com>

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

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

--- a/migration/block-dirty-bitmap.c

+++ b/migration/block-dirty-bitmap.c

@@ -XXX,XX +XXX,XX @@ 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);

@@ -XXX,XX +XXX,XX @@ 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);

}

@@ -XXX,XX +XXX,XX @@ 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)) {

2.30.2