Re: [Qemu-devel] [PATCH v2 5/7] block/qcow2: Make get_refcount() global

["Benoît Canet" <benoit.canet@irqsave.net>]

The Saturday 02 Aug 2014 à 01:49:19 (+0200), Max Reitz wrote :
> Reading the refcount of a cluster is an operation which can be useful in
> all of the qcow2 code, so make that function globally available.
> 
> While touching this function, amend the comment describing the "addend"
> parameter: It is (no longer, if it ever was) necessary to have it set to
> -1 or 1; any value is fine.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/qcow2-refcount.c | 26 +++++++++++++-------------
>  block/qcow2.h          |  2 ++
>  2 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index cc6cf74..1c2ab8c 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -87,7 +87,7 @@ static int load_refcount_block(BlockDriverState *bs,
>   * return value is the refcount of the cluster, negative values are -errno
>   * and indicate an error.
>   */
> -static int get_refcount(BlockDriverState *bs, int64_t cluster_index)
> +int qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index)
>  {
>      BDRVQcowState *s = bs->opaque;
>      uint64_t refcount_table_index, block_index;
> @@ -605,8 +605,7 @@ fail:
>  }
>  
>  /*
> - * Increases or decreases the refcount of a given cluster by one.
> - * addend must be 1 or -1.
> + * Increases or decreases the refcount of a given cluster.
>   *
>   * If the return value is non-negative, it is the new refcount of the cluster.
>   * If it is negative, it is -errno and indicates an error.
> @@ -625,7 +624,7 @@ int qcow2_update_cluster_refcount(BlockDriverState *bs,
>          return ret;
>      }
>  
> -    return get_refcount(bs, cluster_index);
> +    return qcow2_get_refcount(bs, cluster_index);
>  }
>  
>  
> @@ -646,7 +645,7 @@ static int64_t alloc_clusters_noref(BlockDriverState *bs, uint64_t size)
>  retry:
>      for(i = 0; i < nb_clusters; i++) {
>          uint64_t next_cluster_index = s->free_cluster_index++;
> -        refcount = get_refcount(bs, next_cluster_index);
> +        refcount = qcow2_get_refcount(bs, next_cluster_index);
>  
>          if (refcount < 0) {
>              return refcount;
> @@ -710,7 +709,7 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
>          /* Check how many clusters there are free */
>          cluster_index = offset >> s->cluster_bits;
>          for(i = 0; i < nb_clusters; i++) {
> -            refcount = get_refcount(bs, cluster_index++);
> +            refcount = qcow2_get_refcount(bs, cluster_index++);
>  
>              if (refcount < 0) {
>                  return refcount;
> @@ -927,7 +926,7 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
>                                      cluster_index, addend,
>                                      QCOW2_DISCARD_SNAPSHOT);
>                          } else {
> -                            refcount = get_refcount(bs, cluster_index);
> +                            refcount = qcow2_get_refcount(bs, cluster_index);
>                          }
>  
>                          if (refcount < 0) {
> @@ -967,7 +966,7 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
>                  refcount = qcow2_update_cluster_refcount(bs, l2_offset >>
>                          s->cluster_bits, addend, QCOW2_DISCARD_SNAPSHOT);
>              } else {
> -                refcount = get_refcount(bs, l2_offset >> s->cluster_bits);
> +                refcount = qcow2_get_refcount(bs, l2_offset >> s->cluster_bits);
>              }
>              if (refcount < 0) {
>                  ret = refcount;
> @@ -1243,8 +1242,8 @@ fail:
>   * Checks the OFLAG_COPIED flag for all L1 and L2 entries.
>   *
>   * This function does not print an error message nor does it increment
> - * check_errors if get_refcount fails (this is because such an error will have
> - * been already detected and sufficiently signaled by the calling function
> + * check_errors if qcow2_get_refcount fails (this is because such an error will
> + * have been already detected and sufficiently signaled by the calling function
>   * (qcow2_check_refcounts) by the time this function is called).
>   */
>  static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,
> @@ -1265,7 +1264,7 @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,
>              continue;
>          }
>  
> -        refcount = get_refcount(bs, l2_offset >> s->cluster_bits);
> +        refcount = qcow2_get_refcount(bs, l2_offset >> s->cluster_bits);
>          if (refcount < 0) {
>              /* don't print message nor increment check_errors */
>              continue;
> @@ -1307,7 +1306,8 @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,
>  
>              if ((cluster_type == QCOW2_CLUSTER_NORMAL) ||
>                  ((cluster_type == QCOW2_CLUSTER_ZERO) && (data_offset != 0))) {
> -                refcount = get_refcount(bs, data_offset >> s->cluster_bits);
> +                refcount = qcow2_get_refcount(bs,
> +                                              data_offset >> s->cluster_bits);
>                  if (refcount < 0) {
>                      /* don't print message nor increment check_errors */
>                      continue;
> @@ -1597,7 +1597,7 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
>  
>      /* compare ref counts */
>      for (i = 0, highest_cluster = 0; i < nb_clusters; i++) {
> -        refcount1 = get_refcount(bs, i);
> +        refcount1 = qcow2_get_refcount(bs, i);
>          if (refcount1 < 0) {
>              fprintf(stderr, "Can't get refcount for cluster %" PRId64 ": %s\n",
>                  i, strerror(-refcount1));
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 1c4f9bf..c0e1b7b 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -472,6 +472,8 @@ int qcow2_update_header(BlockDriverState *bs);
>  int qcow2_refcount_init(BlockDriverState *bs);
>  void qcow2_refcount_close(BlockDriverState *bs);
>  
> +int qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index);
> +
>  int qcow2_update_cluster_refcount(BlockDriverState *bs, int64_t cluster_index,
>                                    int addend, enum qcow2_discard_type type);
>  
> -- 
> 2.0.3
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>