OpenE2K/qemu-e2k
13
4
Fork 0
Code Issues 4 Pull Requests Projects 2 Releases Activity

block-backend: split blk_do_set_aio_context()

Browse Source 
blk_set_aio_context() is not fully transactional because
blk_do_set_aio_context() updates blk->ctx outside the transaction. Most
of the time this goes unnoticed but a BlockDevOps.drained_end() callback
that invokes blk_get_aio_context() fails assert(ctx == blk->ctx). This
happens because blk->ctx is only assigned after
BlockDevOps.drained_end() is called and we're in an intermediate state
where BlockDrvierState nodes already have the new context and the
BlockBackend still has the old context.

Making blk_set_aio_context() fully transactional solves this assertion
failure because the BlockBackend's context is updated as part of the
transaction (before BlockDevOps.drained_end() is called).

Split blk_do_set_aio_context() in order to solve this assertion failure.
This helper function actually serves two different purposes:
1. It drives blk_set_aio_context().
2. It responds to BdrvChildClass->change_aio_ctx().

Get rid of the helper function. Do #1 inside blk_set_aio_context() and
do #2 inside blk_root_set_aio_ctx_commit(). This simplifies the code.

The only drawback of the fully transactional approach is that
blk_set_aio_context() must contend with blk_root_set_aio_ctx_commit()
being invoked as part of the AioContext change propagation. This can be
solved by temporarily setting blk->allow_aio_context_change to true.

Future patches call blk_get_aio_context() from
BlockDevOps->drained_end(), so this patch will become necessary.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230516190238.8401-2-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This commit is contained in:
Stefan Hajnoczi 2023-05-16 15:02:19 -04:00 committed by Kevin Wolf
parent 80e7faaac6
commit 2d19629581
1 changed files with 28 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

71
block/block-backend.c
View File

@ -2433,52 +2433,31 @@ static AioContext *blk_aiocb_get_aio_context(BlockAIOCB *acb)
return blk_get_aio_context(blk_acb->blk);
}
static int blk_do_set_aio_context(BlockBackend *blk, AioContext *new_context,
bool update_root_node, Error **errp)
{
BlockDriverState *bs = blk_bs(blk);
ThrottleGroupMember *tgm = &blk->public.throttle_group_member;
int ret;
if (bs) {
bdrv_ref(bs);
if (update_root_node) {
/*
* update_root_node MUST be false for blk_root_set_aio_ctx_commit(),
* as we are already in the commit function of a transaction.
*/
ret = bdrv_try_change_aio_context(bs, new_context, blk->root, errp);
if (ret < 0) {
bdrv_unref(bs);
return ret;
}
}
/*
* Make blk->ctx consistent with the root node before we invoke any
* other operations like drain that might inquire blk->ctx
*/
blk->ctx = new_context;
if (tgm->throttle_state) {
bdrv_drained_begin(bs);
throttle_group_detach_aio_context(tgm);
throttle_group_attach_aio_context(tgm, new_context);
bdrv_drained_end(bs);
}
bdrv_unref(bs);
} else {
blk->ctx = new_context;
}
return 0;
}
int blk_set_aio_context(BlockBackend *blk, AioContext *new_context,
Error **errp)
{
bool old_allow_change;
BlockDriverState *bs = blk_bs(blk);
int ret;
GLOBAL_STATE_CODE();
return blk_do_set_aio_context(blk, new_context, true, errp);
if (!bs) {
blk->ctx = new_context;
return 0;
}
bdrv_ref(bs);
old_allow_change = blk->allow_aio_context_change;
blk->allow_aio_context_change = true;
ret = bdrv_try_change_aio_context(bs, new_context, NULL, errp);
blk->allow_aio_context_change = old_allow_change;
bdrv_unref(bs);
return ret;
}
typedef struct BdrvStateBlkRootContext {
@ -2490,8 +2469,14 @@ static void blk_root_set_aio_ctx_commit(void *opaque)
{
BdrvStateBlkRootContext *s = opaque;
BlockBackend *blk = s->blk;
AioContext *new_context = s->new_ctx;
ThrottleGroupMember *tgm = &blk->public.throttle_group_member;
blk_do_set_aio_context(blk, s->new_ctx, false, &error_abort);
blk->ctx = new_context;
if (tgm->throttle_state) {
throttle_group_detach_aio_context(tgm);
throttle_group_attach_aio_context(tgm, new_context);
}
}
static TransactionActionDrv set_blk_root_context = {