[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V12 4/6] rename qcow2-cache.c to block-cache.c
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH V12 4/6] rename qcow2-cache.c to block-cache.c |
Date: |
Tue, 11 Sep 2012 10:41:16 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120605 Thunderbird/13.0 |
Am 10.08.2012 17:39, schrieb Dong Xu Wang:
> add-cow and qcow2 file format will share the same cache code, so rename
> block-cache.c to block-cache.c. And related structure and qcow2 code also
> are changed.
>
> Signed-off-by: Dong Xu Wang <address@hidden>
> ---
> block.h | 3 +
> block/Makefile.objs | 3 +-
> block/qcow2-cache.c | 323
> ------------------------------------------------
> block/qcow2-cluster.c | 66 ++++++----
> block/qcow2-refcount.c | 66 ++++++-----
> block/qcow2.c | 36 +++---
> block/qcow2.h | 24 +---
> trace-events | 13 +-
> 8 files changed, 109 insertions(+), 425 deletions(-)
> delete mode 100644 block/qcow2-cache.c
>
> diff --git a/block.h b/block.h
> index e5dfcd7..c325661 100644
> --- a/block.h
> +++ b/block.h
> @@ -401,6 +401,9 @@ typedef enum {
> BLKDBG_CLUSTER_ALLOC_BYTES,
> BLKDBG_CLUSTER_FREE,
>
> + BLKDBG_ADD_COW_UPDATE,
> + BLKDBG_ADD_COW_LOAD,
> +
I don't think you should add new events, the existing ones should be
generic enough that you can reuse them. It's somewhat hard to see
without block-cache.c, though.
Can you make sure to have one patch with pure code motion, and a
separate one with the changes needed to make it work with add-cow? It
will help reviewers a lot.
> BLKDBG_EVENT_MAX,
> } BlkDebugEvent;
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index e179211..335dc7a 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -28,6 +28,7 @@
> #include "block_int.h"
> #include "block/qcow2.h"
> #include "trace.h"
> +#include "block-cache.h"
>
> int qcow2_grow_l1_table(BlockDriverState *bs, int min_size, bool exact_size)
> {
> @@ -69,7 +70,8 @@ int qcow2_grow_l1_table(BlockDriverState *bs, int min_size,
> bool exact_size)
> return new_l1_table_offset;
> }
>
> - ret = qcow2_cache_flush(bs, s->refcount_block_cache);
> + ret = block_cache_flush(bs, s->refcount_block_cache,
> + BLOCK_TABLE_REF, s->cluster_size);
I think its better to pass s->cluster_size to the cache initialisation
instead of in each call of the cache function.
For the blkdebug events I guess it's possible as well to move this to
the initialisation, but I'd have to see the block-cache.c code to say
something specific about this.
> @@ -659,18 +669,16 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs,
> QCowL2Meta *m)
> * handled.
> */
> if (cow) {
> - qcow2_cache_depends_on_flush(s->l2_table_cache);
> + block_cache_depends_on_flush(s->l2_table_cache);
> }
>
> - if (qcow2_need_accurate_refcounts(s)) {
> - qcow2_cache_set_dependency(bs, s->l2_table_cache,
> - s->refcount_block_cache);
> - }
> + block_cache_set_dependency(bs, s->l2_table_cache, BLOCK_TABLE_L2,
> + s->refcount_block_cache, s->cluster_size);
What happened with lazy refcounting? Is this a mismerge or did you
intentionally remove the condition? (There's a second place where you do
the same)
Kevin
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH V12 4/6] rename qcow2-cache.c to block-cache.c,
Kevin Wolf <=