Re: [Qemu-devel] [PATCH v2 3/7] block/qcow2-refcount: check_refcounts_l2: refactor compressed case

[Max Reitz]

On 17.08.18 14:22, Vladimir Sementsov-Ogievskiy wrote:
> Separate offset and size of compressed cluster.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>  block/qcow2-refcount.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index 566c19fbfa..0ea01e3ee2 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -1570,7 +1570,7 @@ static int check_refcounts_l2(BlockDriverState *bs, 
> BdrvCheckResult *res,
>      BDRVQcow2State *s = bs->opaque;
>      uint64_t *l2_table, l2_entry;
>      uint64_t next_contiguous_offset = 0;
> -    int i, l2_size, nb_csectors, ret;
> +    int i, l2_size, ret;
>  
>      /* Read L2 table from disk */
>      l2_size = s->l2_size * sizeof(uint64_t);
> @@ -1589,6 +1589,9 @@ static int check_refcounts_l2(BlockDriverState *bs, 
> BdrvCheckResult *res,
>  
>          switch (qcow2_get_cluster_type(l2_entry)) {
>          case QCOW2_CLUSTER_COMPRESSED:
> +        {
> +            int64_t csize, coffset;
> +
>              /* Compressed clusters don't have QCOW_OFLAG_COPIED */
>              if (l2_entry & QCOW_OFLAG_COPIED) {
>                  fprintf(stderr, "ERROR: coffset=0x%" PRIx64 ": "
> @@ -1599,12 +1602,13 @@ static int check_refcounts_l2(BlockDriverState *bs, 
> BdrvCheckResult *res,
>              }
>  
>              /* Mark cluster as used */
> -            nb_csectors = ((l2_entry >> s->csize_shift) &
> -                           s->csize_mask) + 1;
> -            l2_entry &= s->cluster_offset_mask;
> +            csize = (((l2_entry >> s->csize_shift) & s->csize_mask) + 1) *
> +                    BDRV_SECTOR_SIZE;
> +            coffset = l2_entry & s->cluster_offset_mask &
> +                      ~(BDRV_SECTOR_SIZE - 1);

This should actually be 512 instead of BDRV_SECTOR_SIZE (or the former
may be a shift by 9 bits), because this is about qcow2 and not about the
block layer's sector size.

(Other places in the qcow2 driver handling compressed clusters use 512
or shift by 9, too.)

Max

>              ret = qcow2_inc_refcounts_imrt(bs, res,
>                                             refcount_table, 
> refcount_table_size,
> -                                           l2_entry & ~511, nb_csectors * 
> 512);
> +                                           coffset, csize);
>              if (ret < 0) {
>                  goto fail;
>              }
> @@ -1621,6 +1625,7 @@ static int check_refcounts_l2(BlockDriverState *bs, 
> BdrvCheckResult *res,
>                  res->bfi.fragmented_clusters++;
>              }
>              break;
> +        }
>  
>          case QCOW2_CLUSTER_ZERO_ALLOC:
>          case QCOW2_CLUSTER_NORMAL:
>

signature.asc
Description: OpenPGP digital signature