Am 13.04.2012 12:30, schrieb Stefan Hajnoczi: > On Thu, Apr 12, 2012 at 4:01 PM, Kevin Wolf <kw...@redhat.com> 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