Re: [Qemu-devel] [RFC PATCH 1/9] block: Add bdrv_aio_cancel_async

[Fam Zheng]

On Thu, 08/21 14:14, Paolo Bonzini wrote:
> Il 21/08/2014 13:56, Fam Zheng ha scritto:
> > +        BlockDriverAIOCB *save = g_new(BlockDriverAIOCB, 1);
> > +        save->cb               = acb->cb;
> > +        save->opaque           = acb->opaque;
> > +        acb->cb                = bdrv_aio_cancel_async_cb;
> > +        acb->opaque            = acb;
> > +        acb->cancel_acb_save   = save;
> 
> No need for this extra field.  You can make a struct with {acb->cb,
> acb->opaque, acb} and pass it as the opaque.  You also do not need to
> allocate it on the heap, since everything is synchronous.
> 
> > +
> > +        /* Use the synchronous version and hope our cb is called. */
> > +        acb->aiocb_info->cancel(acb);
> 
> Unfortunately, acb has been freed at this point

Oops!

> , so you'll be accessing
> a dangling pointer.  I think you need to modify all cancel
> implementations.  For example:
> 
> - return whether they have called the callback
> 
> - only free the acb if they have called the callback
> 
> - otherwise, free the acb in bdrv_aio_cancel

What about we save cb and opaque locally, and set acb->cb to a nop. When cancel
is done we can call the original cb?

Fam