From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53067) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XBvmJ-0001YT-JJ for qemu-devel@nongnu.org; Mon, 28 Jul 2014 21:00:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XBvmD-0001R6-72 for qemu-devel@nongnu.org; Mon, 28 Jul 2014 21:00:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58220) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XBvmC-0001R1-TL for qemu-devel@nongnu.org; Mon, 28 Jul 2014 21:00:25 -0400 Date: Tue, 29 Jul 2014 09:00:43 +0800 From: Fam Zheng Message-ID: <20140729010043.GA6544@T430.nay.redhat.com> References: <1401180562-29680-1-git-send-email-famz@redhat.com> <1401180562-29680-3-git-send-email-famz@redhat.com> <20140728151110.GG13872@stefanha-thinkpad.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140728151110.GG13872@stefanha-thinkpad.redhat.com> Subject: Re: [Qemu-devel] [PATCH v5 2/2] vmdk: Optimize cluster allocation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi On Mon, 07/28 16:11, Stefan Hajnoczi wrote: > On Tue, May 27, 2014 at 04:49:22PM +0800, Fam Zheng wrote: > > + if (!bs->backing_hd) { > > + memset(whole_grain, 0, skip_start_sector << BDRV_SECTOR_BITS); > > + memset(whole_grain + (skip_end_sector << BDRV_SECTOR_BITS), 0, > > + cluster_bytes - (skip_end_sector << BDRV_SECTOR_BITS)); > > + } > > + > > + assert(skip_end_sector <= sector_num + extent->cluster_sectors); > > Does this assertion make sense? skip_end_sector is a small number of > sectors (relative to start of cluster), while sector_num + > extent->cluster_sectors is a large absolute sector offset. skip_end_sector is absolute sector number too. The caller hunk in this patch is: @@ -1406,12 +1468,17 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num, if (!extent) { return -EIO; } - ret = get_cluster_offset( - bs, - extent, - &m_data, - sector_num << 9, !extent->compressed, - &cluster_offset); + extent_begin_sector = extent->end_sector - extent->sectors; + extent_relative_sector_num = sector_num - extent_begin_sector; + index_in_cluster = extent_relative_sector_num % extent->cluster_sectors; + n = extent->cluster_sectors - index_in_cluster; + if (n > nb_sectors) { + n = nb_sectors; + } + ret = get_cluster_offset(bs, extent, &m_data, sector_num << 9, + !(extent->compressed || zeroed), + &cluster_offset, + index_in_cluster, index_in_cluster + n); if (extent->compressed) { if (ret == VMDK_OK) { /* Refuse write to allocated cluster for streamOptimized */ See the last parameter of get_cluster_offset. > > > +/** > > + * get_cluster_offset > > + * > > + * Look up cluster offset in extent file by sector number, and store in > > + * @cluster_offset. > > + * > > + * For flat extent, the start offset as parsed from the description file is > > s/extent/extents/ > > > + * returned. > > + * > > + * For sparse extent, look up in L1, L2 table. If allocate is true, return an > > s/extent/extents/ > > > + * offset for a new cluster and update L2 cache. If there is a backing file, > > + * COW is done before returning; otherwise, zeroes are written to the allocated > > + * cluster. Both COW and zero writting skips the sector range > > s/writting/writing/ Thanks, Fam