On Wed, Feb 28, 2018 at 07:04:53PM +0100, Max Reitz wrote:
> Draining a BDS (in the main loop) may cause it to go be deleted. That
> is rather suboptimal if we still plan to access it afterwards, so let us
> enclose the main body of the function with a bdrv_ref()/bdrv_unref()
> pair.
>
> Signed-off-by: Max Reitz <mre...@redhat.com>
> ---
> block/io.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/block/io.c b/block/io.c
> index ead12c4136..3b61e26114 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -294,12 +294,27 @@ void bdrv_do_drained_begin(BlockDriverState *bs, bool
> recursive,
> BdrvChild *parent)
> {
> BdrvChild *child, *next;
> + bool in_main_loop =
> + qemu_get_current_aio_context() == qemu_get_aio_context();
> + /* bdrv_close() invokes bdrv_drain() with bs->refcnt == 0; then,
> + * we may not invoke bdrv_ref()/bdrv_unref() because the latter
> + * would result in the refcount going back to 0, creating an
> + * infinite loop.
> + * Also, we have to be in the main loop because we may not call
> + * bdrv_unref() elsewhere. But because of that, the BDS is not in
> + * danger of going away without the bdrv_ref()/bdrv_unref() pair
> + * elsewhere, so we are fine then. */
> + bool add_ref = in_main_loop && bs->refcnt > 0;
>
> if (qemu_in_coroutine()) {
> bdrv_co_yield_to_drain(bs, true, recursive, parent);
> return;
> }
>
> + if (add_ref) {
> + bdrv_ref(bs);
> + }
> +
> /* Stop things in parent-to-child order */
> if (atomic_fetch_inc(&bs->quiesce_counter) == 0) {
> aio_disable_external(bdrv_get_aio_context(bs));
> @@ -315,6 +330,10 @@ void bdrv_do_drained_begin(BlockDriverState *bs, bool
> recursive,
> bdrv_do_drained_begin(child->bs, true, child);
> }
> }
> +
> + if (add_ref) {
> + bdrv_unref(bs);
> + }
> }
>
> void bdrv_drained_begin(BlockDriverState *bs)
> --
> 2.14.3
>
>
Reviewed-by: Jeff Cody <jc...@redhat.com>
