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

[Eric Blake <eblake@redhat.com>]

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

On 04/02/2012 04:00 AM, Kevin Wolf wrote:
> 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.
>>>
>>

>>> +
>>> +        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?

Yes - void* on some platforms is 8 bytes, and having everything
guarantee 8-byte alignment can make processing of headers more efficient
when you are reading things on natural alignments.

Furthermore, guaranteeing 8-byte alignment buys us three bits that are
always 0 but which can later be converted to bit flags for future
extensions; by requiring 8-byte alignment, older parsers will reject the
new bit flags (because it looks like a non-multiple-of-8 length), while
newer parsers will know that they are bit flags and what those flags
mean, as well as know to mask out those bits when computing aligned size
of the header.

> 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.

It's always hard to predict what future extensions will look like, but I
argue in return that it is easier to start out strict and relax things
in the future than it is to start relaxed and then wish we could tighten
it up.

> 
> 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.

If sizeof(void*) for natural alignment and the possibility of extension
to 3 bit flags per extension header don't convince you, then I won't insist.

>> 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. ;-)

Unfortunately true :)  But I'll quit bothering you about this one, as
I'm swimming against the current on that one.

> 
>> 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?

Maybe:

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.

There can be at most one feature name table, and within that table, each
feature name may only appear once.  The number of entries (n) in the
feature name table is determined by the length of the header extension
data. Its entries look like this:

    Byte 48*n + 0:   Type of feature (select feature bitmap)
                        0: Incompatible feature
                        1: Compatible feature
                        2: Autoclear feature

         48*n + 1:   Bit number within the selected feature bitmap

   48*n + 2 to 47:   Feature name (padded with zeros, but not
necessarily null
                     terminated if it has full length)

Do we also need to clarify that at offsets 48*n + 1, the bit number must
be 0-63 (and thus the upper two bits must be 0)?  Do we also want to
enforce that the table is sorted (that is, given the tuple <feature,bit>
in bytes 0 and 1 of each entry, we want to require that entry <0,0>
appears before <0,1> appears before <1,0>)?

-- 
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: 620 bytes --]