Message ID | 20230830114942.449060-2-stefanha@redhat.com |
---|---|
State | New |
Headers | show |
Series | [PULL,v3,1/5] block-migration: Ensure we don't crash during migration cleanup | expand |
30.08.2023 14:49, Stefan Hajnoczi wrote: > From: Fabiano Rosas <farosas@suse.de> > > We can fail the blk_insert_bs() at init_blk_migration(), leaving the > BlkMigDevState without a dirty_bitmap and BlockDriverState. Account > for the possibly missing elements when doing cleanup. > > Fix the following crashes: > > Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault. > 0x0000555555ec83ef in bdrv_release_dirty_bitmap (bitmap=0x0) at ../block/dirty-bitmap.c:359 > 359 BlockDriverState *bs = bitmap->bs; > #0 0x0000555555ec83ef in bdrv_release_dirty_bitmap (bitmap=0x0) at ../block/dirty-bitmap.c:359 > #1 0x0000555555bba331 in unset_dirty_tracking () at ../migration/block.c:371 > #2 0x0000555555bbad98 in block_migration_cleanup_bmds () at ../migration/block.c:681 > > Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault. > 0x0000555555e971ff in bdrv_op_unblock (bs=0x0, op=BLOCK_OP_TYPE_BACKUP_SOURCE, reason=0x0) at ../block.c:7073 > 7073 QLIST_FOREACH_SAFE(blocker, &bs->op_blockers[op], list, next) { > #0 0x0000555555e971ff in bdrv_op_unblock (bs=0x0, op=BLOCK_OP_TYPE_BACKUP_SOURCE, reason=0x0) at ../block.c:7073 > #1 0x0000555555e9734a in bdrv_op_unblock_all (bs=0x0, reason=0x0) at ../block.c:7095 > #2 0x0000555555bbae13 in block_migration_cleanup_bmds () at ../migration/block.c:690 This smells like -stable material, is it not? (applies to 7.2, 8.0 and 8.1). /mjt
Michael Tokarev <mjt@tls.msk.ru> writes: > 30.08.2023 14:49, Stefan Hajnoczi wrote: >> From: Fabiano Rosas <farosas@suse.de> >> >> We can fail the blk_insert_bs() at init_blk_migration(), leaving the >> BlkMigDevState without a dirty_bitmap and BlockDriverState. Account >> for the possibly missing elements when doing cleanup. >> >> Fix the following crashes: >> >> Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault. >> 0x0000555555ec83ef in bdrv_release_dirty_bitmap (bitmap=0x0) at ../block/dirty-bitmap.c:359 >> 359 BlockDriverState *bs = bitmap->bs; >> #0 0x0000555555ec83ef in bdrv_release_dirty_bitmap (bitmap=0x0) at ../block/dirty-bitmap.c:359 >> #1 0x0000555555bba331 in unset_dirty_tracking () at ../migration/block.c:371 >> #2 0x0000555555bbad98 in block_migration_cleanup_bmds () at ../migration/block.c:681 >> >> Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault. >> 0x0000555555e971ff in bdrv_op_unblock (bs=0x0, op=BLOCK_OP_TYPE_BACKUP_SOURCE, reason=0x0) at ../block.c:7073 >> 7073 QLIST_FOREACH_SAFE(blocker, &bs->op_blockers[op], list, next) { >> #0 0x0000555555e971ff in bdrv_op_unblock (bs=0x0, op=BLOCK_OP_TYPE_BACKUP_SOURCE, reason=0x0) at ../block.c:7073 >> #1 0x0000555555e9734a in bdrv_op_unblock_all (bs=0x0, reason=0x0) at ../block.c:7095 >> #2 0x0000555555bbae13 in block_migration_cleanup_bmds () at ../migration/block.c:690 > > This smells like -stable material, is it not? > (applies to 7.2, 8.0 and 8.1). Yes, I agree.
diff --git a/migration/block.c b/migration/block.c index b9580a6c7e..86c2256a2b 100644 --- a/migration/block.c +++ b/migration/block.c @@ -368,7 +368,9 @@ static void unset_dirty_tracking(void) BlkMigDevState *bmds; QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) { - bdrv_release_dirty_bitmap(bmds->dirty_bitmap); + if (bmds->dirty_bitmap) { + bdrv_release_dirty_bitmap(bmds->dirty_bitmap); + } } } @@ -676,13 +678,18 @@ static int64_t get_remaining_dirty(void) static void block_migration_cleanup_bmds(void) { BlkMigDevState *bmds; + BlockDriverState *bs; AioContext *ctx; unset_dirty_tracking(); while ((bmds = QSIMPLEQ_FIRST(&block_mig_state.bmds_list)) != NULL) { QSIMPLEQ_REMOVE_HEAD(&block_mig_state.bmds_list, entry); - bdrv_op_unblock_all(blk_bs(bmds->blk), bmds->blocker); + + bs = blk_bs(bmds->blk); + if (bs) { + bdrv_op_unblock_all(bs, bmds->blocker); + } error_free(bmds->blocker); /* Save ctx, because bmds->blk can disappear during blk_unref. */