From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KseUQ-0006Xt-VL for qemu-devel@nongnu.org; Wed, 22 Oct 2008 10:15:11 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KseUN-0006XP-EB for qemu-devel@nongnu.org; Wed, 22 Oct 2008 10:15:10 -0400 Received: from [199.232.76.173] (port=34257 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KseUN-0006XG-90 for qemu-devel@nongnu.org; Wed, 22 Oct 2008 10:15:07 -0400 Received: from mx2.redhat.com ([66.187.237.31]:50851) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KseUN-00057x-8g for qemu-devel@nongnu.org; Wed, 22 Oct 2008 10:15:07 -0400 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id m9MEF1qi007236 for ; Wed, 22 Oct 2008 10:15:03 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id m9MEF0LY013605 for ; Wed, 22 Oct 2008 10:15:01 -0400 Received: from duo.random (vpn-4-153.str.redhat.com [10.32.4.153]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id m9MEExUj026148 for ; Wed, 22 Oct 2008 10:15:00 -0400 Date: Wed, 22 Oct 2008 16:14:59 +0200 From: Andrea Arcangeli Message-ID: <20081022141459.GA26426@duo.random> References: <20080829135249.GI24884@duo.random> <20080901104356.GD25764@duo.random> <20080901105314.GE25764@duo.random> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20080901105314.GE25764@duo.random> Subject: [Qemu-devel] [PATCH] fix bdrv_aio_read API breakage in qcow2 Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org From: Andrea Arcangeli I noticed the qemu_aio_flush was doing nothing at all. And a flood of cmd_writeb commands leading to a noop-invocation of qemu_aio_flush were executed. In short all 'memset;goto redo' places must be fixed to use the bh and not to call the callback in the context of bdrv_aio_read or the bdrv_aio_read model falls apart. Reading from qcow2 holes is possible with phyisical readahead (kind of breada in linux buffer cache). This is needed at least for scsi, ide is lucky (or it has been band-aided against this API breakage by fixing the symptom and not the real bug). Same bug exists in qcow of course, can be fixed later as it's less urgent. Signed-off-by: Andrea Arcangeli -- Index: block-qcow2.c =================================================================== --- block-qcow2.c (revision 5506) +++ block-qcow2.c (working copy) @@ -1165,8 +1165,18 @@ uint64_t cluster_offset; uint8_t *cluster_data; BlockDriverAIOCB *hd_aiocb; + QEMUBH *bh; } QCowAIOCB; +static void qcow_aio_read_cb(void *opaque, int ret); +static void qcow_aio_read_bh(void *opaque) +{ + QCowAIOCB *acb = opaque; + qemu_bh_delete(acb->bh); + acb->bh = NULL; + qcow_aio_read_cb(opaque, 0); +} + static void qcow_aio_read_cb(void *opaque, int ret) { QCowAIOCB *acb = opaque; @@ -1182,7 +1192,6 @@ return; } - redo: /* post process the read buffer */ if (!acb->cluster_offset) { /* nothing to do */ @@ -1223,12 +1232,30 @@ if (acb->hd_aiocb == NULL) goto fail; } else { - goto redo; + if (acb->bh) { + ret = -EIO; + goto fail; + } + acb->bh = qemu_bh_new(qcow_aio_read_bh, acb); + if (!acb->bh) { + ret = -EIO; + goto fail; + } + qemu_bh_schedule(acb->bh); } } else { /* Note: in this case, no need to wait */ memset(acb->buf, 0, 512 * acb->n); - goto redo; + if (acb->bh) { + ret = -EIO; + goto fail; + } + acb->bh = qemu_bh_new(qcow_aio_read_bh, acb); + if (!acb->bh) { + ret = -EIO; + goto fail; + } + qemu_bh_schedule(acb->bh); } } else if (acb->cluster_offset & QCOW_OFLAG_COMPRESSED) { /* add AIO support for compressed blocks ? */ @@ -1236,7 +1263,16 @@ goto fail; memcpy(acb->buf, s->cluster_cache + index_in_cluster * 512, 512 * acb->n); - goto redo; + if (acb->bh) { + ret = -EIO; + goto fail; + } + acb->bh = qemu_bh_new(qcow_aio_read_bh, acb); + if (!acb->bh) { + ret = -EIO; + goto fail; + } + qemu_bh_schedule(acb->bh); } else { if ((acb->cluster_offset & 511) != 0) { ret = -EIO;