[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH v1 1/3] gluster: Convert aio routines into c
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [RFC PATCH v1 1/3] gluster: Convert aio routines into coroutines |
Date: |
Mon, 16 Dec 2013 17:33:48 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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