Patchwork [RFC,V6,31/33] qcow: Set large dedup hash block size.

login
register
mail settings
Submitter Benoît Canet
Date Feb. 6, 2013, 12:32 p.m.
Message ID <1360153926-9492-32-git-send-email-benoit@irqsave.net>
Download mbox | patch
Permalink /patch/218622/
State New
Headers show

Comments

Benoît Canet - Feb. 6, 2013, 12:32 p.m.
Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 block/qcow2-refcount.c |    4 ++--
 block/qcow2.c          |    3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)
Stefan Hajnoczi - Feb. 8, 2013, 11:07 a.m.
On Wed, Feb 06, 2013 at 01:32:04PM +0100, Benoît Canet wrote:
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index 337fb65..d173c18 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -1051,7 +1051,7 @@ static int check_dedup_l2(BlockDriverState *bs, BdrvCheckResult *res,
>      int i, l2_size;
>  
>      /* Read L2 table from disk */
> -    l2_size = s->cluster_size;
> +    l2_size = s->hash_block_size;
>      l2_table = g_malloc(l2_size);
>  
>      if (bdrv_pread(bs->file, l2_offset, l2_table, l2_size) != l2_size) {
> @@ -1141,7 +1141,7 @@ static int check_refcounts_l1(BlockDriverState *bs,
>              /* Mark L2 table as used */
>              l2_offset &= L1E_OFFSET_MASK;
>              inc_refcounts(bs, res, refcount_table, refcount_table_size,
> -                l2_offset, s->cluster_size);
> +                l2_offset, dedup ? s->hash_block_size : s->l2_size << 3);

Why s->l2_size << 3 instead of s->cluster_size for the non-dedup case?
Benoît Canet - Feb. 27, 2013, 2:53 p.m.
> >              inc_refcounts(bs, res, refcount_table, refcount_table_size,
> > -                l2_offset, s->cluster_size);
> > +                l2_offset, dedup ? s->hash_block_size : s->l2_size << 3);
> 
> Why s->l2_size << 3 instead of s->cluster_size for the non-dedup case?

I wrote it like this so the code would work fine is l2_size is different of
cluster size. It is almost a bugfix.

Regards

Benoît
Stefan Hajnoczi - Feb. 28, 2013, 9:47 a.m.
On Wed, Feb 27, 2013 at 03:53:43PM +0100, Benoît Canet wrote:
> > >              inc_refcounts(bs, res, refcount_table, refcount_table_size,
> > > -                l2_offset, s->cluster_size);
> > > +                l2_offset, dedup ? s->hash_block_size : s->l2_size << 3);
> > 
> > Why s->l2_size << 3 instead of s->cluster_size for the non-dedup case?
> 
> I wrote it like this so the code would work fine is l2_size is different of
> cluster size. It is almost a bugfix.

I see.  Please use s->l2_size * sizeof(uint64_t) to make the expression
clear - this is what the rest of the code uses to calculate the L2 table
size in bytes.

Stefan

Patch

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 337fb65..d173c18 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1051,7 +1051,7 @@  static int check_dedup_l2(BlockDriverState *bs, BdrvCheckResult *res,
     int i, l2_size;
 
     /* Read L2 table from disk */
-    l2_size = s->cluster_size;
+    l2_size = s->hash_block_size;
     l2_table = g_malloc(l2_size);
 
     if (bdrv_pread(bs->file, l2_offset, l2_table, l2_size) != l2_size) {
@@ -1141,7 +1141,7 @@  static int check_refcounts_l1(BlockDriverState *bs,
             /* Mark L2 table as used */
             l2_offset &= L1E_OFFSET_MASK;
             inc_refcounts(bs, res, refcount_table, refcount_table_size,
-                l2_offset, s->cluster_size);
+                l2_offset, dedup ? s->hash_block_size : s->l2_size << 3);
 
             /* L2 tables are cluster aligned */
             if (l2_offset & (s->cluster_size - 1)) {
diff --git a/block/qcow2.c b/block/qcow2.c
index 7263e74..9097e64 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -435,6 +435,9 @@  static int qcow2_open(BlockDriverState *bs, int flags)
     s->cluster_sectors = 1 << (s->cluster_bits - 9);
     s->l2_bits = s->cluster_bits - 3; /* L2 is always one cluster */
     s->l2_size = 1 << s->l2_bits;
+    if (s->incompatible_features & QCOW2_INCOMPAT_DEDUP) {
+        s->hash_block_size = DEFAULT_CLUSTER_SIZE * 5;
+    }
     bs->total_sectors = header.size / 512;
     s->csize_shift = (62 - (s->cluster_bits - 8));
     s->csize_mask = (1 << (s->cluster_bits - 8)) - 1;