From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:45434)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <namei.unix@gmail.com>) id 1TkT9z-0006np-TK
	for qemu-devel@nongnu.org; Mon, 17 Dec 2012 00:22:41 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <namei.unix@gmail.com>) id 1TkT9y-0000Ww-T9
	for qemu-devel@nongnu.org; Mon, 17 Dec 2012 00:22:39 -0500
Received: from mail-pa0-f45.google.com ([209.85.220.45]:43168)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <namei.unix@gmail.com>) id 1TkT9y-0000W1-Mr
	for qemu-devel@nongnu.org; Mon, 17 Dec 2012 00:22:38 -0500
Received: by mail-pa0-f45.google.com with SMTP id bg2so3610882pad.4
	for <qemu-devel@nongnu.org>; Sun, 16 Dec 2012 21:22:37 -0800 (PST)
Message-ID: <50CEAC17.8040905@gmail.com>
Date: Mon, 17 Dec 2012 13:22:31 +0800
From: Liu Yuan <namei.unix@gmail.com>
MIME-Version: 1.0
References: <1355565174-5121-1-git-send-email-namei.unix@gmail.com>
	<1355565552-18346-1-git-send-email-namei.unix@gmail.com>
	<87obhtjqd3.wl%morita.kazutaka@lab.ntt.co.jp>
In-Reply-To: <87obhtjqd3.wl%morita.kazutaka@lab.ntt.co.jp>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH Update] sheepdog: don't update inode when
 create_and_write fails
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Cc: Kevin Wolf <kwolf@redhat.com>, qemu-devel@nongnu.org

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;
+
+    }

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.

Thanks,
Yuan