On 2014-12-03 at 17:11, Eric Blake wrote:
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).
Ah, right, I think it should have been in patch 3. I even looked through
the patches before sending them, but somehow I missed this...
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,
Hm, I don't think this is something that should be fixed up by the
applying maintainer. I will respin, even if it's just for this, but I'll
wait until this series has been sufficiently looked at.
Reviewed-by: Eric Blake <ebl...@redhat.com>
Once again, thank you!
Max