Re: [Qemu-devel] [PATCH v11 01/19] block: Add bdrv_drain_all_{begin, end}()

[Paolo Bonzini]

On 14/10/2016 15:08, Alberto Garcia wrote:
> bdrv_drain_all() doesn't allow the caller to do anything after all
> pending requests have been completed but before block jobs are
> resumed.
> 
> This patch splits bdrv_drain_all() into _begin() and _end() for that
> purpose. It also adds aio_{disable,enable}_external() calls to disable
> external clients in the meantime.
> 
> Signed-off-by: Alberto Garcia <address@hidden>
> ---
>  block/io.c            | 24 +++++++++++++++++++++---
>  include/block/block.h |  2 ++
>  2 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index b136c89..9418f8b 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -275,8 +275,11 @@ void bdrv_drain(BlockDriverState *bs)
>   *
>   * This function does not flush data to disk, use bdrv_flush_all() for that
>   * after calling this function.
> + *
> + * This pauses all block jobs and disables external clients. It must
> + * be paired with bdrv_drain_all_end().
>   */
> -void bdrv_drain_all(void)
> +void bdrv_drain_all_begin(void)
>  {
>      /* Always run first iteration so any pending completion BHs run */
>      bool busy = true;
> @@ -300,6 +303,7 @@ void bdrv_drain_all(void)
>          bdrv_parent_drained_begin(bs);
>          bdrv_io_unplugged_begin(bs);
>          bdrv_drain_recurse(bs);
> +        aio_disable_external(aio_context);
>          aio_context_release(aio_context);
>  
>          if (!g_slist_find(aio_ctxs, aio_context)) {
> @@ -333,17 +337,25 @@ void bdrv_drain_all(void)
>          }
>      }
>  
> +    g_slist_free(aio_ctxs);
> +}
> +
> +void bdrv_drain_all_end(void)
> +{
> +    BlockDriverState *bs;
> +    BdrvNextIterator it;
> +    BlockJob *job = NULL;
> +
>      for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
>          AioContext *aio_context = bdrv_get_aio_context(bs);
>  
>          aio_context_acquire(aio_context);
> +        aio_enable_external(aio_context);
>          bdrv_io_unplugged_end(bs);
>          bdrv_parent_drained_end(bs);
>          aio_context_release(aio_context);
>      }
> -    g_slist_free(aio_ctxs);
>  
> -    job = NULL;
>      while ((job = block_job_next(job))) {
>          AioContext *aio_context = blk_get_aio_context(job->blk);
>  
> @@ -353,6 +365,12 @@ void bdrv_drain_all(void)
>      }
>  }
>  
> +void bdrv_drain_all(void)
> +{
> +    bdrv_drain_all_begin();
> +    bdrv_drain_all_end();
> +}
> +
>  /**
>   * Remove an active request from the tracked requests list
>   *
> diff --git a/include/block/block.h b/include/block/block.h
> index 107c603..301d713 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -338,6 +338,8 @@ int bdrv_flush_all(void);
>  void bdrv_close_all(void);
>  void bdrv_drain(BlockDriverState *bs);
>  void coroutine_fn bdrv_co_drain(BlockDriverState *bs);
> +void bdrv_drain_all_begin(void);
> +void bdrv_drain_all_end(void);
>  void bdrv_drain_all(void);
>  
>  int bdrv_pdiscard(BlockDriverState *bs, int64_t offset, int count);
> 

Reviewed-by: Paolo Bonzini <address@hidden>