Am 01.08.2012 16:14, schrieb Stefan Hajnoczi: > On Wed, Aug 1, 2012 at 2:57 PM, Eric Blake <ebl...@redhat.com> 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 <wdon...@linux.vnet.ibm.com> >>> --- >>> 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