| Message ID | 20221118174110.55183-14-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: Simplify drain | expand |
On 11/18/22 20:41, Kevin Wolf wrote: > The next patch adds a parent drain to bdrv_attach_child_common(), which > shouldn't be, but is currently called from coroutines in some cases (e.g. > .bdrv_co_create implementations generally open new nodes). Therefore, > the assertion that we're not in a coroutine doesn't hold true any more. > > We could just remove the assertion because there is nothing in the > function that should be in conflict with running in a coroutine, but > just to be on the safe side, we can reverse the caller relationship > between bdrv_do_drained_begin() and bdrv_do_drained_begin_quiesce() so > that the latter also just drops out of coroutine context and we can > still be certain in the future that any drain code doesn't run in > coroutines. > > As a nice side effect, the structure of bdrv_do_drained_begin() is now > symmetrical with bdrv_do_drained_end(). > > Signed-off-by: Kevin Wolf<kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
diff --git a/block/io.c b/block/io.c index 2e9503df6a..5e9150d92c 100644 --- a/block/io.c +++ b/block/io.c @@ -346,10 +346,15 @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs, } } -void bdrv_do_drained_begin_quiesce(BlockDriverState *bs, BdrvChild *parent) +static void bdrv_do_drained_begin(BlockDriverState *bs, BdrvChild *parent, + bool poll) { IO_OR_GS_CODE(); - assert(!qemu_in_coroutine()); + + if (qemu_in_coroutine()) { + bdrv_co_yield_to_drain(bs, true, parent, poll); + return; + } /* Stop things in parent-to-child order */ if (qatomic_fetch_inc(&bs->quiesce_counter) == 0) { @@ -359,17 +364,6 @@ void bdrv_do_drained_begin_quiesce(BlockDriverState *bs, BdrvChild *parent) bs->drv->bdrv_drain_begin(bs); } } -} - -static void bdrv_do_drained_begin(BlockDriverState *bs, BdrvChild *parent, - bool poll) -{ - if (qemu_in_coroutine()) { - bdrv_co_yield_to_drain(bs, true, parent, poll); - return; - } - - bdrv_do_drained_begin_quiesce(bs, parent); /* * Wait for drained requests to finish. @@ -385,6 +379,11 @@ static void bdrv_do_drained_begin(BlockDriverState *bs, BdrvChild *parent, } } +void bdrv_do_drained_begin_quiesce(BlockDriverState *bs, BdrvChild *parent) +{ + bdrv_do_drained_begin(bs, parent, false); +} + void bdrv_drained_begin(BlockDriverState *bs) { IO_OR_GS_CODE();
The next patch adds a parent drain to bdrv_attach_child_common(), which shouldn't be, but is currently called from coroutines in some cases (e.g. .bdrv_co_create implementations generally open new nodes). Therefore, the assertion that we're not in a coroutine doesn't hold true any more. We could just remove the assertion because there is nothing in the function that should be in conflict with running in a coroutine, but just to be on the safe side, we can reverse the caller relationship between bdrv_do_drained_begin() and bdrv_do_drained_begin_quiesce() so that the latter also just drops out of coroutine context and we can still be certain in the future that any drain code doesn't run in coroutines. As a nice side effect, the structure of bdrv_do_drained_begin() is now symmetrical with bdrv_do_drained_end(). Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/io.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-)