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

[Kevin Wolf <kwolf@redhat.com>]

Am 27.03.2012 18:25, schrieb Eric Blake:
> On 03/27/2012 09:03 AM, Kevin Wolf wrote:
>> This is the second draft for what I think could be added when we increase qcow2's
>> version number to 3. This includes points that have been made by several people
>> over the past few months. We're probably not going to implement this next week,
>> but I think it's important to get discussions started early, so here it is.
>>
> 
>> +If the version is 3 or higher, the header has the following additional fields.
>> +For version 2, the values are assumed to be zero, unless specified otherwise
>> +in the description of a field.
>> +
>> +         72 -  79:  incompatible_features
>> +                    Bitmask of incompatible features. An implementation must
>> +                    fail to open an image if an unknown bit is set.
>> +
>> +                    Bit 0:      The reference counts in the image file may be
>> +                                inaccurate. Implementations must check/rebuild
>> +                                them if they rely on them.
>> +
>> +                    Bit 1:      Enable subclusters. This affects the L2 table
>> +                                format.
>> +
>> +                    Bits 2-31:  Reserved (set to 0)
> 
> Offsets 72-79 forms 8 bytes, so this should be bits 2-63 are reserved.

Thanks, good catch! This was a 32 bit field initially and when I updated
it, I forgot this.

>> +
>> +         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.
>> +                    [ TODO: Define order in sub-byte sizes ]
>> +
>> +        100 - 103:  header_length
>> +                    Length of the header structure in bytes. For version 2
>> +                    images, the length is always assumed to be 72 bytes.
> 
> Might be a good idea to require this to be a multiple of 8, since both
> 72 and 104 qualify, and since header extensions are also required to be
> padded out to multiples of 8.

Do you see any arguments for padding to multiples of 8 besides
consistency? If I did the format from scratch, without having to pay
attention to compatibility, I would drop the requirement even for header
extensions as I don't see what it buys us.

Consistency is important and certainly good enough to make me unsure
about this, but I don't like artificial restrictions either. If we had
another good reason, it would be easier for me to decide.

>> +== Feature name table ==
>> +
>> +A feature name table is an optional header extension that contains the name for
>> +features used by the image. It can be used by applications that don't know
>> +the respective feature (e.g. because the feature was introduced only later) to
>> +display a useful error message.
>> +
>> +The number of entries in the feature name table is determined by the length of
>> +the header extension data. Its entries look like this:
>> +
>> +    Byte       0:   Type of feature (select feature bitmap)
>> +                        0: Incompatible feature
>> +                        1: Compatible feature
>> +                        2: Autoclear feature
>> +
>> +               1:   Bit number within the selected feature bitmap
>> +
>> +          2 - 47:   Feature name (padded with zeros, but not necessarily null
>> +                    terminated if it has full length)
> 
> Semantic nit: The NUL character is all zeros; it is one byte in all
> unibyte and multi-byte encodings, and the NUL wide character is the
> all-zero wchar_t value; while 'null' refers to a pointer to nowhere.
> Saying a string is null terminated is wrong, because you don't have a 4-
> or 8-byte NULL pointer at the end of the string, just a one-byte NUL
> character.  Therefore, strings are nul-terminated, not null-terminated.

"null-terminated" is much more common. Google and Wikipedia are the
proof. ;-)

> Is this extension capped at 48 bytes, or it is a repeating table of as
> many 48-byte multiples as necessary to represent each feature name?

The latter. All feature names are in a single table in a single header
extensions. Any suggestion how to clarify this? Would something like
"There shall be at most one feature name table header extension in an
image" be clear enough?

Kevin