The Wednesday 13 Aug 2014 à 23:01:44 (+0200), Max Reitz wrote :
> Put the code for comparing the calculated refcounts against the on-disk
> refcounts during qemu-img check into an own function.
>
> Signed-off-by: Max Reitz <mre...@redhat.com>
> ---
> block/qcow2-refcount.c | 91
> ++++++++++++++++++++++++++++----------------------
> 1 file changed, 52 insertions(+), 39 deletions(-)
>
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index 9793c27..d1da8d5 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -1629,46 +1629,22 @@ static int calculate_refcounts(BlockDriverState *bs,
> BdrvCheckResult *res,
> }
>
> /*
> - * 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.
> + * Compares the actual reference count for each cluster in the image against
> the
> + * refcount as reported by the refcount structures on-disk.
> */
> -int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
> - BdrvCheckMode fix)
> +static void compare_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
> + BdrvCheckMode fix, int64_t *highest_cluster,
> + uint16_t *refcount_table, int64_t nb_clusters)
> {
> 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;
> - }
> + int64_t i;
> + int refcount1, refcount2, ret;
>
> - /* compare ref counts */
> - for (i = 0, highest_cluster = 0; i < nb_clusters; i++) {
> + for (i = 0, *highest_cluster = 0; i < nb_clusters; i++) {
> refcount1 = get_refcount(bs, i);
> if (refcount1 < 0) {
> fprintf(stderr, "Can't get refcount for cluster %" PRId64 ":
> %s\n",
> - i, strerror(-refcount1));
> + i, strerror(-refcount1));
Free coding fix.
> res->check_errors++;
> continue;
> }
> @@ -1676,11 +1652,10 @@ int qcow2_check_refcounts(BlockDriverState *bs,
> BdrvCheckResult *res,
> refcount2 = refcount_table[i];
>
> if (refcount1 > 0 || refcount2 > 0) {
> - highest_cluster = i;
> + *highest_cluster = i;
idem.
> }
>
> if (refcount1 != refcount2) {
> -
Spurious blank line removal.
> /* Check if we're allowed to fix the mismatch */
> int *num_fixed = NULL;
> if (refcount1 > refcount2 && (fix & BDRV_FIX_LEAKS)) {
> @@ -1690,10 +1665,10 @@ int qcow2_check_refcounts(BlockDriverState *bs,
> BdrvCheckResult *res,
> }
>
> fprintf(stderr, "%s cluster %" PRId64 " refcount=%d
> reference=%d\n",
> - num_fixed != NULL ? "Repairing" :
> - refcount1 < refcount2 ? "ERROR" :
> - "Leaked",
> - i, refcount1, refcount2);
> + num_fixed != NULL ? "Repairing" :
> + refcount1 < refcount2 ? "ERROR" :
> + "Leaked",
> + i, refcount1, refcount2);
Gratuitous coding style fix.
Patch 1 have similar issues.
I think the patchs would be smaller and cleaner without these spurious coding
style fixes.
>
> if (num_fixed) {
> ret = update_refcount(bs, i << s->cluster_bits, 1,
> @@ -1713,6 +1688,44 @@ int qcow2_check_refcounts(BlockDriverState *bs,
> BdrvCheckResult *res,
> }
> }
> }
> +}
> +
> +/*
> + * 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, highest_cluster, nb_clusters;
> + 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_refcounts(bs, res, fix, &highest_cluster, refcount_table,
> + nb_clusters);
>
> /* check OFLAG_COPIED */
> ret = check_oflag_copied(bs, res, fix);
> --
> 2.0.3
>
>
