On 12/03/2014 06:37 AM, Max Reitz wrote: > Refcounts may have a width of up to 64 bits, so qemu should use the same > width to represent refcount values internally. > > Signed-off-by: Max Reitz <mre...@redhat.com> > --- > block/qcow2-cluster.c | 2 +- > block/qcow2-refcount.c | 42 ++++++++++++++++++++---------------------- > block/qcow2.h | 4 ++-- > 3 files changed, 23 insertions(+), 25 deletions(-) >
> @@ -1698,7 +1696,7 @@ static void compare_refcounts(BlockDriverState *bs,
> BdrvCheckResult *res,
> refcount2 - refcount1,
> refcount1 > refcount2,
> QCOW2_DISCARD_ALWAYS);
> - if (ret >= 0) {
> + if (ret == 0) {
> (*num_fixed)++;
> continue;
> }
This hunk looks a bit misplaced (it is unrelated to a sizing change).
Patch 5 altered update_refcount, but does not document its return value.
But a careful step through update_refcount() shows that all error paths
return negative, and the only places where we do things like 'ret =
alloc_refcount_block(...)" are later followed by an unconditional 'ret =
0' on success paths (so I didn't check whether alloc_refcount_block
returns positive values, but didn't need to). If you have to respin, it
might be better to add documentation of the return value and update
callers to assume a non-positive return as a separate patch, rather than
sliding it in with this one. But as I don't see any code faults, and am
not sure it is worth a respin just for this issue,
Reviewed-by: Eric Blake <ebl...@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
