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 <mre...@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 <ebl...@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
signature.asc
Description: OpenPGP digital signature
