Re: [Qemu-devel] [PATCH v2 31/32] qcow2: Allow configuring the L2 slice size

[Anton Nefedov]

On 15/12/2017 3:53 PM, Alberto Garcia wrote:
> diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
> index 2fcecbd7a8..fe58d1ec70 100644
> --- a/block/qcow2-cache.c
> +++ b/block/qcow2-cache.c
[..]
> @@ -879,9 +896,13 @@ static int qcow2_update_options_prepare(BlockDriverState 
> *bs,
>           }
>       }
>   
> -    r->l2_slice_size = s->cluster_size / sizeof(uint64_t);
> -    r->l2_table_cache = qcow2_cache_create(bs, l2_cache_size);
> -    r->refcount_block_cache = qcow2_cache_create(bs, refcount_cache_size);
> +    l2_cache_size *= s->cluster_size / l2_cache_entry_size;
> +

A bit confusing; l2_cache_size first means the size in bytes, then the
size in clusters and now the size in entries.

Maybe in the comparison with MIN_L2_CACHE_SIZE, we should multiply
MIN_L2_CACHE_SIZE to cluster_size instead.
And perhaps MIN_L2_CACHE_CLUSTERS is a better name. Or should it even be
MIN_L2_CACHE_ENTRIES instead, taking into account its motivation to make
it possible to handle COW.

Also, I guess the final size-in-entries needs to be compared with
INT_MAX, not the size-in-clusters.

> +    r->l2_slice_size = l2_cache_entry_size / sizeof(uint64_t);
> +    r->l2_table_cache = qcow2_cache_create(bs, l2_cache_size,
> +                                           l2_cache_entry_size);

my gcc thinks l2_cache_entry_size may be used uninitialized here.
can't quite see how that may happen though..