Message ID | 20230731203338.27581-1-farosas@suse.de |
---|---|
State | New |
Headers | show |
Series | block-migration: Ensure we don't crash during migration cleanup | expand |
added Kevin and Hanna for block, since this seems still untouched? Thanks, Claudio On 7/31/23 22:33, Fabiano Rosas wrote: > 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 > > Signed-off-by: Fabiano Rosas <farosas@suse.de> > --- > migration/block.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > 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. */
On Mon, Jul 31, 2023 at 05:33:38PM -0300, Fabiano Rosas wrote: > 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 > > Signed-off-by: Fabiano Rosas <farosas@suse.de> > --- > migration/block.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) Sorry, I missed this patch! If this needs to be in QEMU 8.1 (-rc3 is being tagged today), please reply and provide a justification. At this point only security fixes and showstoppers will be merged. Thanks! Applied to my block-next tree for QEMU 8.2: https://gitlab.com/stefanha/qemu/commits/block-next Stefan
On 8/8/23 19:08, Stefan Hajnoczi wrote: > On Mon, Jul 31, 2023 at 05:33:38PM -0300, Fabiano Rosas wrote: >> 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 >> >> Signed-off-by: Fabiano Rosas <farosas@suse.de> >> --- >> migration/block.c | 11 +++++++++-- >> 1 file changed, 9 insertions(+), 2 deletions(-) > > Sorry, I missed this patch! > > If this needs to be in QEMU 8.1 (-rc3 is being tagged today), please > reply and provide a justification. At this point only security fixes and > showstoppers will be merged. Thanks! > > Applied to my block-next tree for QEMU 8.2: > https://gitlab.com/stefanha/qemu/commits/block-next > > Stefan Thanks, and in my personal view I think it's ok for 8.2, IIUC it happens during the migration to file work which is not in 8.1 anyway, Fabiano correct me here if I am wrong, Ciao, Claudio
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. */
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 Signed-off-by: Fabiano Rosas <farosas@suse.de> --- migration/block.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)