Re: [Qemu-devel] [RFC PATCH v1 1/3] gluster: Convert aio routines into coroutines

[Stefan Hajnoczi <stefanha@gmail.com>]

On Thu, Dec 05, 2013 at 04:31:26PM +0530, Bharata B Rao wrote:
> -static void qemu_gluster_complete_aio(GlusterAIOCB *acb, BDRVGlusterState *s)
> +static void qemu_gluster_complete_aio(void *opaque)
>  {
> -    int ret;
> -    bool *finished = acb->finished;
> -    BlockDriverCompletionFunc *cb = acb->common.cb;
> -    void *opaque = acb->common.opaque;
> -
> -    if (!acb->ret || acb->ret == acb->size) {
> -        ret = 0; /* Success */
> -    } else if (acb->ret < 0) {
> -        ret = acb->ret; /* Read/Write failed */
> -    } else {
> -        ret = -EIO; /* Partial read/write - fail it */
> -    }
[...]
> +    if (acb->ret == acb->size) {
> +        acb->ret = 0;
> +    } else if (acb->ret > 0) {
> +        acb->ret = -EIO; /* Partial read/write - fail it */
>      }

This change is a little ugly since qemu_gluster_complete_aio() now
modifies acb->ret in-place.  I suggest moving the if statements down
into gluster_finish_aiocb() where we first receive the request's return
value.  Then qemu_gluster_complete_aio() simply enters the coroutine and
doesn't modify acb->ret.

>  static const AIOCBInfo gluster_aiocb_info = {
>      .aiocb_size = sizeof(GlusterAIOCB),
> -    .cancel = qemu_gluster_aio_cancel,
>  };

At this point using BlockDriverAIOCB and qemu_aio_get() becomes
questionable.  We no longer implement .cancel() because we don't need
the aio interface.

It would be cleaner to manage our own request struct and allocate using
g_slice_new()/g_slice_free().  That way we don't "reuse"
BlockDriverAIOCB without fully implementing the AIOCBInfo interface.

Stefan