update_refcount() and qcow2_update_cluster_refcount() currently take a
signed addend. At least one caller passes a value directly derived from
an absolute refcount that should be reached ("l2_refcount - 1" in
expand_zero_clusters_in_l1()). Therefore, the addend should be unsigned
as well; this will be especially important for 64 bit refcounts.
Because update_refcount() then no longer knows whether the refcount
should be increased or decreased, it now requires an additional flag
which specified exactly that. The same applies to
qcow2_update_cluster_refcount().
Signed-off-by: Max Reitz <mre...@redhat.com>
---
block/qcow2-cluster.c | 3 ++-
block/qcow2-refcount.c | 65 +++++++++++++++++++++++++++++++++-----------------
block/qcow2.h | 8 ++++++-
3 files changed, 52 insertions(+), 24 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index ee50500..405329a 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1707,7 +1707,8 @@ static int expand_zero_clusters_in_l1(BlockDriverState
*bs, uint64_t *l1_table,
/* For shared L2 tables, set the refcount accordingly (it
is
* already 1 and needs to be l2_refcount) */
ret = qcow2_update_cluster_refcount(bs,
- offset >> s->cluster_bits, l2_refcount - 1,
+ offset >> s->cluster_bits,
+ refcount_diff(1, l2_refcount), false,
QCOW2_DISCARD_OTHER);
if (ret < 0) {
qcow2_free_clusters(bs, offset, s->cluster_size,
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 9286c64..42136eb 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -29,8 +29,8 @@
static int64_t alloc_clusters_noref(BlockDriverState *bs, uint64_t size);
static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
- int64_t offset, int64_t length,
- int addend, enum qcow2_discard_type type);
+ int64_t offset, int64_t length, uint16_t addend,
+ bool decrease, enum qcow2_discard_type type);
/*********************************************************/
@@ -263,7 +263,7 @@ static int alloc_refcount_block(BlockDriverState *bs,
} else {
/* Described somewhere else. This can recurse at most twice before we
* arrive at a block that describes itself. */
- ret = update_refcount(bs, new_block, s->cluster_size, 1,
+ ret = update_refcount(bs, new_block, s->cluster_size, 1, false,
QCOW2_DISCARD_NEVER);
if (ret < 0) {
goto fail_block;
@@ -542,8 +542,14 @@ found:
}
/* XXX: cache several refcount block clusters ? */
+/* @addend is the absolute value of the addend; if @decrease is set, @addend
+ * will be subtracted from the current refcount, otherwise it will be added */
static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
- int64_t offset, int64_t length, int addend, enum qcow2_discard_type type)
+ int64_t offset,
+ int64_t length,
+ uint16_t addend,
+ bool decrease,
+ enum qcow2_discard_type
type)
{
BDRVQcowState *s = bs->opaque;
int64_t start, last, cluster_offset;
@@ -552,8 +558,9 @@ static int QEMU_WARN_UNUSED_RESULT
update_refcount(BlockDriverState *bs,
int ret;
#ifdef DEBUG_ALLOC2
- fprintf(stderr, "update_refcount: offset=%" PRId64 " size=%" PRId64 "
addend=%d\n",
- offset, length, addend);
+ fprintf(stderr, "update_refcount: offset=%" PRId64 " size=%" PRId64
+ " addend=%s%" PRIu16 "\n", offset, length, decrease ? "-" : "",
+ addend);
#endif
if (length < 0) {
return -EINVAL;
@@ -561,7 +568,7 @@ static int QEMU_WARN_UNUSED_RESULT
update_refcount(BlockDriverState *bs,
return 0;
}
- if (addend < 0) {
+ if (decrease) {
qcow2_cache_set_dependency(bs, s->refcount_block_cache,
s->l2_table_cache);
}
@@ -571,7 +578,8 @@ static int QEMU_WARN_UNUSED_RESULT
update_refcount(BlockDriverState *bs,
for(cluster_offset = start; cluster_offset <= last;
cluster_offset += s->cluster_size)
{
- int block_index, refcount;
+ int block_index;
+ uint16_t refcount;
int64_t cluster_index = cluster_offset >> s->cluster_bits;
int64_t table_index = cluster_index >> s->refcount_block_bits;
@@ -598,11 +606,18 @@ static int QEMU_WARN_UNUSED_RESULT
update_refcount(BlockDriverState *bs,
block_index = cluster_index & (s->refcount_block_size - 1);
refcount = be16_to_cpu(refcount_block[block_index]);
- refcount += addend;
- if (refcount < 0 || refcount > s->refcount_max) {
+ if (decrease ? ((uint16_t)(refcount - addend) > refcount)
+ : ((uint16_t)(refcount + addend) < refcount ||
+ (uint16_t)(refcount + addend) > s->refcount_max))
+ {
ret = -EINVAL;
goto fail;
}
+ if (decrease) {
+ refcount -= addend;
+ } else {
+ refcount += addend;
+ }
if (refcount == 0 && cluster_index < s->free_cluster_index) {
s->free_cluster_index = cluster_index;
}
@@ -635,8 +650,8 @@ fail:
*/
if (ret < 0) {
int dummy;
- dummy = update_refcount(bs, offset, cluster_offset - offset, -addend,
- QCOW2_DISCARD_NEVER);
+ dummy = update_refcount(bs, offset, cluster_offset - offset, addend,
+ !decrease, QCOW2_DISCARD_NEVER);
(void)dummy;
}
@@ -646,18 +661,21 @@ fail:
/*
* Increases or decreases the refcount of a given cluster.
*
+ * @addend is the absolute value of the addend; if @decrease is set, @addend
+ * will be subtracted from the current refcount, otherwise it will be added.
+ *
* On success 0 is returned; on failure -errno is returned.
*/
int qcow2_update_cluster_refcount(BlockDriverState *bs,
int64_t cluster_index,
- int addend,
+ uint16_t addend, bool decrease,
enum qcow2_discard_type type)
{
BDRVQcowState *s = bs->opaque;
int ret;
ret = update_refcount(bs, cluster_index << s->cluster_bits, 1, addend,
- type);
+ decrease, type);
if (ret < 0) {
return ret;
}
@@ -721,7 +739,7 @@ int64_t qcow2_alloc_clusters(BlockDriverState *bs, uint64_t
size)
return offset;
}
- ret = update_refcount(bs, offset, size, 1, QCOW2_DISCARD_NEVER);
+ ret = update_refcount(bs, offset, size, 1, false, QCOW2_DISCARD_NEVER);
} while (ret == -EAGAIN);
if (ret < 0) {
@@ -758,7 +776,7 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t
offset,
}
/* And then allocate them */
- ret = update_refcount(bs, offset, i << s->cluster_bits, 1,
+ ret = update_refcount(bs, offset, i << s->cluster_bits, 1, false,
QCOW2_DISCARD_NEVER);
} while (ret == -EAGAIN);
@@ -809,7 +827,7 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
}
assert(offset);
- ret = update_refcount(bs, offset, size, 1, QCOW2_DISCARD_NEVER);
+ ret = update_refcount(bs, offset, size, 1, false, QCOW2_DISCARD_NEVER);
if (ret < 0) {
return ret;
}
@@ -833,7 +851,7 @@ void qcow2_free_clusters(BlockDriverState *bs,
int ret;
BLKDBG_EVENT(bs->file, BLKDBG_CLUSTER_FREE);
- ret = update_refcount(bs, offset, size, -1, type);
+ ret = update_refcount(bs, offset, size, 1, true, type);
if (ret < 0) {
fprintf(stderr, "qcow2_free_clusters failed: %s\n", strerror(-ret));
/* TODO Remember the clusters to free them later and avoid leaking */
@@ -899,6 +917,8 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
uint16_t refcount;
int ret;
+ assert(addend >= -1 && addend <= 1);
+
l2_table = NULL;
l1_table = NULL;
l1_size2 = l1_size * sizeof(uint64_t);
@@ -963,7 +983,7 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
if (addend != 0) {
ret = update_refcount(bs,
(offset & s->cluster_offset_mask) & ~511,
- nb_csectors * 512, addend,
+ nb_csectors * 512, abs(addend), addend < 0,
QCOW2_DISCARD_SNAPSHOT);
if (ret < 0) {
goto fail;
@@ -994,7 +1014,7 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
}
if (addend != 0) {
ret = qcow2_update_cluster_refcount(bs,
- cluster_index, addend,
+ cluster_index, abs(addend), addend < 0,
QCOW2_DISCARD_SNAPSHOT);
if (ret < 0) {
goto fail;
@@ -1037,7 +1057,7 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
if (addend != 0) {
ret = qcow2_update_cluster_refcount(bs, l2_offset >>
s->cluster_bits,
- addend,
+ abs(addend), addend < 0,
QCOW2_DISCARD_SNAPSHOT);
if (ret < 0) {
goto fail;
@@ -1690,7 +1710,8 @@ static void compare_refcounts(BlockDriverState *bs,
BdrvCheckResult *res,
if (num_fixed) {
ret = update_refcount(bs, i << s->cluster_bits, 1,
- (int)refcount2 - (int)refcount1,
+ refcount_diff(refcount1, refcount2),
+ refcount1 > refcount2,
QCOW2_DISCARD_ALWAYS);
if (ret >= 0) {
(*num_fixed)++;
diff --git a/block/qcow2.h b/block/qcow2.h
index 1e59277..616d720 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -470,6 +470,11 @@ static inline uint64_t l2meta_cow_end(QCowL2Meta *m)
+ (m->cow_end.nb_sectors << BDRV_SECTOR_BITS);
}
+static inline uint16_t refcount_diff(uint16_t r1, uint16_t r2)
+{
+ return r1 > r2 ? r1 - r2 : r2 - r1;
+}
+
// FIXME Need qcow2_ prefix to global functions
/* qcow2.c functions */
@@ -493,7 +498,8 @@ int qcow2_get_refcount(BlockDriverState *bs, int64_t
cluster_index,
uint16_t *refcount);
int qcow2_update_cluster_refcount(BlockDriverState *bs, int64_t cluster_index,
- int addend, enum qcow2_discard_type type);
+ uint16_t addend, bool decrease,
+ enum qcow2_discard_type type);
int64_t qcow2_alloc_clusters(BlockDriverState *bs, uint64_t size);
int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
--
2.1.0
