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

[Fam Zheng]

On Fri, 08/22 12:10, Paolo Bonzini wrote:
> Il 22/08/2014 11:37, Fam Zheng ha scritto:
> > Exactly. I'd rather not change the contract then.
> > 
> > Alternatively, we may add a refcnt field to BlockDriverAioCB and grab one 
> > before
> > calling .cancel, so the qemu_aio_release will not free it.
> 
> Yes, and I don't exclude that sooner or later we'll have to add
> reference counts to AIOCB anyway.  However, reference counting is not
> _that_ cheap so for now I'd rather see other solutions explored.

Why doesn it have an performance effect? Just because of the would-be
"if (likely(--acb->refcnt == 0))" testing?

> 
> The problem is implementing cancel_sync in terms of cancel.  The
> simplest solution, for now, is to make bdrv_aio_cancel_async return
> false if the callback is not implemented, and fall back to synchronous
> cancellation.

This does keep the code change local, but not necessarily simple, since there
would be two cancelling code paths in scsi-bus. I already find the scsi req
ref/unref pairs a bit hard to track.

I prefer that we change the implementation to avoid complicating interface:
don't call qemu_aio_release in .cancel, but call it in
bdrv_aio_cancel{,_async} after calling .cancel(). Does that work?

Fam