From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36766) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XHtkz-00013r-QK for qemu-devel@nongnu.org; Thu, 14 Aug 2014 08:03:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XHtkt-0006DO-K1 for qemu-devel@nongnu.org; Thu, 14 Aug 2014 08:03:49 -0400 Received: from lputeaux-656-01-25-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:35446 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XHtkt-0006DK-AM for qemu-devel@nongnu.org; Thu, 14 Aug 2014 08:03:43 -0400 Date: Thu, 14 Aug 2014 14:02:52 +0200 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140814120252.GG2009@irqsave.net> References: <1407963710-4942-1-git-send-email-mreitz@redhat.com> <1407963710-4942-3-git-send-email-mreitz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1407963710-4942-3-git-send-email-mreitz@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 2/8] qcow2: Factor out refcount comparison for check List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi The Wednesday 13 Aug 2014 =E0 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. >=20 > Signed-off-by: Max Reitz > --- > block/qcow2-refcount.c | 91 ++++++++++++++++++++++++++++--------------= -------- > 1 file changed, 52 insertions(+), 39 deletions(-) >=20 > 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, > } > =20 > /* > - * 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 a= gainst 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 *r= es, > + BdrvCheckMode fix, int64_t *highest_clus= ter, > + uint16_t *refcount_table, int64_t nb_clu= sters) > { > BDRVQcowState *s =3D bs->opaque; > - int64_t size, i, highest_cluster, nb_clusters; > - int refcount1, refcount2; > - uint16_t *refcount_table =3D NULL; > - int ret; > - > - size =3D bdrv_getlength(bs->file); > - if (size < 0) { > - res->check_errors++; > - return size; > - } > - > - nb_clusters =3D size_to_clusters(s, size); > - if (nb_clusters > INT_MAX) { > - res->check_errors++; > - return -EFBIG; > - } > - > - res->bfi.total_clusters =3D > - size_to_clusters(s, bs->total_sectors * BDRV_SECTOR_SIZE); > - > - ret =3D calculate_refcounts(bs, res, fix, &refcount_table, &nb_clu= sters); > - if (ret < 0) { > - goto fail; > - } > + int64_t i; > + int refcount1, refcount2, ret; > =20 > - /* compare ref counts */ > - for (i =3D 0, highest_cluster =3D 0; i < nb_clusters; i++) { > + for (i =3D 0, *highest_cluster =3D 0; i < nb_clusters; i++) { > refcount1 =3D 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 =3D refcount_table[i]; > =20 > if (refcount1 > 0 || refcount2 > 0) { > - highest_cluster =3D i; > + *highest_cluster =3D i; idem. > } > =20 > if (refcount1 !=3D refcount2) { > - Spurious blank line removal. > /* Check if we're allowed to fix the mismatch */ > int *num_fixed =3D NULL; > if (refcount1 > refcount2 && (fix & BDRV_FIX_LEAKS)) { > @@ -1690,10 +1665,10 @@ int qcow2_check_refcounts(BlockDriverState *bs,= BdrvCheckResult *res, > } > =20 > fprintf(stderr, "%s cluster %" PRId64 " refcount=3D%d refe= rence=3D%d\n", > - num_fixed !=3D NULL ? "Repairing" : > - refcount1 < refcount2 ? "ERROR" : > - "Leaked", > - i, refcount1, refcount2); > + num_fixed !=3D 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 co= ding style fixes. > =20 > if (num_fixed) { > ret =3D 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 =3D bs->opaque; > + int64_t size, highest_cluster, nb_clusters; > + uint16_t *refcount_table =3D NULL; > + int ret; > + > + size =3D bdrv_getlength(bs->file); > + if (size < 0) { > + res->check_errors++; > + return size; > + } > + > + nb_clusters =3D size_to_clusters(s, size); > + if (nb_clusters > INT_MAX) { > + res->check_errors++; > + return -EFBIG; > + } > + > + res->bfi.total_clusters =3D > + size_to_clusters(s, bs->total_sectors * BDRV_SECTOR_SIZE); > + > + ret =3D calculate_refcounts(bs, res, fix, &refcount_table, &nb_clu= sters); > + if (ret < 0) { > + goto fail; > + } > + > + compare_refcounts(bs, res, fix, &highest_cluster, refcount_table, > + nb_clusters); > =20 > /* check OFLAG_COPIED */ > ret =3D check_oflag_copied(bs, res, fix); > --=20 > 2.0.3 >=20 >=20