Re: [Qemu-devel] [PATCH 01/19] Specification for qcow2 version 3

[Kevin Wolf]

Am 13.04.2012 12:30, schrieb Stefan Hajnoczi:
> On Thu, Apr 12, 2012 at 4:01 PM, Kevin Wolf <address@hidden> wrote:
>> +         96 -  99:  refcount_bits
>> +                    Size of a reference count block entry in bits. For 
>> version 2
>> +                    images, the size is always assumed to be 16 bits. The 
>> size
>> +                    must be a power of two.
> 
> "The size must be a power of two"
> 
> If refcount_bits is the number of bits then the resulting refcount
> block size is always a power of 2.  recount_size = 1 << refcount_bits.
> 
> Do you want to drop this statement?  Just want to check that I'm
> understanding the spec correctly.

That's a bug/ambiguity in the spec. What it was meant to say is that the
number of bits must be a power of two (so that you never have to extract
a single refcount from two partial bytes).

Anthony's suggestion to make it a refcount_order fixes this nicely.

>>  The remaining space between the end of the header extension area and the 
>> end of
>> -the first cluster can be used for other data. Usually, the backing file 
>> name is
>> -stored there.
>> +the first cluster can be used for the backing file name. It is not allowed 
>> to
>> +store other data here, so that an implementation can safely modify the 
>> header
>> +and add extensions without harming data of compatible features that it
>> +doesn't support. Compatible features that need space for additional data can
>> +use a header extension.
> 
> Does this change the spec for qcow2 version 2?  Previously anything
> could be after the header extension area, now this has been changed so
> only the backing filename is allowed (for safe modification).  In
> practice this should be okay but in theory I think this is changes the
> qcow2 version 2 semantics.

I guess it does. Are you aware of anyone using this? We could in theory
add the restriction only for version 3+, but I'd rather avoid special
cases if possible.

I think in practice even the old qcow2 implementation would break images
if you put arbitrary data there.

> 
>> +               1:   Bit number within the selected feature bitmap
> 
> I suggest explicitly stating the valid range: 0-63

Ok.

> 
>> -If a cluster is unallocated, read requests shall read the data from the 
>> backing
>> -file. If there is no backing file or the backing file is smaller than the 
>> image,
>> +If a cluster or a subcluster is unallocated, read requests shall read the 
>> data
> 
> I think "subcluster" is from an older version of this spec change
> because the subcluster feature is not part of this patch series.

Right.

Kevin