[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 03/19] block: Make bdrv_drain() driver callbacks
From: |
Fam Zheng |
Subject: |
Re: [Qemu-block] [PATCH 03/19] block: Make bdrv_drain() driver callbacks non-recursive |
Date: |
Wed, 20 Dec 2017 21:16:18 +0800 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
On Wed, 12/20 11:33, Kevin Wolf wrote:
> bdrv_drain_begin() doesn't increase bs->quiesce_counter recursively,
Pretty trivial but:
s/bdrv_drain_begin/bdrv_drained_begin/
> which means that the child nodes are not actually drained. To keep this
> consistent, we also shouldn't call the block driver callbacks.
>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
> block/io.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/block/io.c b/block/io.c
> index b94740b8ff..91a52e2d82 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -158,7 +158,7 @@ static void coroutine_fn bdrv_drain_invoke_entry(void
> *opaque)
> }
>
> /* Recursively call BlockDriver.bdrv_co_drain_begin/end callbacks */
> -static void bdrv_drain_invoke(BlockDriverState *bs, bool begin)
> +static void bdrv_drain_invoke(BlockDriverState *bs, bool begin, bool
> recursive)
> {
> BdrvChild *child, *tmp;
> BdrvCoDrainData data = { .bs = bs, .done = false, .begin = begin};
> @@ -172,8 +172,10 @@ static void bdrv_drain_invoke(BlockDriverState *bs, bool
> begin)
> bdrv_coroutine_enter(bs, data.co);
> BDRV_POLL_WHILE(bs, !data.done);
>
> - QLIST_FOREACH_SAFE(child, &bs->children, next, tmp) {
> - bdrv_drain_invoke(child->bs, begin);
> + if (recursive) {
> + QLIST_FOREACH_SAFE(child, &bs->children, next, tmp) {
> + bdrv_drain_invoke(child->bs, begin, true);
> + }
> }
> }
>
> @@ -265,7 +267,7 @@ void bdrv_drained_begin(BlockDriverState *bs)
> bdrv_parent_drained_begin(bs);
> }
>
> - bdrv_drain_invoke(bs, true);
> + bdrv_drain_invoke(bs, true, false);
I'm not sure I understand this patch. If we call bdrv_drained_begin on a quorum
BDS that has a QED child, we do want to call bdrv_qed_co_drain_begin() on the
child, don't we? I think after this patch, we don't do that any more?
The same question arises when I look at throttle_co_drain_begin()..
Fam
- [Qemu-block] [PATCH 00/19] Drain fixes and cleanups, part 2, Kevin Wolf, 2017/12/20
- [Qemu-block] [PATCH 04/19] test-bdrv-drain: Test callback for bdrv_drain, Kevin Wolf, 2017/12/20
- [Qemu-block] [PATCH 05/19] test-bdrv-drain: Test bs->quiesce_counter, Kevin Wolf, 2017/12/20
- [Qemu-block] [PATCH 07/19] test-bdrv-drain: Test drain vs. block jobs, Kevin Wolf, 2017/12/20
- [Qemu-block] [PATCH 08/19] block: Don't block_job_pause_all() in bdrv_drain_all(), Kevin Wolf, 2017/12/20
- [Qemu-block] [PATCH 06/19] blockjob: Pause job on draining any job BDS, Kevin Wolf, 2017/12/20
- [Qemu-block] [PATCH 09/19] block: Nested drain_end must still call callbacks, Kevin Wolf, 2017/12/20
- [Qemu-block] [PATCH 11/19] block: Don't notify parents in drain call chain, Kevin Wolf, 2017/12/20