[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/3] block: guard bdrv_drain in bdrv_close with
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH 3/3] block: guard bdrv_drain in bdrv_close with aio_context_acquire |
Date: |
Fri, 6 Nov 2015 13:44:24 +0000 |
User-agent: |
Mutt/1.5.23 (2015-06-09) |
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
[Qemu-devel] [PATCH 2/3] blockdev: acquire AioContext in hmp_commit(), Denis V. Lunev, 2015/11/04