[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v7 1/2] docs: improve qcow2 spec about extending image header
From: |
Eric Blake |
Subject: |
Re: [PATCH v7 1/2] docs: improve qcow2 spec about extending image header |
Date: |
Mon, 7 Oct 2019 15:21:41 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.0 |
On 10/7/19 11:04 AM, Vladimir Sementsov-Ogievskiy wrote:
Make it more obvious how to add new fields to the version 3 header and
how to interpret them.
Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
---
docs/interop/qcow2.txt | 26 +++++++++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
index af5711e533..3f2855593f 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 header:
Offset into the image file at which the snapshot table
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 otherwise
-in the description of a field.
+For version 2, header is always 72 bytes length and finishes here.
+For version 3 or higher the header length is at least 104 bytes and has at
+least next five fields, up to the @header_length field.
This hunk seems okay.
72 - 79: incompatible_features
Bitmask of incompatible features. An implementation must
@@ -165,6 +165,26 @@ in the description of a field.
Length of the header structure in bytes. For version 2
images, the length is always assumed to be 72 bytes.
+Additional fields (version 3 and higher)
+
+The following fields of the header are optional: if software don't know how to
+interpret the field, it may safely ignore it. Still the field must be kept as
is
+when rewriting the image.
if software doesn't know how to interpret the field, it may be safely
ignored, other than preserving the field unchanged when rewriting the
image header.
Missing:
If header_length excludes an optional field, the value of 0 should be
used for that field.
@header_length must be bound to the end of one of
+these fields (or to @header_length field end itself, to be 104 bytes).
We don't use the @header_length markup anywhere else in this file,
starting to do so here is odd.
I would suggest a stronger requirement:
header_length must be a multiple of 4, and must not land in the middle
of any optional 8-byte field.
Or maybe even add our compression type extension with 4 bytes of
padding, so that we could go even stronger:
header_length must be a multiple of 8.
+This definition implies the following:
+1. Software may support some of these optional fields and ignore the others,
+ which means that features may be backported to downstream Qemu
independently.
I don't think this belongs in the spec. Ideally, we add fields so
infrequently that backporting doesn't have to worry about backporting
field 2 while skipping field 1.
+2. Software may check @header_length, if it knows optional fields specification
+ enough (knows about the field which exceeds @header_length).
Again, I don't think this adds anything. Since we already documented
fields are optional, and that if header_length is too short, the missing
field is treated as 0, software that knows about a longer header_length
will already handle it correctly.
+3. If @header_length is higher than the highest field end that software knows,
+ it should assume that additional fields are correct, @header_length is
+ correct and keep @header_length and additional unknown fields as is on
+ rewriting the image.
+3. 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.
+
+ < ... No additional fields in the header currently ... >
+
I'm still not seeing the value in adding any of this paragraph to the
spec. Maybe in the commit message that accompanies the spec change, but
the spec is clear enough if it documents how optional header fields are
to be managed (treat as 0 if missing, preserve on write if unknown, and
with a mandated alignment to avoid having to worry about other issues).
Directly after the image header, optional sections called header extensions
can
be stored. Each extension has a structure like the following:
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
- [PATCH v7 0/2] qcow2: add zstd cluster compression, Vladimir Sementsov-Ogievskiy, 2019/10/07
- [PATCH v7 1/2] docs: improve qcow2 spec about extending image header, Vladimir Sementsov-Ogievskiy, 2019/10/07
- Re: [PATCH v7 1/2] docs: improve qcow2 spec about extending image header,
Eric Blake <=
- Re: [PATCH v7 1/2] docs: improve qcow2 spec about extending image header, Vladimir Sementsov-Ogievskiy, 2019/10/08
- Re: [PATCH v7 1/2] docs: improve qcow2 spec about extending image header, Vladimir Sementsov-Ogievskiy, 2019/10/18
- Re: [PATCH v7 1/2] docs: improve qcow2 spec about extending image header, Vladimir Sementsov-Ogievskiy, 2019/10/18
- Re: [PATCH v7 1/2] docs: improve qcow2 spec about extending image header, Eric Blake, 2019/10/18
- Re: [PATCH v7 1/2] docs: improve qcow2 spec about extending image header, Vladimir Sementsov-Ogievskiy, 2019/10/18
[PATCH v7 2/2] docs: qcow2: introduce compression type feature, Vladimir Sementsov-Ogievskiy, 2019/10/07