Re: [Qemu-devel] [PATCH v5 01/11] qcow2: Calculate refcount block entry count

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2614 bytes --]

On 08/29/2014 03:40 PM, Max Reitz wrote:
> The size of a refblock entry is (in theory) variable; calculate
> therefore the number of entries per refblock and the according bit shift
> (1 << x == entry count) when opening an image.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/qcow2.c | 2 ++
>  block/qcow2.h | 2 ++
>  2 files changed, 4 insertions(+)

What is the maximum refcount_order?  The specs don't mention; the file
format is wide open to overflows.  Even something as benign-sounding as
refcount_order=6 (64 bits) means that each cluster can be referenced
2**64 times, which is far longer than our lifetimes to build it up that
high incrementally, and represents far greater than the amount of
storage in existence being deduplicated!  Shockingly easy to start
getting into undefined territory, so maybe we ought to explicitly cap
refcount_order to 6.

> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index f9e045f..172ad00 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -689,6 +689,8 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
>  
>      s->l2_bits = s->cluster_bits - 3; /* L2 is always one cluster */
>      s->l2_size = 1 << s->l2_bits;
> +    s->refcount_block_bits = s->cluster_bits - (s->refcount_order - 3);

Hmm; we document that qemu requires cluster_bits to be between 9 and 21
inclusive.  So, if cluster_bits is 9 (512-byte clusters), and
refcount_order is 6, then we can pack in 9 - (6 - 3) or 2**6 (that is,
64) refcounts per cluster.

On the other extreme, the minimum refcount_order of 0 (each cluster
occupies refcount bits, and so is either allocated or not, but no
sharing), starts having the math looks ugly, because you are mixing:

(int) = (uint32_t) - ( (uint32_t) - (int) )

so at one point, you are doing s->cluster_bits - (4294967293U), but that
wraps around (thankfully, wraparound is well-defined on unsigned types)
for a net answer of cluster_bits + 3.  But in the worst case, that means
an image with 2M clusters will be packing 21 - (0 - 3) or 2**24 (that
is, 16M) refcounts in one cluster.  Still fits in an int, so it looks
like you are safe...

> +    s->refcount_block_size = 1 << s->refcount_block_bits;

...that this particular shift will not cause undefined behavior, for
reasonable refcount_order in the range [0,6].

Reviewed-by: Eric Blake <eblake@redhat.com>

[We really ought to tighten the qcow2 spec - but that's a separate patch]

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]