On 11/14/2014 06:06 AM, Max Reitz wrote: > 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. > > Signed-off-by: Max Reitz <mre...@redhat.com> > --- > block/qcow2-refcount.c | 128 > ++++++++++++++++++++++++++++++------------------- > block/qcow2.h | 8 ++++ > 2 files changed, 87 insertions(+), 49 deletions(-) >
> @@ -1216,7 +1249,7 @@ enum { > * error occurred. > */ > static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res, > - uint16_t **refcount_table, int64_t *refcount_table_size, int64_t > l2_offset, > + void **refcount_table, int64_t *refcount_table_size, int64_t l2_offset, > int flags) > { Might be worth fixing the indentation here in addition to all the other places you adjusted. But that's minor. > @@ -1933,17 +1967,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)); Here is where you are relying on the guarantee that you added in 6/21, which is why I ask for that one to mention it. Nice reduction of a bounce buffer, by the way :) Worth mentioning in the commit message as an intentional part of this commit? > @@ -2087,7 +2117,7 @@ int qcow2_check_refcounts(BlockDriverState *bs, > BdrvCheckResult *res, > /* Because the old reftable has been exchanged for a new one the > * references have to be recalculated */ > rebuild = false; > - memset(refcount_table, 0, nb_clusters * sizeof(uint16_t)); > + memset(refcount_table, 0, nb_clusters * s->refcount_bits / 8); Phew; we're safe that this won't overflow; and good that you do the * first (if you did the /8 first, it would fail for sub-byte refcounts). Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature