Am 15.12.2014 um 13:50 hat Max Reitz geschrieben:
> Since refcounts do not always have to be a uint16_t, all refcount blocks
> and arrays in memory should not have a specific type (thus they become
> pointers to void) and for accessing them, two helper functions are used
> (a getter and a setter). Those functions are called indirectly through
> function pointers in the BDRVQcowState so they may later be exchanged
> for different refcount orders.
>
> With the check and repair functions using this function, the refcount
> array they are creating will be in big endian byte order; additionally,
> using realloc_refcount_array() makes the size of this refcount array
> always cluster-aligned. Both combined allow rebuild_refcount_structure()
> to drop the bounce buffer which was used to convert parts of the
> refcount array to big endian byte order and store them on disk. Instead,
> those parts can now be written directly.
>
> Signed-off-by: Max Reitz <mre...@redhat.com>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com>
> ---
> block/qcow2-refcount.c | 122
> ++++++++++++++++++++++++++++---------------------
> block/qcow2.h | 8 ++++
> 2 files changed, 79 insertions(+), 51 deletions(-)
> @@ -541,7 +561,7 @@ static int QEMU_WARN_UNUSED_RESULT
> update_refcount(BlockDriverState *bs,
> {
> BDRVQcowState *s = bs->opaque;
> int64_t start, last, cluster_offset;
> - uint16_t *refcount_block = NULL;
> + void *refcount_block = NULL;
> int64_t old_table_index = -1;
> int ret;
>
> @@ -593,7 +613,7 @@ static int QEMU_WARN_UNUSED_RESULT
> update_refcount(BlockDriverState *bs,
> /* we can update the count and save it */
> block_index = cluster_index & (s->refcount_block_size - 1);
>
> - refcount = be16_to_cpu(refcount_block[block_index]);
> + refcount = s->get_refcount(refcount_block, block_index);
> if (decrease ? (refcount - addend > refcount)
> : (refcount + addend < refcount ||
> refcount + addend > s->refcount_max))
> @@ -609,7 +629,7 @@ static int QEMU_WARN_UNUSED_RESULT
> update_refcount(BlockDriverState *bs,
> if (refcount == 0 && cluster_index < s->free_cluster_index) {
> s->free_cluster_index = cluster_index;
> }
> - refcount_block[block_index] = cpu_to_be16(refcount);
> + s->set_refcount(refcount_block, block_index, refcount);
>
> if (refcount == 0 && s->discard_passthrough[type]) {
> update_refcount_discard(bs, cluster_offset, s->cluster_size);
> @@ -625,8 +645,7 @@ fail:
> /* Write last changed block to disk */
> if (refcount_block) {
> int wret;
> - wret = qcow2_cache_put(bs, s->refcount_block_cache,
> - (void**) &refcount_block);
> + wret = qcow2_cache_put(bs, s->refcount_block_cache, &refcount_block);
> if (wret < 0) {
> return ret < 0 ? ret : wret;
> }
There is a (void**) cast left in the other qcow2_cache_put() call in
update_refcount().
> @@ -1883,7 +1907,7 @@ static int64_t alloc_clusters_imrt(BlockDriverState *bs,
> */
> static int rebuild_refcount_structure(BlockDriverState *bs,
> BdrvCheckResult *res,
> - uint16_t **refcount_table,
> + void **refcount_table,
> int64_t *nb_clusters)
> {
> BDRVQcowState *s = bs->opaque;
> @@ -1891,8 +1915,8 @@ static int rebuild_refcount_structure(BlockDriverState
> *bs,
> int64_t refblock_offset, refblock_start, refblock_index;
> uint32_t reftable_size = 0;
> uint64_t *on_disk_reftable = NULL;
> - uint16_t *on_disk_refblock;
> - int i, ret = 0;
> + void *on_disk_refblock;
> + int ret = 0;
> struct {
> uint64_t reftable_offset;
> uint32_t reftable_clusters;
> @@ -1902,7 +1926,7 @@ static int rebuild_refcount_structure(BlockDriverState
> *bs,
>
> write_refblocks:
> for (; cluster < *nb_clusters; cluster++) {
> - if (!(*refcount_table)[cluster]) {
> + if (!s->get_refcount(*refcount_table, cluster)) {
> continue;
> }
>
> @@ -1975,17 +1999,13 @@ write_refblocks:
> goto fail;
> }
>
> - on_disk_refblock = qemu_blockalign0(bs->file, s->cluster_size);
> - for (i = 0; i < s->refcount_block_size &&
> - refblock_start + i < *nb_clusters; i++)
> - {
> - on_disk_refblock[i] =
> - cpu_to_be16((*refcount_table)[refblock_start + i]);
> - }
> + /* The size of *refcount_table is always cluster-aligned, therefore
> the
> + * write operation will not overflow */
> + on_disk_refblock = (void *)((uintptr_t)*refcount_table +
> + (refblock_index <<
> s->refcount_block_bits));
Are you sure about s->refcount_block_bits? You're now calculating in
bytes and not in entries any more like before with uint16_t*. So I think
this should be s->cluster_bits now (or refblock_index * s->cluster_size
if you don't want to confuse people more than necessary :-)).
> ret = bdrv_write(bs->file, refblock_offset / BDRV_SECTOR_SIZE,
> - (void *)on_disk_refblock, s->cluster_sectors);
> - qemu_vfree(on_disk_refblock);
> + on_disk_refblock, s->cluster_sectors);
> if (ret < 0) {
> fprintf(stderr, "ERROR writing refblock: %s\n", strerror(-ret));
> goto fail;
Kevin
