Re: [Qemu-devel] [PATCH 03/19] block: Make bdrv_drain() driver callbacks non-recursive

[Fam Zheng <famz@redhat.com>]

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 <kwolf@redhat.com>
> ---
>  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