From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: Eric Blake <eblake@redhat.com>,
"qemu-block@nongnu.org" <qemu-block@nongnu.org>
Cc: "kwolf@redhat.com" <kwolf@redhat.com>,
Denis Lunev <den@virtuozzo.com>,
"armbru@redhat.com" <armbru@redhat.com>,
"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
Denis Plotnikov <dplotnikov@virtuozzo.com>,
"mreitz@redhat.com" <mreitz@redhat.com>
Subject: Re: [PATCH v8 3/3] docs: qcow2: introduce compression type feature
Date: Fri, 18 Oct 2019 14:38:34 +0000 [thread overview]
Message-ID: <1696022a-fafc-64d6-7ad3-5389aee4d7f5@virtuozzo.com> (raw)
In-Reply-To: <29df7fb8-8218-2228-9575-e84e369a7d9a@redhat.com>
18.10.2019 17:04, Eric Blake wrote:
> On 10/18/19 4:47 AM, Vladimir Sementsov-Ogievskiy wrote:
>> The patch add new additional field to qcow2 header: compression_type,
>> which specifies compression type. If field is absent or zero, default
>> compression type is set: ZLIB, which corresponds to current behavior.
>>
>> New compression type (ZSTD) is to be added in further commit.
>>
>> Suggested-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>> docs/interop/qcow2.txt | 19 ++++++++++++++++++-
>> 1 file changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
>> index b971e59b1a..4eabd81363 100644
>> --- a/docs/interop/qcow2.txt
>> +++ b/docs/interop/qcow2.txt
>> @@ -109,6 +109,12 @@ least next five fields, up to the @header_length field.
>> An External Data File Name header extension may
>> be present if this bit is set.
>> + Bit 3: Compression type bit. If this bit is set,
>> + non-default compression is used for compressed
>> + clusters. In this case, @header_length must
>> + be at least 105 and @compression_type field
>> + must be non-zero.
>
> I want to insist on 8-byte alignment, so this should be at least 112.
>
> Also, as pointed out in v7, use of the decoration '@header_length' and '@compression_type' is not consistent with the rest of the document. Drop the @.
Missed this, sorry(
>
>> +
>> Bits 3-63: Reserved (set to 0)
>> 80 - 87: compatible_features
>> @@ -183,7 +189,18 @@ It's allowed for the header end to cut some field in the middle (in this case
>> the field is considered as absent), but in this case the part of the field
>> which is covered by @header_length must be zeroed.
>> - < ... No additional fields in the header currently ... >
>> + 104: compression_type
>> + Defines the compression method used for compressed clusters.
>> + A single compression type is applied to all compressed image
>> + clusters.
>> + If incompatible compression type bit is set: the field must
>> + exist (i.e. @header_length >= 105) and must be non-zero (
>> + which means non-zlib compression type)
>> + If incompatible compression type bit is unset: the field
>> + may not exist (if @header_length < 105) or it must be zero
>> + (which means zlib).
>> + Available compression type values:
>> + 0: zlib <https://www.zlib.net/>
>
> One byte for the field is fine, but I'd still explicitly document 7 bytes of unused padding, since I want to insist on an 8-byte multiple.
>
OK
--
Best regards,
Vladimir
prev parent reply other threads:[~2019-10-18 14:39 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-18 9:47 [PATCH v8 0/3] qcow2: add zstd cluster compression Vladimir Sementsov-Ogievskiy
2019-10-18 9:47 ` [PATCH v8 1/3] docs: improve qcow2 spec about extending image header Vladimir Sementsov-Ogievskiy
2019-10-18 14:00 ` Eric Blake
2019-10-18 14:36 ` Vladimir Sementsov-Ogievskiy
2019-11-06 16:17 ` Vladimir Sementsov-Ogievskiy
2019-11-06 19:19 ` Eric Blake
2019-11-07 12:26 ` Vladimir Sementsov-Ogievskiy
2019-12-02 14:14 ` Vladimir Sementsov-Ogievskiy
2019-10-18 9:47 ` [PATCH v8 2/3] docs: define padding for qcow2 header Vladimir Sementsov-Ogievskiy
2019-10-18 14:02 ` Eric Blake
2019-10-18 14:37 ` Vladimir Sementsov-Ogievskiy
2019-10-18 9:47 ` [PATCH v8 3/3] docs: qcow2: introduce compression type feature Vladimir Sementsov-Ogievskiy
2019-10-18 14:04 ` Eric Blake
2019-10-18 14:38 ` Vladimir Sementsov-Ogievskiy [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1696022a-fafc-64d6-7ad3-5389aee4d7f5@virtuozzo.com \
--to=vsementsov@virtuozzo.com \
--cc=armbru@redhat.com \
--cc=den@virtuozzo.com \
--cc=dplotnikov@virtuozzo.com \
--cc=eblake@redhat.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).