[PATCH] qcow2: Explicit mention of padding bytes

Eric Blake posted 1 patch 5 years, 7 months ago
Test docker-quick@centos7 failed
Test FreeBSD failed
Test docker-mingw@fedora failed
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200227144508.1078501-1-eblake@redhat.com
Maintainers: Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>
There is a newer version of this series
docs/interop/qcow2.txt | 2 ++
1 file changed, 2 insertions(+)
[PATCH] qcow2: Explicit mention of padding bytes
Posted by Eric Blake 5 years, 7 months ago
Although we already covered the need for padding bytes with our
changes in commit 3ae3fcfa, commit 66fcbca5 just added one byte and
relied on the earlier text for implicitly covering 7 padding bytes.
For consistency with other parts of the header, it does not hurt to be
explicit that this version of the header is using padding bytes, and
if we later add other extension fields, we can rework the allocation
of those padding bytes to match those additions.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 docs/interop/qcow2.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
index 5597e244746e..193ac7c5c1af 100644
--- a/docs/interop/qcow2.txt
+++ b/docs/interop/qcow2.txt
@@ -209,6 +209,8 @@ version 2.
                     Available compression type values:
                         0: zlib <https://www.zlib.net/>

+          105 - m:  Zero padding to round up the header size to the next
+                    multiple of 8.

 === Header padding ===

-- 
2.25.1


Re: [PATCH] qcow2: Explicit mention of padding bytes
Posted by Max Reitz 5 years, 7 months ago
On 27.02.20 15:45, Eric Blake wrote:
> Although we already covered the need for padding bytes with our
> changes in commit 3ae3fcfa, commit 66fcbca5 just added one byte and
> relied on the earlier text for implicitly covering 7 padding bytes.
> For consistency with other parts of the header,

Those other places are about table entries where there is padding to the
next entry of the table, though.  In addition, for those other places
it’s the only mention that they are padded.

For the header, there is a whole own section describing the padding, so
I don’t quite see the point.

> it does not hurt to be
> explicit that this version of the header is using padding bytes, and
> if we later add other extension fields, we can rework the allocation
> of those padding bytes to match those additions.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  docs/interop/qcow2.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
> index 5597e244746e..193ac7c5c1af 100644
> --- a/docs/interop/qcow2.txt
> +++ b/docs/interop/qcow2.txt
> @@ -209,6 +209,8 @@ version 2.
>                      Available compression type values:
>                          0: zlib <https://www.zlib.net/>
> 
> +          105 - m:  Zero padding to round up the header size to the next
> +                    multiple of 8.

And if we really want this, I think it might as well be specific, i.e.
“105 - 112”.  We would have to adjust it every time we add a new field
anyway.

Max

>  === Header padding ===
> 


Re: [PATCH] qcow2: Explicit mention of padding bytes
Posted by Vladimir Sementsov-Ogievskiy 5 years, 7 months ago
27.02.2020 17:45, Eric Blake wrote:
> Although we already covered the need for padding bytes with our
> changes in commit 3ae3fcfa, commit 66fcbca5 just added one byte and
> relied on the earlier text for implicitly covering 7 padding bytes.
> For consistency with other parts of the header, it does not hurt to be
> explicit that this version of the header is using padding bytes, and
> if we later add other extension fields, we can rework the allocation
> of those padding bytes to match those additions.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>   docs/interop/qcow2.txt | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
> index 5597e244746e..193ac7c5c1af 100644
> --- a/docs/interop/qcow2.txt
> +++ b/docs/interop/qcow2.txt
> @@ -209,6 +209,8 @@ version 2.
>                       Available compression type values:
>                           0: zlib <https://www.zlib.net/>
> 
> +          105 - m:  Zero padding to round up the header size to the next
> +                    multiple of 8.
> 

Hmm. Strictly speaking, you defined it as one of additional fields. And by
this definition, we may start to check in qemu that these bytes are zero,
instead of ignoring them and keeping as is..

But may be it's just a nitpicking..


-- 
Best regards,
Vladimir