From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:54793) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SwZnd-0000UR-6b for qemu-devel@nongnu.org; Wed, 01 Aug 2012 10:21:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SwZnW-0001yi-KX for qemu-devel@nongnu.org; Wed, 01 Aug 2012 10:21:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60647) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SwZnW-0001yY-B4 for qemu-devel@nongnu.org; Wed, 01 Aug 2012 10:21:14 -0400 Message-ID: <50193B51.1050609@redhat.com> Date: Wed, 01 Aug 2012 16:21:05 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1343753510-24661-1-git-send-email-wdongxu@linux.vnet.ibm.com> <1343753510-24661-3-git-send-email-wdongxu@linux.vnet.ibm.com> <501935E7.8000607@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 3/6] add-cow file format List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Dong Xu Wang , Eric Blake , qemu-devel@nongnu.org Am 01.08.2012 16:14, schrieb Stefan Hajnoczi: > On Wed, Aug 1, 2012 at 2:57 PM, Eric Blake wrote: >> On 07/31/2012 10:51 AM, Dong Xu Wang wrote: >>> This is the implementation code for add-cow file format. Because image_file >>> might be very huge, then we can't read entire bitmap into memory, we must use >>> a cache. Since qcow-cache.c has implemted cache code, we can create our cache >> >> s/implemted/implemented/ >> >>> code based on it. >> >> Just wondering if Paolo's HBitmap code for drive-mirror might be a more >> efficient way to implement your caching. >> >>> >>> Signed-off-by: Dong Xu Wang >>> --- >>> block/Makefile.objs | 1 + >>> block/add-cow-cache.c | 206 +++++++++++++++++ >>> block/add-cow.c | 599 +++++++++++++++++++++++++++++++++++++++++++++++++ >>> block/add-cow.h | 101 +++++++++ >>> block_int.h | 2 + >> >> Rather than adding a new implementation for code duplication, can you >> refactor the existing implementation to be reusable, and update >> qcow-cache.c to call into the common refactored code? > > I looked at this and block/qcow2-cache.c seems fairly clean and > generic. The only things that need to be parameterized are the > s->l2_table_cache/s->refcount_block_cache and s->cluster_size. > > These can be fixed by adding fields to Qcow2Cache: > size_t table_size; /* cached table size in bytes */ > BlkDebugEvent flush_event; > BlkDebugEvent load_event; > > Or maybe a table type enum (L2/refcount blocks/bitmap blocks). > > If Kevin agrees with extracting this from qcow2 (i.e. we can't add > more qcow2-specific behavior easily in the future), then this looks > doable. I don't think we'd want to add qcow2 specific stuff there, and I've been meaning to generalise it for quite a while, so feel free to do it. One thing to note is that it currently depends on qcow2's s->lock being taken while it's called. Eventually we'll want finer grained locking in the cache itself and I believe this will also require some changes in the interface, but for now add-cow can just do the same qcow2 and take its global CoMutex. Kevin