Re: [Qemu-devel] [PATCH] block: Clean up after deleting BHs

[Christoph Hellwig <hch@lst.de>]

On Tue, Jun 23, 2009 at 04:20:36PM +0300, Avi Kivity wrote:
> Commit 6a7ad299 ("Call qemu_bh_delete at bdrv_aio_bh_cb") deletes emulated
> aio bottom halves to prevent endless accumulation.  However, it leaves a
> stale ->bh pointer, which is then waited on when the aio is reused.
> 
> Zeroing the pointer fixes the issue, allowing vmdk format images to be used.

What operations on vmdk images does this cause to fail?  qemu-iotests
seems to do fine on vmdk so it's nothing yet exercised by it.

> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
>  block.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/block.c b/block.c
> index aca5a6d..cefbe77 100644
> --- a/block.c
> +++ b/block.c
> @@ -1374,6 +1374,7 @@ static void bdrv_aio_cancel_em(BlockDriverAIOCB *blockacb)
>  {
>      BlockDriverAIOCBSync *acb = (BlockDriverAIOCBSync *)blockacb;
>      qemu_bh_delete(acb->bh);
> +    acb->bh = NULL;
>      qemu_aio_release(acb);
>  }
>  
> @@ -1391,6 +1392,7 @@ static void bdrv_aio_bh_cb(void *opaque)
>      qemu_vfree(acb->bounce);
>      acb->common.cb(acb->common.opaque, acb->ret);
>      qemu_bh_delete(acb->bh);
> +    acb->bh = NULL;
>      qemu_aio_release(acb);
>  }

I think not having the state of the private acb area cleared over a
free/realloc cycle is pretty dangerous.  Wouldn't it be better to always
clear that space in qemu_aio_get?