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
