Re: [Qemu-devel] [PATCH 3/6] add-cow file format

[Stefan Hajnoczi <stefanha@gmail.com>]

On Wed, Aug 1, 2012 at 2:57 PM, Eric Blake <eblake@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 <wdongxu@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.

Stefan