On Tue, 29 May 2018 22:19:17 +0200 Kevin Wolf <kw...@redhat.com> wrote:
> Am 28.05.2018 um 14:03 hat Greg Kurz geschrieben: > > Removing a drive with drive_del while it is being used to run an I/O > > intensive workload can cause QEMU to crash. > > > > An AIO flush can yield at some point: > > > > blk_aio_flush_entry() > > blk_co_flush(blk) > > bdrv_co_flush(blk->root->bs) > > ... > > qemu_coroutine_yield() > > > > and let the HMP command to run, free blk->root and give control > > back to the AIO flush: > > > > hmp_drive_del() > > blk_remove_bs() > > bdrv_root_unref_child(blk->root) > > child_bs = blk->root->bs > > bdrv_detach_child(blk->root) > > bdrv_replace_child(blk->root, NULL) > > blk->root->bs = NULL > > g_free(blk->root) <============== blk->root becomes stale > > bdrv_unref(child_bs) > > bdrv_delete(child_bs) > > bdrv_close() > > bdrv_drained_begin() > > bdrv_do_drained_begin() > > bdrv_drain_recurse() > > aio_poll() > > ... > > qemu_coroutine_switch() > > > > and the AIO flush completion ends up dereferencing blk->root: > > > > blk_aio_complete() > > scsi_aio_complete() > > blk_get_aio_context(blk) > > bs = blk_bs(blk) > > ie, bs = blk->root ? blk->root->bs : NULL > > ^^^^^ > > stale > > > > The problem is that we should avoid making block driver graph > > changes while we have in-flight requests. Let's drain all I/O > > for this BB before calling bdrv_root_unref_child(). > > > > Signed-off-by: Greg Kurz <gr...@kaod.org> > > Hmm... It sounded convincing, but 'make check-tests/test-replication' > fails now. The good news is that with the drain fixes, for which I sent > v2 today, it passes, so instead of staging it in my block branch, I'll > put it at the end of my branch for the drain fixes. > > Might take a bit longer than planned until it's in master, sorry. > > Kevin Works for me :) Thanks ! -- Greg