Re: [Qemu-devel] [PATCH 3/3] block: guard bdrv_drain in bdrv_close with aio_context_acquire

[Stefan Hajnoczi]

On Wed, Nov 04, 2015 at 08:27:24PM +0300, Denis V. Lunev wrote:
> bdrv_close is called in tooooo much places to properly track at the moment.

bdrv_close() is called in 5 places.  Let's figure out what the callers
are doing wrong:

block.c:bdrv_open_inherit() - internal function, guaranteed to be safe
block.c:bdrv_close_all() - takes AioContext
block.c:bdrv_delete() - always called from main loop, after IOThreads
                        have stopped accessing BDS
blockdev.c:eject_device() - takes AioContext
blockdev.c:hmp_drive_del() - takes AioContext

I don't see a reason why adding this acquire/release to bdrv_close()
changes behavior or fixes a bug.

Can you explain why this is necessary?

> Signed-off-by: Denis V. Lunev <address@hidden>
> CC: Stefan Hajnoczi <address@hidden>
> ---
>  block.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/block.c b/block.c
> index e9f40dc..98b0b66 100644
> --- a/block.c
> +++ b/block.c
> @@ -1895,6 +1895,7 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state)
>  void bdrv_close(BlockDriverState *bs)
>  {
>      BdrvAioNotifier *ban, *ban_next;
> +    AioContext *ctx;
>  
>      if (bs->job) {
>          block_job_cancel_sync(bs->job);
> @@ -1905,9 +1906,13 @@ void bdrv_close(BlockDriverState *bs)
>          bdrv_io_limits_disable(bs);
>      }
>  
> +    ctx = bdrv_get_aio_context(bs);
> +    aio_context_acquire(ctx);
>      bdrv_drain(bs); /* complete I/O */
>      bdrv_flush(bs);
>      bdrv_drain(bs); /* in case flush left pending I/O */
> +    aio_context_release(ctx);
> +
>      notifier_list_notify(&bs->close_notifiers, bs);
>  
>      if (bs->blk) {
> -- 
> 2.5.0
> 
>

signature.asc
Description: PGP signature