From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36370) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YNfG5-0000IM-Kd for qemu-devel@nongnu.org; Tue, 17 Feb 2015 05:20:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YNfFz-0003pO-UU for qemu-devel@nongnu.org; Tue, 17 Feb 2015 05:20:01 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40674) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YNfFz-0003p9-Nb for qemu-devel@nongnu.org; Tue, 17 Feb 2015 05:19:55 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t1HAJsE6003027 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Tue, 17 Feb 2015 05:19:54 -0500 Date: Tue, 17 Feb 2015 11:19:52 +0100 From: Kevin Wolf Message-ID: <20150217101952.GD4213@noname.str.redhat.com> References: <1423600146-7642-1-git-send-email-mreitz@redhat.com> <1423600146-7642-5-git-send-email-mreitz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1423600146-7642-5-git-send-email-mreitz@redhat.com> Subject: Re: [Qemu-devel] [PATCH v6 04/24] qcow2: Only return status from qcow2_get_refcount List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: qemu-devel@nongnu.org, Stefan Hajnoczi Am 10.02.2015 um 21:28 hat Max Reitz geschrieben: > Refcounts can theoretically be of type uint64_t; in order to be able to > represent the full range, qcow2_get_refcount() cannot use a single > variable to represent both all refcount values and also keep some values > reserved for errors. > > One solution would be to add an Error pointer parameter to > qcow2_get_refcount(); however, no caller could (currently) pass that > error message, so it would have to be emitted immediately and be > passed to the next caller by returning -EIO or something similar. > Therefore, an Error parameter does not offer any advantages here. > > The solution applied by this patch is simpler to use. Because no caller > would be able to pass the error message, they would have to print it and > free it, whereas with this patch the caller only needs to pass the > returned integer (which is often a no-op from the code perspective, > because that integer will be stored in a variable "ret" which will be > returned by the fail path of many callers). > > Signed-off-by: Max Reitz > @@ -1646,13 +1653,14 @@ static void compare_refcounts(BlockDriverState *bs, BdrvCheckResult *res, > { > BDRVQcowState *s = bs->opaque; > int64_t i; > - int refcount1, refcount2, ret; > + uint16_t refcount1, refcount2; > + int ret; > > for (i = 0, *highest_cluster = 0; i < nb_clusters; i++) { > - refcount1 = qcow2_get_refcount(bs, i); > - if (refcount1 < 0) { > + ret = qcow2_get_refcount(bs, i, &refcount1); > + if (ret < 0) { > fprintf(stderr, "Can't get refcount for cluster %" PRId64 ": %s\n", > - i, strerror(-refcount1)); > + i, strerror(-ret)); > res->check_errors++; > continue; > } > @@ -1682,7 +1690,7 @@ static void compare_refcounts(BlockDriverState *bs, BdrvCheckResult *res, > > if (num_fixed) { > ret = update_refcount(bs, i << s->cluster_bits, 1, > - refcount2 - refcount1, > + (int)refcount2 - (int)refcount1, > QCOW2_DISCARD_ALWAYS); > if (ret >= 0) { > (*num_fixed)++; Wouldn't both refcounts be promoted to int anyway, even without a cast? But then, being explicit probably can't hurt either. Kevin