On Thu, May 25, 2023 at 02:47:07PM +0200, Kevin Wolf wrote: > qcow2_open() doesn't work correctly when opening the 'file' child moves > bs to an iothread, for several reasons: > > - It uses BDRV_POLL_WHILE() to wait for the qcow2_open_entry() > coroutine, which involves dropping the AioContext lock for bs when it > is not in the main context - but we don't hold it, so this crashes. > > - It runs the qcow2_open_entry() coroutine in the current thread instead > of the new AioContext of bs. > > - qcow2_open_entry() doesn't notify the main loop when it's done. > > This patches fixes these issues around delegating work to a coroutine. > Temporarily dropping the main AioContext lock is not necessary because > we know we run in the main thread. > > Signed-off-by: Kevin Wolf <[email protected]> > --- > block/qcow2.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/block/qcow2.c b/block/qcow2.c > index b00b4e7575..7f3948360d 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -1904,6 +1904,8 @@ static void coroutine_fn qcow2_open_entry(void *opaque) > qoc->ret = qcow2_do_open(qoc->bs, qoc->options, qoc->flags, true, > qoc->errp); > qemu_co_mutex_unlock(&s->lock); > + > + aio_wait_kick(); > } > > static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, > @@ -1929,8 +1931,10 @@ static int qcow2_open(BlockDriverState *bs, QDict > *options, int flags, > > assert(!qemu_in_coroutine()); > assert(qemu_get_current_aio_context() == qemu_get_aio_context()); > - qemu_coroutine_enter(qemu_coroutine_create(qcow2_open_entry, &qoc)); > - BDRV_POLL_WHILE(bs, qoc.ret == -EINPROGRESS); > + > + aio_co_enter(bdrv_get_aio_context(bs), > + qemu_coroutine_create(qcow2_open_entry, &qoc)); > + AIO_WAIT_WHILE_UNLOCKED(NULL, qoc.ret == -EINPROGRESS);
Want to update the doc comment for bdrv_open_file_child() with a warning that @parent's AioContext can change? Otherwise: Reviewed-by: Stefan Hajnoczi <[email protected]>
signature.asc
Description: PGP signature
