From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55199) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X1ZqG-0003r8-Dm for qemu-devel@nongnu.org; Mon, 30 Jun 2014 07:33:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X1ZqA-0005Nn-3R for qemu-devel@nongnu.org; Mon, 30 Jun 2014 07:33:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41618) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X1Zq9-0005Nh-JA for qemu-devel@nongnu.org; Mon, 30 Jun 2014 07:33:41 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s5UBXfC9012308 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Mon, 30 Jun 2014 07:33:41 -0400 Date: Mon, 30 Jun 2014 13:33:39 +0200 From: Kevin Wolf Message-ID: <20140630113339.GE4334@noname.str.redhat.com> References: <1402167080-20316-1-git-send-email-mreitz@redhat.com> <1402167080-20316-4-git-send-email-mreitz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1402167080-20316-4-git-send-email-mreitz@redhat.com> Subject: Re: [Qemu-devel] [PATCH v8 03/14] qcow2: Optimize bdrv_make_empty() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: qemu-devel@nongnu.org, Stefan Hajnoczi Am 07.06.2014 um 20:51 hat Max Reitz geschrieben: > bdrv_make_empty() is currently only called if the current image > represents an external snapshot that has been committed to its base > image; it is therefore unlikely to have internal snapshots. In this > case, bdrv_make_empty() can be greatly sped up by creating an empty L1 > table and dropping all data clusters at once by recreating the refcount > structure accordingly instead of normally discarding all clusters. > > If there are snapshots, fall back to the simple implementation (discard > all clusters). > > Signed-off-by: Max Reitz > Reviewed-by: Eric Blake This approach looks a bit too complicated to me, and calulating the required metadata size seems error-prone. How about this: 1. Set the dirty flag in the header so we can mess with the L1 table without keeping the refcounts consistent 2. Overwrite the L1 table with zeros 3. Overwrite the first n clusters after the header with zeros (n = 2 + l1_clusters). 4. Update the header: refcount_table_offset = cluster_size refcount_table_clusters = 1 l1_table_offset = 3 * cluster_size 6. bdrv_truncate to n + 1 clusters 7. Now update the first 8 bytes at cluster_size (the first new refcount table entry) to point to 2 * cluster_size (new refcount block) 8. Reset refcount block and L2 cache 9. Allocate n + 1 clusters (the header, too) and make sure you get offset 0 10. Remove the dirty flag Surprisingly (or not) this is much like an ordinary image creation. The main difference is that we keep the full size of the L1 table so the image stays always valid (the spec would even allow us to temporarily set l1_size = 0, but qcow2_open() doesn't seem to like that) and all areas where the L1 table could be are zeroed (this includes the new refcount table/block until the header is updated). I wanted to check whether this would still give the preallocation=full series what it needs, but a v11 doesn't seem to be on the list yet and v10 doesn't have the dependency on this series yet. Kevin