[1/8] qcow2: Factor out refcount accounting for check

Put the code for calculating the reference counts during qemu-img check
into an own function.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-refcount.c | 202 +++++++++++++++++++++++++++++--------------------
 1 file changed, 122 insertions(+), 80 deletions(-)

The Wednesday 13 Aug 2014 à 23:01:43 (+0200), Max Reitz wrote :
> Put the code for calculating the reference counts during qemu-img check
> into an own function.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/qcow2-refcount.c | 202 +++++++++++++++++++++++++++++--------------------
>  1 file changed, 122 insertions(+), 80 deletions(-)
> 
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index d60e2fe..9793c27 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -1496,71 +1496,17 @@ done:
>  }
>  
>  /*
> - * Checks an image for refcount consistency.
> - *
> - * Returns 0 if no errors are found, the number of errors in case the image is
> - * detected as corrupted, and -errno when an internal error occurred.
> + * Checks consistency of refblocks and accounts for each refblock in
> + * *refcount_table.
>   */
> -int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
> -                          BdrvCheckMode fix)
> +static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res,
> +                           BdrvCheckMode fix, uint16_t **refcount_table,
> +                           int64_t *nb_clusters)
>  {
>      BDRVQcowState *s = bs->opaque;
> -    int64_t size, i, highest_cluster, nb_clusters;
> -    int refcount1, refcount2;
> -    QCowSnapshot *sn;
> -    uint16_t *refcount_table;
> -    int ret;
> -
> -    size = bdrv_getlength(bs->file);
> -    if (size < 0) {
> -        res->check_errors++;
> -        return size;
> -    }
> -
> -    nb_clusters = size_to_clusters(s, size);
> -    if (nb_clusters > INT_MAX) {
> -        res->check_errors++;
> -        return -EFBIG;
> -    }
> -
> -    refcount_table = g_try_malloc0(nb_clusters * sizeof(uint16_t));
> -    if (nb_clusters && refcount_table == NULL) {
> -        res->check_errors++;
> -        return -ENOMEM;
> -    }
> -
> -    res->bfi.total_clusters =
> -        size_to_clusters(s, bs->total_sectors * BDRV_SECTOR_SIZE);
> -
> -    /* header */
> -    inc_refcounts(bs, res, refcount_table, nb_clusters,
> -        0, s->cluster_size);
> -
> -    /* current L1 table */
> -    ret = check_refcounts_l1(bs, res, refcount_table, nb_clusters,
> -                             s->l1_table_offset, s->l1_size, CHECK_FRAG_INFO);
> -    if (ret < 0) {
> -        goto fail;
> -    }
> +    int64_t i;
>  
> -    /* snapshots */
> -    for(i = 0; i < s->nb_snapshots; i++) {
> -        sn = s->snapshots + i;
> -        ret = check_refcounts_l1(bs, res, refcount_table, nb_clusters,
> -            sn->l1_table_offset, sn->l1_size, 0);
> -        if (ret < 0) {
> -            goto fail;
> -        }
> -    }
> -    inc_refcounts(bs, res, refcount_table, nb_clusters,
> -        s->snapshots_offset, s->snapshots_size);
> -
> -    /* refcount data */
> -    inc_refcounts(bs, res, refcount_table, nb_clusters,
> -        s->refcount_table_offset,
> -        s->refcount_table_size * sizeof(uint64_t));
> -
> -    for(i = 0; i < s->refcount_table_size; i++) {
> +    for (i = 0; i < s->refcount_table_size; i++) {
>          uint64_t offset, cluster;
>          offset = s->refcount_table[i];
>          cluster = offset >> s->cluster_bits;
> @@ -1568,12 +1514,12 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
>          /* Refcount blocks are cluster aligned */
>          if (offset_into_cluster(s, offset)) {
>              fprintf(stderr, "ERROR refcount block %" PRId64 " is not "
> -                "cluster aligned; refcount table entry corrupted\n", i);
> +                    "cluster aligned; refcount table entry corrupted\n", i);
>              res->corruptions++;
>              continue;
>          }
>  
> -        if (cluster >= nb_clusters) {
> +        if (cluster >= *nb_clusters) {
>              fprintf(stderr, "ERROR refcount block %" PRId64
>                      " is outside image\n", i);
>              res->corruptions++;
> @@ -1581,14 +1527,14 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
>          }
>  
>          if (offset != 0) {
> -            inc_refcounts(bs, res, refcount_table, nb_clusters,
> -                offset, s->cluster_size);
> -            if (refcount_table[cluster] != 1) {
> +            inc_refcounts(bs, res, *refcount_table, *nb_clusters,
> +                          offset, s->cluster_size);
> +            if ((*refcount_table)[cluster] != 1) {
>                  fprintf(stderr, "%s refcount block %" PRId64
> -                    " refcount=%d\n",
> -                    fix & BDRV_FIX_ERRORS ? "Repairing" :
> -                                            "ERROR",
> -                    i, refcount_table[cluster]);
> +                        " refcount=%d\n",
> +                        fix & BDRV_FIX_ERRORS ? "Repairing" :
> +                                                "ERROR",
> +                        i, (*refcount_table)[cluster]);
>  
>                  if (fix & BDRV_FIX_ERRORS) {
>                      int64_t new_offset;
> @@ -1600,18 +1546,25 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
>                      }
>  
>                      /* update refcounts */
> -                    if ((new_offset >> s->cluster_bits) >= nb_clusters) {
> +                    if ((new_offset >> s->cluster_bits) >= *nb_clusters) {
>                          /* increase refcount_table size if necessary */
> -                        int old_nb_clusters = nb_clusters;
> -                        nb_clusters = (new_offset >> s->cluster_bits) + 1;
> -                        refcount_table = g_realloc(refcount_table,
> -                                nb_clusters * sizeof(uint16_t));
> -                        memset(&refcount_table[old_nb_clusters], 0, (nb_clusters
> -                                - old_nb_clusters) * sizeof(uint16_t));
> +                        int old_nb_clusters = *nb_clusters;
> +                        *nb_clusters = (new_offset >> s->cluster_bits) + 1;
> +
> +                        *refcount_table = g_try_realloc(*refcount_table,
> +                                *nb_clusters * sizeof(uint16_t));
> +                        if (!*refcount_table) {
> +                            res->check_errors++;
> +                            return -ENOMEM;
> +                        }
> +
> +                        memset(&(*refcount_table)[old_nb_clusters], 0,
> +                               (*nb_clusters - old_nb_clusters) *
> +                               sizeof(uint16_t));
>                      }
> -                    refcount_table[cluster]--;
> -                    inc_refcounts(bs, res, refcount_table, nb_clusters,
> -                            new_offset, s->cluster_size);
> +                    (*refcount_table)[cluster]--;
> +                    inc_refcounts(bs, res, *refcount_table, *nb_clusters,
> +                                  new_offset, s->cluster_size);
>  
>                      res->corruptions_fixed++;
>                  } else {
> @@ -1621,6 +1574,95 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
>          }
>      }
>  
> +    return 0;
> +}
> +
> +/*
> + * Calculates an in-memory refcount table.
> + */
> +static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
> +                               BdrvCheckMode fix, uint16_t **refcount_table,
> +                               int64_t *nb_clusters)
> +{
> +    BDRVQcowState *s = bs->opaque;
> +    QCowSnapshot *sn;
> +    int64_t i;
> +    int ret;
> +
> +    if (!*refcount_table) {
> +        *refcount_table = g_try_malloc0(*nb_clusters * sizeof(uint16_t));
> +        if (*nb_clusters && !*refcount_table) {
> +            res->check_errors++;
> +            return -ENOMEM;
> +        }
> +    }
> +
> +    /* header */
> +    inc_refcounts(bs, res, *refcount_table, *nb_clusters,
> +                  0, s->cluster_size);
> +
> +    /* current L1 table */
> +    ret = check_refcounts_l1(bs, res, *refcount_table, *nb_clusters,
> +                             s->l1_table_offset, s->l1_size, CHECK_FRAG_INFO);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    /* snapshots */
> +    for (i = 0; i < s->nb_snapshots; i++) {
> +        sn = s->snapshots + i;
> +        ret = check_refcounts_l1(bs, res, *refcount_table, *nb_clusters,
> +                                 sn->l1_table_offset, sn->l1_size, 0);
> +        if (ret < 0) {
> +            return ret;
> +        }
> +    }
> +    inc_refcounts(bs, res, *refcount_table, *nb_clusters,
> +                  s->snapshots_offset, s->snapshots_size);
> +
> +    /* refcount data */
> +    inc_refcounts(bs, res, *refcount_table, *nb_clusters,
> +                  s->refcount_table_offset,
> +                  s->refcount_table_size * sizeof(uint64_t));
> +
> +    return check_refblocks(bs, res, fix, refcount_table, nb_clusters);
> +}
> +
> +/*
> + * Checks an image for refcount consistency.
> + *
> + * Returns 0 if no errors are found, the number of errors in case the image is
> + * detected as corrupted, and -errno when an internal error occurred.
> + */
> +int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
> +                          BdrvCheckMode fix)
> +{
> +    BDRVQcowState *s = bs->opaque;
> +    int64_t size, i, highest_cluster, nb_clusters;
> +    int refcount1, refcount2;
> +    uint16_t *refcount_table = NULL;
> +    int ret;
> +
> +    size = bdrv_getlength(bs->file);
> +    if (size < 0) {
> +        res->check_errors++;
> +        return size;
> +    }
> +
> +    nb_clusters = size_to_clusters(s, size);
> +    if (nb_clusters > INT_MAX) {
> +        res->check_errors++;
> +        return -EFBIG;
> +    }
> +
> +    res->bfi.total_clusters =
> +        size_to_clusters(s, bs->total_sectors * BDRV_SECTOR_SIZE);
> +
> +    ret = calculate_refcounts(bs, res, fix, &refcount_table, &nb_clusters);
> +    if (ret < 0) {
> +        goto fail;
> +    }
> +
>      /* compare ref counts */
>      for (i = 0, highest_cluster = 0; i < nb_clusters; i++) {
>          refcount1 = get_refcount(bs, i);
> -- 
> 2.0.3
> 
> 

The diff is impressive but the patch is mostly moving stuff around.
So
Reviewed-by: Benoit Canet <benoit.canet@nodalink.com>