At Mon, 17 Dec 2012 13:22:31 +0800,
Liu Yuan wrote:
>
> On 12/17/2012 11:43 AM, MORITA Kazutaka wrote:
> > send_pending_req() needs to be called even in error case. Rather than
> > moving the error check, I think it looks better to update
> > s->inode.data_vdi_id only when rsp.result is SD_RES_SUCCESS.
>
> Why can't we check the rsp.result in the first place? double check
> rsp.result inside one function looks a little bit complicated to me.
>
> How about
> + if (rsp.result != SD_RES_SUCCESS) {
> + acb->ret = -EIO;
> + error_report("%s", sd_strerror(rsp.result));
> + send_pending_req(s, aio_req->oid);
> + goto err;
> +
> + }
Either is okay, but "s->co_recv = NULL" is necessary before
send_pending_req() because we can receive another response while
sending pending requests. In addition, it is better to call
send_pending_req() only when we update inode; in most cases, we don't
need to traverse pending list.
>
> By the way, seems that
> send_pending_req(s, vid_to_data_oid(s->inode.vdi_id, idx));
> can be reduced to
> send_pending_req(s, aio_req->oid);
>
> If yes, I can send another patch to fix.
Looks correct.
Thanks,
Kazutaka
