[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 18/69] qcow2: Only return status from qcow2_get_refco
From: |
Stefan Hajnoczi |
Subject: |
[Qemu-devel] [PULL 18/69] qcow2: Only return status from qcow2_get_refcount |
Date: |
Fri, 27 Feb 2015 18:18:16 +0000 |
From: Max Reitz <address@hidden>
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 <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>
---
block/qcow2-cluster.c | 8 ++---
block/qcow2-refcount.c | 86 +++++++++++++++++++++++++++-----------------------
block/qcow2.h | 3 +-
3 files changed, 53 insertions(+), 44 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 183177d..ee50500 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1640,7 +1640,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState
*bs, uint64_t *l1_table,
for (i = 0; i < l1_size; i++) {
uint64_t l2_offset = l1_table[i] & L1E_OFFSET_MASK;
bool l2_dirty = false;
- int l2_refcount;
+ uint16_t l2_refcount;
if (!l2_offset) {
/* unallocated */
@@ -1672,9 +1672,9 @@ static int expand_zero_clusters_in_l1(BlockDriverState
*bs, uint64_t *l1_table,
goto fail;
}
- l2_refcount = qcow2_get_refcount(bs, l2_offset >> s->cluster_bits);
- if (l2_refcount < 0) {
- ret = l2_refcount;
+ ret = qcow2_get_refcount(bs, l2_offset >> s->cluster_bits,
+ &l2_refcount);
+ if (ret < 0) {
goto fail;
}
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 1a85bcd..d853490 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -87,26 +87,29 @@ static int load_refcount_block(BlockDriverState *bs,
}
/*
- * Returns the refcount of the cluster given by its index. Any non-negative
- * return value is the refcount of the cluster, negative values are -errno
- * and indicate an error.
+ * Retrieves the refcount of the cluster given by its index and stores it in
+ * *refcount. Returns 0 on success and -errno on failure.
*/
-int qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index)
+int qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index,
+ uint16_t *refcount)
{
BDRVQcowState *s = bs->opaque;
uint64_t refcount_table_index, block_index;
int64_t refcount_block_offset;
int ret;
uint16_t *refcount_block;
- uint16_t refcount;
refcount_table_index = cluster_index >> s->refcount_block_bits;
- if (refcount_table_index >= s->refcount_table_size)
+ if (refcount_table_index >= s->refcount_table_size) {
+ *refcount = 0;
return 0;
+ }
refcount_block_offset =
s->refcount_table[refcount_table_index] & REFT_OFFSET_MASK;
- if (!refcount_block_offset)
+ if (!refcount_block_offset) {
+ *refcount = 0;
return 0;
+ }
if (offset_into_cluster(s, refcount_block_offset)) {
qcow2_signal_corruption(bs, true, -1, -1, "Refblock offset %#" PRIx64
@@ -122,7 +125,7 @@ int qcow2_get_refcount(BlockDriverState *bs, int64_t
cluster_index)
}
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]);
ret = qcow2_cache_put(bs, s->refcount_block_cache,
(void**) &refcount_block);
@@ -130,7 +133,7 @@ int qcow2_get_refcount(BlockDriverState *bs, int64_t
cluster_index)
return ret;
}
- return refcount;
+ return 0;
}
/*
@@ -662,16 +665,17 @@ static int64_t alloc_clusters_noref(BlockDriverState *bs,
uint64_t size)
{
BDRVQcowState *s = bs->opaque;
uint64_t i, nb_clusters;
- int refcount;
+ uint16_t refcount;
+ int ret;
nb_clusters = size_to_clusters(s, size);
retry:
for(i = 0; i < nb_clusters; i++) {
uint64_t next_cluster_index = s->free_cluster_index++;
- refcount = qcow2_get_refcount(bs, next_cluster_index);
+ ret = qcow2_get_refcount(bs, next_cluster_index, &refcount);
- if (refcount < 0) {
- return refcount;
+ if (ret < 0) {
+ return ret;
} else if (refcount != 0) {
goto retry;
}
@@ -721,7 +725,8 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t
offset,
BDRVQcowState *s = bs->opaque;
uint64_t cluster_index;
uint64_t i;
- int refcount, ret;
+ uint16_t refcount;
+ int ret;
assert(nb_clusters >= 0);
if (nb_clusters == 0) {
@@ -732,10 +737,9 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t
offset,
/* Check how many clusters there are free */
cluster_index = offset >> s->cluster_bits;
for(i = 0; i < nb_clusters; i++) {
- refcount = qcow2_get_refcount(bs, cluster_index++);
-
- if (refcount < 0) {
- return refcount;
+ ret = qcow2_get_refcount(bs, cluster_index++, &refcount);
+ if (ret < 0) {
+ return ret;
} else if (refcount != 0) {
break;
}
@@ -769,9 +773,10 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
offset = s->free_byte_offset;
if (offset) {
- int refcount = qcow2_get_refcount(bs, offset >> s->cluster_bits);
- if (refcount < 0) {
- return refcount;
+ uint16_t refcount;
+ ret = qcow2_get_refcount(bs, offset >> s->cluster_bits, &refcount);
+ if (ret < 0) {
+ return ret;
}
if (refcount == s->refcount_max) {
@@ -878,7 +883,8 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
uint64_t *l1_table, *l2_table, l2_offset, offset, l1_size2;
bool l1_allocated = false;
int64_t old_offset, old_l2_offset;
- int i, j, l1_modified = 0, nb_csectors, refcount;
+ int i, j, l1_modified = 0, nb_csectors;
+ uint16_t refcount;
int ret;
l2_table = NULL;
@@ -983,9 +989,8 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
}
}
- refcount = qcow2_get_refcount(bs, cluster_index);
- if (refcount < 0) {
- ret = refcount;
+ ret = qcow2_get_refcount(bs, cluster_index, &refcount);
+ if (ret < 0) {
goto fail;
}
break;
@@ -1026,9 +1031,9 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
goto fail;
}
}
- refcount = qcow2_get_refcount(bs, l2_offset >> s->cluster_bits);
- if (refcount < 0) {
- ret = refcount;
+ ret = qcow2_get_refcount(bs, l2_offset >> s->cluster_bits,
+ &refcount);
+ if (ret < 0) {
goto fail;
} else if (refcount == 1) {
l2_offset |= QCOW_OFLAG_COPIED;
@@ -1346,7 +1351,7 @@ static int check_oflag_copied(BlockDriverState *bs,
BdrvCheckResult *res,
BDRVQcowState *s = bs->opaque;
uint64_t *l2_table = qemu_blockalign(bs, s->cluster_size);
int ret;
- int refcount;
+ uint16_t refcount;
int i, j;
for (i = 0; i < s->l1_size; i++) {
@@ -1358,8 +1363,9 @@ static int check_oflag_copied(BlockDriverState *bs,
BdrvCheckResult *res,
continue;
}
- refcount = qcow2_get_refcount(bs, l2_offset >> s->cluster_bits);
- if (refcount < 0) {
+ ret = qcow2_get_refcount(bs, l2_offset >> s->cluster_bits,
+ &refcount);
+ if (ret < 0) {
/* don't print message nor increment check_errors */
continue;
}
@@ -1400,9 +1406,10 @@ static int check_oflag_copied(BlockDriverState *bs,
BdrvCheckResult *res,
if ((cluster_type == QCOW2_CLUSTER_NORMAL) ||
((cluster_type == QCOW2_CLUSTER_ZERO) && (data_offset != 0))) {
- refcount = qcow2_get_refcount(bs,
- data_offset >> s->cluster_bits);
- if (refcount < 0) {
+ ret = qcow2_get_refcount(bs,
+ data_offset >> s->cluster_bits,
+ &refcount);
+ if (ret < 0) {
/* don't print message nor increment check_errors */
continue;
}
@@ -1634,13 +1641,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;
}
@@ -1670,7 +1678,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)++;
diff --git a/block/qcow2.h b/block/qcow2.h
index 55138c9..a33431f 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -478,7 +478,8 @@ void qcow2_signal_corruption(BlockDriverState *bs, bool
fatal, int64_t offset,
int qcow2_refcount_init(BlockDriverState *bs);
void qcow2_refcount_close(BlockDriverState *bs);
-int qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index);
+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);
--
2.1.0
- [Qemu-devel] [PULL 08/69] qcow2: Remove unused struct QCowCreateState, (continued)
- [Qemu-devel] [PULL 08/69] qcow2: Remove unused struct QCowCreateState, Stefan Hajnoczi, 2015/02/27
- [Qemu-devel] [PULL 09/69] virtio-blk: Check return value of blk_aio_ioctl, Stefan Hajnoczi, 2015/02/27
- [Qemu-devel] [PULL 11/69] tests: Prepare virtio-blk-test for multi-arch implementation, Stefan Hajnoczi, 2015/02/27
- [Qemu-devel] [PULL 10/69] libqos: Change use of pointers to uint64_t in virtio, Stefan Hajnoczi, 2015/02/27
- [Qemu-devel] [PULL 12/69] libqos: Remove PCI assumptions in constants of virtio driver, Stefan Hajnoczi, 2015/02/27
- [Qemu-devel] [PULL 13/69] libqos: Add malloc generic, Stefan Hajnoczi, 2015/02/27
- [Qemu-devel] [PULL 06/69] sheepdog: selectable object size support, Stefan Hajnoczi, 2015/02/27
- [Qemu-devel] [PULL 16/69] qcow2: Add refcount_bits to format-specific info, Stefan Hajnoczi, 2015/02/27
- [Qemu-devel] [PULL 14/69] libqos: Add virtio MMIO support, Stefan Hajnoczi, 2015/02/27
- [Qemu-devel] [PULL 15/69] qcow2: Add two new fields to BDRVQcowState, Stefan Hajnoczi, 2015/02/27
- [Qemu-devel] [PULL 18/69] qcow2: Only return status from qcow2_get_refcount,
Stefan Hajnoczi <=
- [Qemu-devel] [PULL 20/69] qcow2: Use 64 bits for refcount values, Stefan Hajnoczi, 2015/02/27
- [Qemu-devel] [PULL 17/69] qcow2: Do not return new value after refcount update, Stefan Hajnoczi, 2015/02/27
- [Qemu-devel] [PULL 19/69] qcow2: Use unsigned addend for update_refcount(), Stefan Hajnoczi, 2015/02/27
- [Qemu-devel] [PULL 21/69] qcow2: Helper for refcount array reallocation, Stefan Hajnoczi, 2015/02/27
- [Qemu-devel] [PULL 22/69] qcow2: Helper function for refcount modification, Stefan Hajnoczi, 2015/02/27
- [Qemu-devel] [PULL 24/69] qcow2: Open images with refcount order != 4, Stefan Hajnoczi, 2015/02/27
- [Qemu-devel] [PULL 26/69] qcow2: Use symbolic macros in qcow2_amend_options, Stefan Hajnoczi, 2015/02/27
- [Qemu-devel] [PULL 23/69] qcow2: More helpers for refcount modification, Stefan Hajnoczi, 2015/02/27
- [Qemu-devel] [PULL 25/69] qcow2: refcount_order parameter for qcow2_create2, Stefan Hajnoczi, 2015/02/27
- [Qemu-devel] [PULL 28/69] qcow2: Allow creation with refcount order != 4, Stefan Hajnoczi, 2015/02/27