mirrors/qemu
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

qcow2: Use unsigned addend for update_refcount()

Browse Source 
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 <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This commit is contained in:
Max Reitz 2015-02-10 15:28:47 -05:00 committed by Kevin Wolf
parent 7324c10f96
commit 2aabe7c7a1
3 changed files with 52 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
block/qcow2-cluster.c
View File

@ -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 /* For shared L2 tables, set the refcount accordingly (it is
* already 1 and needs to be l2_refcount) */ * already 1 and needs to be l2_refcount) */
ret = qcow2_update_cluster_refcount(bs, 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); QCOW2_DISCARD_OTHER);
if (ret < 0) { if (ret < 0) {
qcow2_free_clusters(bs, offset, s->cluster_size, qcow2_free_clusters(bs, offset, s->cluster_size,

65
block/qcow2-refcount.c
View File

@ -29,8 +29,8 @@
static int64_t alloc_clusters_noref(BlockDriverState *bs, uint64_t size); static int64_t alloc_clusters_noref(BlockDriverState *bs, uint64_t size);
static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs, static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
int64_t offset, int64_t length, int64_t offset, int64_t length, uint16_t addend,
int addend, enum qcow2_discard_type type); bool decrease, enum qcow2_discard_type type);
/*********************************************************/ /*********************************************************/
@ -263,7 +263,7 @@ static int alloc_refcount_block(BlockDriverState *bs,
} else { } else {
/* Described somewhere else. This can recurse at most twice before we /* Described somewhere else. This can recurse at most twice before we
* arrive at a block that describes itself. */ * 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); QCOW2_DISCARD_NEVER);
if (ret < 0) { if (ret < 0) {
goto fail_block; goto fail_block;
@ -530,8 +530,14 @@ found:
} }
/* XXX: cache several refcount block clusters ? */ /* 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, 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; BDRVQcowState *s = bs->opaque;
int64_t start, last, cluster_offset; int64_t start, last, cluster_offset;
@ -540,8 +546,9 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
int ret; int ret;
#ifdef DEBUG_ALLOC2 #ifdef DEBUG_ALLOC2
fprintf(stderr, "update_refcount: offset=%" PRId64 " size=%" PRId64 " addend=%d\n", fprintf(stderr, "update_refcount: offset=%" PRId64 " size=%" PRId64
offset, length, addend); " addend=%s%" PRIu16 "\n", offset, length, decrease ? "-" : "",
addend);
#endif #endif
if (length < 0) { if (length < 0) {
return -EINVAL; return -EINVAL;
@ -549,7 +556,7 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
return 0; return 0;
} }
if (addend < 0) { if (decrease) {
qcow2_cache_set_dependency(bs, s->refcount_block_cache, qcow2_cache_set_dependency(bs, s->refcount_block_cache,
s->l2_table_cache); s->l2_table_cache);
} }
@ -559,7 +566,8 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
for(cluster_offset = start; cluster_offset <= last; for(cluster_offset = start; cluster_offset <= last;
cluster_offset += s->cluster_size) 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 cluster_index = cluster_offset >> s->cluster_bits;
int64_t table_index = cluster_index >> s->refcount_block_bits; int64_t table_index = cluster_index >> s->refcount_block_bits;
@ -586,11 +594,18 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
block_index = cluster_index & (s->refcount_block_size - 1); block_index = cluster_index & (s->refcount_block_size - 1);
refcount = be16_to_cpu(refcount_block[block_index]); refcount = be16_to_cpu(refcount_block[block_index]);
refcount += addend; if (decrease ? ((uint16_t)(refcount - addend) > refcount)
if (refcount < 0 || refcount > s->refcount_max) { : ((uint16_t)(refcount + addend) < refcount ||
(uint16_t)(refcount + addend) > s->refcount_max))
{
ret = -EINVAL; ret = -EINVAL;
goto fail; goto fail;
} }
if (decrease) {
refcount -= addend;
} else {
refcount += addend;
}
if (refcount == 0 && cluster_index < s->free_cluster_index) { if (refcount == 0 && cluster_index < s->free_cluster_index) {
s->free_cluster_index = cluster_index; s->free_cluster_index = cluster_index;
} }
@ -623,8 +638,8 @@ fail:
*/ */
if (ret < 0) { if (ret < 0) {
int dummy; int dummy;
dummy = update_refcount(bs, offset, cluster_offset - offset, -addend, dummy = update_refcount(bs, offset, cluster_offset - offset, addend,
QCOW2_DISCARD_NEVER); !decrease, QCOW2_DISCARD_NEVER);
(void)dummy; (void)dummy;
} }
@ -634,18 +649,21 @@ fail:
/* /*
* Increases or decreases the refcount of a given cluster. * 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. * On success 0 is returned; on failure -errno is returned.
*/ */
int qcow2_update_cluster_refcount(BlockDriverState *bs, int qcow2_update_cluster_refcount(BlockDriverState *bs,
int64_t cluster_index, int64_t cluster_index,
int addend, uint16_t addend, bool decrease,
enum qcow2_discard_type type) enum qcow2_discard_type type)
{ {
BDRVQcowState *s = bs->opaque; BDRVQcowState *s = bs->opaque;
int ret; int ret;
ret = update_refcount(bs, cluster_index << s->cluster_bits, 1, addend, ret = update_refcount(bs, cluster_index << s->cluster_bits, 1, addend,
type); decrease, type);
if (ret < 0) { if (ret < 0) {
return ret; return ret;
} }
@ -709,7 +727,7 @@ int64_t qcow2_alloc_clusters(BlockDriverState *bs, uint64_t size)
return offset; 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); } while (ret == -EAGAIN);
if (ret < 0) { if (ret < 0) {
@ -746,7 +764,7 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
} }
/* And then allocate them */ /* 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); QCOW2_DISCARD_NEVER);
} while (ret == -EAGAIN); } while (ret == -EAGAIN);
@ -797,7 +815,7 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
} }
assert(offset); 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) { if (ret < 0) {
return ret; return ret;
} }
@ -821,7 +839,7 @@ void qcow2_free_clusters(BlockDriverState *bs,
int ret; int ret;
BLKDBG_EVENT(bs->file, BLKDBG_CLUSTER_FREE); 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) { if (ret < 0) {
fprintf(stderr, "qcow2_free_clusters failed: %s\n", strerror(-ret)); fprintf(stderr, "qcow2_free_clusters failed: %s\n", strerror(-ret));
/* TODO Remember the clusters to free them later and avoid leaking */ /* TODO Remember the clusters to free them later and avoid leaking */
@ -887,6 +905,8 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
uint16_t refcount; uint16_t refcount;
int ret; int ret;
assert(addend >= -1 && addend <= 1);
l2_table = NULL; l2_table = NULL;
l1_table = NULL; l1_table = NULL;
l1_size2 = l1_size * sizeof(uint64_t); l1_size2 = l1_size * sizeof(uint64_t);
@ -951,7 +971,7 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
if (addend != 0) { if (addend != 0) {
ret = update_refcount(bs, ret = update_refcount(bs,
(offset & s->cluster_offset_mask) & ~511, (offset & s->cluster_offset_mask) & ~511,
nb_csectors * 512, addend, nb_csectors * 512, abs(addend), addend < 0,
QCOW2_DISCARD_SNAPSHOT); QCOW2_DISCARD_SNAPSHOT);
if (ret < 0) { if (ret < 0) {
goto fail; goto fail;
@ -982,7 +1002,7 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
} }
if (addend != 0) { if (addend != 0) {
ret = qcow2_update_cluster_refcount(bs, ret = qcow2_update_cluster_refcount(bs,
cluster_index, addend, cluster_index, abs(addend), addend < 0,
QCOW2_DISCARD_SNAPSHOT); QCOW2_DISCARD_SNAPSHOT);
if (ret < 0) { if (ret < 0) {
goto fail; goto fail;
@ -1025,7 +1045,7 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
if (addend != 0) { if (addend != 0) {
ret = qcow2_update_cluster_refcount(bs, l2_offset >> ret = qcow2_update_cluster_refcount(bs, l2_offset >>
s->cluster_bits, s->cluster_bits,
addend, abs(addend), addend < 0,
QCOW2_DISCARD_SNAPSHOT); QCOW2_DISCARD_SNAPSHOT);
if (ret < 0) { if (ret < 0) {
goto fail; goto fail;
@ -1678,7 +1698,8 @@ static void compare_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
if (num_fixed) { if (num_fixed) {
ret = update_refcount(bs, i << s->cluster_bits, 1, ret = update_refcount(bs, i << s->cluster_bits, 1,
(int)refcount2 - (int)refcount1, refcount_diff(refcount1, refcount2),
refcount1 > refcount2,
QCOW2_DISCARD_ALWAYS); QCOW2_DISCARD_ALWAYS);
if (ret >= 0) { if (ret >= 0) {
(*num_fixed)++; (*num_fixed)++;

8
block/qcow2.h
View File

@ -459,6 +459,11 @@ static inline uint64_t l2meta_cow_end(QCowL2Meta *m)
+ (m->cow_end.nb_sectors << BDRV_SECTOR_BITS); + (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 // FIXME Need qcow2_ prefix to global functions
/* qcow2.c functions */ /* qcow2.c functions */
@ -482,7 +487,8 @@ int qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index,
uint16_t *refcount); uint16_t *refcount);
int qcow2_update_cluster_refcount(BlockDriverState *bs, int64_t cluster_index, 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); int64_t qcow2_alloc_clusters(BlockDriverState *bs, uint64_t size);
int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset, int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,