From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:54354) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RStHj-00059K-8N for qemu-devel@nongnu.org; Tue, 22 Nov 2011 11:33:32 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RStHb-0004Hg-QC for qemu-devel@nongnu.org; Tue, 22 Nov 2011 11:33:26 -0500 Received: from mx1.redhat.com ([209.132.183.28]:18852) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RStHb-0004GE-DM for qemu-devel@nongnu.org; Tue, 22 Nov 2011 11:33:19 -0500 Message-ID: <4ECBCF83.6090701@redhat.com> Date: Tue, 22 Nov 2011 17:36:19 +0100 From: Kevin Wolf MIME-Version: 1.0 References: <1321537232-799-1-git-send-email-stefanha@linux.vnet.ibm.com> <1321537232-799-7-git-send-email-stefanha@linux.vnet.ibm.com> <4ECBBA5D.1080006@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 6/8] block: request overlap detection List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Paolo Bonzini , Marcelo Tosatti , Stefan Hajnoczi , qemu-devel@nongnu.org Am 22.11.2011 16:10, schrieb Stefan Hajnoczi: > On Tue, Nov 22, 2011 at 3:06 PM, Kevin Wolf wrote: >> Am 17.11.2011 14:40, schrieb Stefan Hajnoczi: >>> Detect overlapping requests and remember to align to cluster boundaries >>> if the image format uses them. This assumes that allocating I/O is >>> performed in cluster granularity - which is true for qcow2, qed, etc. >>> >>> Signed-off-by: Stefan Hajnoczi >> >>> static void coroutine_fn wait_for_overlapping_requests(BlockDriverState *bs, >>> int64_t sector_num, int nb_sectors) >>> { >>> BdrvTrackedRequest *req; >>> + int64_t cluster_sector_num; >>> + int cluster_nb_sectors; >>> bool retry; >>> >>> + /* If we touch the same cluster it counts as an overlap */ >>> + round_to_clusters(bs, sector_num, nb_sectors, >>> + &cluster_sector_num, &cluster_nb_sectors); >> >> Is this really required? Image formats must be able to deal with two >> concurrent write requests to the same cluster, and I don't think it >> makes a difference whether it's a guest write request or a COR one. >> >> Or does the queuing protect more than just that a COR never takes >> precedence over a guest write? > > It guarantees that a write request and a copy-on-read request racing > for the same cluster will be serialized. Either: > 1. CoR, then write. > 2. Allocating write, then normal read. > > If we don't do this we risk: > 3. CoR (part 1: read), allocating write, CoR (part 2: write) > > The result is that we dropped the write request! Right, this requirement comes in with the next patch that makes COR round clusters for optimisation. I think this relationship deserves a comment here. Anyway, I merged the series into the block branch (Only to notice that it would have been better to merge bdrv_co_is_allocated first... Will review that tomorrow.) If you send another version for this patch to add a comment, I'll replace it. Kevin