Am 15.12.2014 um 13:50 hat Max Reitz geschrieben:
> qcow2_alloc_bytes() may reuse a cluster multiple times, in which case
> the refcount is increased accordingly. However, if this would lead to an
> overflow the function should instead just not reuse this cluster and
> allocate a new one.
>
> 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 | 31 ++++++++++++++++++++++++++++++-
> 1 file changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index db81647..fd28a13 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -780,9 +780,11 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
> BDRVQcowState *s = bs->opaque;
> int64_t offset, cluster_offset, new_cluster;
> int free_in_cluster, ret;
> + uint64_t refcount;
>
> BLKDBG_EVENT(bs->file, BLKDBG_CLUSTER_ALLOC_BYTES);
> assert(size > 0 && size <= s->cluster_size);
> + redo:
> if (s->free_byte_offset == 0) {
> offset = qcow2_alloc_clusters(bs, s->cluster_size);
> if (offset < 0) {
> @@ -790,12 +792,25 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int
> size)
> }
> s->free_byte_offset = offset;
> }
> - redo:
> +
> free_in_cluster = s->cluster_size -
> offset_into_cluster(s, s->free_byte_offset);
> if (size <= free_in_cluster) {
> /* enough space in current cluster */
> offset = s->free_byte_offset;
> +
> + if (offset_into_cluster(s, offset) != 0) {
> + /* We will have to increase the refcount of this cluster; if the
> + * maximum has been reached already, this cluster cannot be used
> */
> + ret = qcow2_get_refcount(bs, offset >> s->cluster_bits,
> &refcount);
> + if (ret < 0) {
> + return ret;
> + } else if (refcount == s->refcount_max) {
> + s->free_byte_offset = 0;
> + goto redo;
> + }
> + }
> +
> s->free_byte_offset += size;
> free_in_cluster -= size;
> if (free_in_cluster == 0)
> @@ -816,6 +831,20 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
> if ((cluster_offset + s->cluster_size) == new_cluster) {
> /* we are lucky: contiguous data */
> offset = s->free_byte_offset;
> +
> + /* Same as above: In order to reuse the cluster, the refcount
> has to
> + * be increased; if that will not work, we are not so lucky after
> + * all */
> + ret = qcow2_get_refcount(bs, offset >> s->cluster_bits,
> &refcount);
> + if (ret < 0) {
> + qcow2_free_clusters(bs, new_cluster, s->cluster_size,
> + QCOW2_DISCARD_NEVER);
> + return ret;
> + } else if (refcount == s->refcount_max) {
> + s->free_byte_offset = offset;
I think you mean 0. offset is already the old value.
> + goto redo;
> + }
> +
> ret = qcow2_update_cluster_refcount(bs, offset >>
> s->cluster_bits,
> 1, false,
> QCOW2_DISCARD_NEVER);
> if (ret < 0) {
I wonder if the code duplication is necessary. I was already thinking
that there was some duplication when I reviewed the previous patch, but
now it seems to become even more obvious that the three parts of this
function are:
1. Allocate a new cluster
2. Allocate space in the already allocated cluster
3. Allocate a new cluster and space inside it, which is just 1. + 2.
Kevin
