From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:54353) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T6LnN-0000u7-BR for qemu-devel@nongnu.org; Tue, 28 Aug 2012 09:25:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T6LnH-0001ki-Bv for qemu-devel@nongnu.org; Tue, 28 Aug 2012 09:25:29 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40641) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T6LnH-0001kQ-1R for qemu-devel@nongnu.org; Tue, 28 Aug 2012 09:25:23 -0400 Message-ID: <503CC6BC.2030101@redhat.com> Date: Tue, 28 Aug 2012 15:25:16 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1346159067-28734-1-git-send-email-stefanha@linux.vnet.ibm.com> In-Reply-To: <1346159067-28734-1-git-send-email-stefanha@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH for-1.2] qed: refuse unaligned zero writes with a backing file List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , Anthony Liguori , qemu-devel@nongnu.org Il 28/08/2012 15:04, Stefan Hajnoczi ha scritto: > Zero writes have cluster granularity in QED. Therefore they can only be > used to zero entire clusters. > > If the zero write request leaves sectors untouched, zeroing the entire > cluster would obscure the backing file. Instead return -ENOTSUP, which > is handled by block.c:bdrv_co_do_write_zeroes() and falls back to a > regular write. > > The qemu-iotests 034 test cases covers this scenario. Reviewed-by: Paolo Bonzini Makes sense since both streaming and copy-on-read will do cluster-aligned writes. The "right fix" would not be much more complex though, something like this, right? (untested). diff --git a/block/qed.c b/block/qed.c index a02dbfd..a885671 100644 --- a/block/qed.c +++ b/block/qed.c @@ -1133,8 +1133,14 @@ static void qed_aio_write_alloc(QEDAIOCB *acb, size_t len) return; } + if (qed_offset_into_cluster(s, acb->cur_pos) != 0 || + qed_offset_into_cluster(s, acb->cur_pos + acb->cur_qiov.size) != 0) { + goto copy; + } + cb = qed_aio_write_zero_cluster; } else { +copy: cb = qed_aio_write_prefill; acb->cur_cluster = qed_alloc_clusters(s, acb->cur_nclusters); } Paolo