From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:55404)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <olaf@aepfle.de>) id 1WAkrw-0005uw-P9
	for qemu-devel@nongnu.org; Tue, 04 Feb 2014 13:37:13 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <olaf@aepfle.de>) id 1WAkrv-00021T-UA
	for qemu-devel@nongnu.org; Tue, 04 Feb 2014 13:37:12 -0500
Received: from mo6-p00-ob.smtp.rzone.de ([2a01:238:20a:202:5300::3]:55889)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <olaf@aepfle.de>) id 1WAjQ8-0003zy-5O
	for qemu-devel@nongnu.org; Tue, 04 Feb 2014 12:04:24 -0500
Date: Tue, 4 Feb 2014 17:58:19 +0100
From: Olaf Hering <olaf@aepfle.de>
Message-ID: <20140204165819.GA28066@aepfle.de>
References: <1391094138-26798-1-git-send-email-olaf@aepfle.de>
	<20140203154908.GO3643@dhcp-200-207.str.redhat.com>
	<20140204154737.GA19903@aepfle.de>
	<20140204161523.GQ3384@dhcp-200-207.str.redhat.com>
	<20140204165115.GA27169@aepfle.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20140204165115.GA27169@aepfle.de>
Subject: Re: [Qemu-devel] [PATCH] xen_disk: add discard support
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: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-devel@nongnu.org, stefanha@redhat.com, stefano.stabellini@eu.citrix.com

On Tue, Feb 04, Olaf Hering wrote:

> On Tue, Feb 04, Kevin Wolf wrote:
> 
> > Now you call bdrv_acct_done() in the callback without having a matching
> > bdrv_acct_start(). You need to make it conditional in the callback.

> Stefano,
> Is ioreq_runio_qemu_aio symetric in this regard anyway? In case of
> BLKIF_OP_WRITE|BLKIF_OP_FLUSH_DISKCACHE and no ioreq->req.nr_segments
> then qemu_aio_complete is called anyway. Will qemu_aio_complete get down
> to the bdrv_acct_done call at all in this case?

What I have in mind is something like the (not compile tested) change below.

Olaf

diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index e74efc7..99d36b8 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -486,7 +486,16 @@ static void qemu_aio_complete(void *opaque, int ret)
     ioreq->status = ioreq->aio_errors ? BLKIF_RSP_ERROR : BLKIF_RSP_OKAY;
     ioreq_unmap(ioreq);
     ioreq_finish(ioreq);
-    bdrv_acct_done(ioreq->blkdev->bs, &ioreq->acct);
+    switch (ioreq->req.operation) {
+    case BLKIF_OP_DISCARD:
+        break;
+    case BLKIF_OP_WRITE:
+    case BLKIF_OP_FLUSH_DISKCACHE:
+        if (!ioreq->req.nr_segments) {
+            break;
+        }
+        bdrv_acct_done(ioreq->blkdev->bs, &ioreq->acct);
+    }
     qemu_bh_schedule(ioreq->blkdev->bh);
 }