diff mbox

[3/3] block: guard bdrv_drain in bdrv_close with aio_context_acquire

Message ID 1446658044-22042-4-git-send-email-den@openvz.org
State New
Headers show

Commit Message

Denis V. Lunev Nov. 4, 2015, 5:27 p.m. UTC
bdrv_close is called in tooooo much places to properly track at the moment.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Stefan Hajnoczi <stefanha@redhat.com>
---
 block.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Stefan Hajnoczi Nov. 6, 2015, 1:44 p.m. UTC | #1
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 <den@openvz.org>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  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
> 
>
diff mbox

Patch

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) {