[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v10 1/2] docs: improve qcow2 spec about extending image heade

From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PATCH v10 1/2] docs: improve qcow2 spec about extending image header
Date: Fri, 31 Jan 2020 17:05:04 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1

21.01.2020 19:18, Max Reitz wrote:
On 20.01.20 20:42, Eric Blake wrote:
On 1/20/20 11:13 AM, Vladimir Sementsov-Ogievskiy wrote:
Make it more obvious how to add new fields to the version 3 header and
how to interpret them.

The specification is adjusted so for new defined optional fields:

s/so for/so that for/

1. Software may support some of these optional fields and ignore the
     others, which means that features may be backported to downstream
     Qemu independently.
2. If we want to add incompatible field (or a field, for which some its
     values would be incompatible), it must be accompanied by
     incompatible feature bit.

Also the concept of "default is zero" is clarified, as it's strange to
say that the value of the field is assumed to be zero for the software
version which don't know about the field at all and don't know how to
treat it be it zero or not.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
   docs/interop/qcow2.txt | 44 +++++++++++++++++++++++++++++++++++++++---
   1 file changed, 41 insertions(+), 3 deletions(-)

diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
index af5711e533..355925c35e 100644
--- a/docs/interop/qcow2.txt
+++ b/docs/interop/qcow2.txt
@@ -79,9 +79,9 @@ The first cluster of a qcow2 image contains the file
                       Offset into the image file at which the snapshot
                       starts. Must be aligned to a cluster boundary.
   -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
-in the description of a field.
+For version 2, the header is exactly 72 bytes in length, and finishes
+For version 3 or higher, the header length is at least 104 bytes,
+the next fields through header_length.
              72 -  79:  incompatible_features
                       Bitmask of incompatible features. An
implementation must
@@ -164,6 +164,44 @@ in the description of a field.
           100 - 103:  header_length
                       Length of the header structure in bytes. For
version 2
                       images, the length is always assumed to be 72
+                    For version 3 it's at least 104 bytes and must be
a multiple
+                    of 8.
+=== Additional fields (version 3 and higher) ===
+In general, these fields are optional and may be safely ignored by
the software,
+as well as filled by zeros (which is equal to field absence), if
software needs

We're inconsistent on 'zeros' (git grep has 201 hits) vs. 'zeroes' (688
hits); I prefer the latter, but won't object if you don't tweak it since
this is the first use of either spelling in qcow2.txt.

+to set field B, but does not care about field A, which precedes B. More

s/A, which/A which/

I’ve heard before that one should always use a comma before “which” and
never before “that” (in that a subordinate clause opened by “that” is a
mandatory description, whereas those that start with “why” are not
necessary for understanding).

So if this is a mandatory description (which I suppose it is), shouldn’t
it also be s/which/that/?

I suppose “field A that precedes B” sounds a bit weird because “A”
hasn’t been introduced before.  That is, “the field that precedes B”
would sound more natural.  Or is that precisely the kind of instance
where one would use “which” without comma? :-)

All in all, I was wondering whether there isn’t a more natural way to
rephrase the whole paragraph.  (No, I don’t have an excuse why I didn’t
say so in the last revision.)


In general, these fields are optional and may be safely ignored when
read and filled with zeroes when written.

The end of sentence is a bit in conflict with following "... other than preserving 
its value when rewriting the image header ..."

(yes, here we say about creating new fields, not rewriting the existing ones, 
but this is not clear from the context)

So, for v10 I tend to keep it as is (with Eric's corrections).

  For example, say software
wants to set field B but does not care about its preceding field A.  It
may then set A to zero, B to its desired value, and adjust header_length
to include A and B.



Best regards,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]