[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v1 1/8] qcow2: introduce compression type feature
From: |
Eric Blake |
Subject: |
Re: [PATCH v1 1/8] qcow2: introduce compression type feature |
Date: |
Thu, 27 Feb 2020 08:39:05 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 2/27/20 8:30 AM, Vladimir Sementsov-Ogievskiy wrote:
But what is the benefit? We have already documented padding in the
spec, and discussed it so much time... What is the problem with
padding? And why to add 8 bytes for every new feature which needs
only one byte?
Okay, so requiring an 8-byte field is not necessary. But still, at
least mentioning padding bytes (that may be assigned meanings later)
is consistent with the rest of the document (for example, we have
padding bits documented for the compatible/incompatible/autoclear
feature bits), and reminds implementers to keep size rounded to a
multiple of 8.
Yes, we can add something about it.. But I'm not sure we need, and I
can't imaging correct short wording.
We have section about padding:
"
=== Header padding ===
@header_length must be a multiple of 8, which means that if the end of
the last
additional field is not aligned, some padding is needed. This padding
must be
zeroed, so that if some existing (or future) additional field will fall
into
the padding, it will be interpreted accordingly to point [3.] of the
previous
paragraph, i.e. in the same manner as when this field is not present.
"
So, if we want to add something about 104-111, it should be added to
this section, not to previous "=== Additional fields (version 3 and
higher) ===".
And, if we want, to add something, we should consider both cases when
compression type field exists and when not... What to write?
"105 - 111: These bytes are padding, if header length > 104. May be
turned into new additional fields in future."
Sounds a bit strange... Keeping in mind that different versions of qemu
may consider the same bytes as additional field or as padding, and it is
correct.
Looking at the header extension, it can probably be as simple as:
105 - m: Zero padding to round up the header size to the next
multiple of 8.
I guess I'll propose a patch to make my idea concrete.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org