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

[Avi Kivity]

On 06/23/2009 07:50 PM, Christoph Hellwig wrote:
> 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.
>    

Just starting qemu with a vmdk image hangs.  I think the very first read 
triggers it.

> > --- 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?
>    

Maybe, but that's a bigger change.  Let's start with this (in stable- 
too) and rework aio later.

--
error compiling committee.c: too many arguments to function