Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
qcow2: Fix open with 'file' in iothread
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 <kwolf@redhat.com>
Message-Id: <20230525124713.401149-7-kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
  • Loading branch information
Kevin Wolf committed May 30, 2023
1 parent 2626d27 commit aa269ff
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 2 deletions.
6 changes: 6 additions & 0 deletions block.c
Expand Up @@ -3646,6 +3646,9 @@ bdrv_open_child_bs(const char *filename, QDict *options, const char *bdref_key,
* BlockdevRef.
*
* The BlockdevRef will be removed from the options QDict.
*
* @parent can move to a different AioContext in this function. Callers must
* make sure that their AioContext locking is still correct after this.
*/
BdrvChild *bdrv_open_child(const char *filename,
QDict *options, const char *bdref_key,
Expand All @@ -3670,6 +3673,9 @@ BdrvChild *bdrv_open_child(const char *filename,

/*
* Wrapper on bdrv_open_child() for most popular case: open primary child of bs.
*
* @parent can move to a different AioContext in this function. Callers must
* make sure that their AioContext locking is still correct after this.
*/
int bdrv_open_file_child(const char *filename,
QDict *options, const char *bdref_key,
Expand Down
8 changes: 6 additions & 2 deletions block/qcow2.c
Expand Up @@ -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,
Expand All @@ -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);

return qoc.ret;
}
Expand Down

0 comments on commit aa269ff

Please sign in to comment.