[PATCH for-5.0? v2] qcow2: Explicit mention of padding bytes

Eric Blake posted 1 patch 4 years, 1 month ago
Failed in applying to current master (apply log)
docs/interop/qcow2.txt | 1 +
1 file changed, 1 insertion(+)
[PATCH for-5.0? v2] qcow2: Explicit mention of padding bytes
Posted by Eric Blake 4 years, 1 month 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 rest of the text for implicitly covering 7 padding
bytes.  For consistency with other parts of the header (such as the
header extension format listing padding from n - m, or the snapshot
table entry mentioning variable padding), we might as well call out
the remaining 7 bytes as padding until such time (as any) as they gain
another meaning.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

v2: Call out explicit byte range rather than '105 - m' [Max]

Safe for 5.0 as it is just a doc fix, but only if we actually want it.

 docs/interop/qcow2.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
index 640e0eca4000..80728bc2008d 100644
--- a/docs/interop/qcow2.txt
+++ b/docs/interop/qcow2.txt
@@ -210,3 +210,4 @@ version 2.
                     Available compression type values:
                         0: zlib <https://www.zlib.net/>

+        105 - 111:  Padding, leave as zero.

 === Header padding ===

-- 
2.26.0.rc2


Re: [PATCH for-5.0? v2] qcow2: Explicit mention of padding bytes
Posted by Vladimir Sementsov-Ogievskiy 4 years, 1 month ago
03.04.2020 21:19, 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 rest of the text for implicitly covering 7 padding
> bytes.  For consistency with other parts of the header (such as the
> header extension format listing padding from n - m, or the snapshot
> table entry mentioning variable padding), we might as well call out
> the remaining 7 bytes as padding until such time (as any) as they gain
> another meaning.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> 
> v2: Call out explicit byte range rather than '105 - m' [Max]
> 
> Safe for 5.0 as it is just a doc fix, but only if we actually want it.
> 
>   docs/interop/qcow2.txt | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
> index 640e0eca4000..80728bc2008d 100644
> --- a/docs/interop/qcow2.txt
> +++ b/docs/interop/qcow2.txt
> @@ -210,3 +210,4 @@ version 2.
>                       Available compression type values:
>                           0: zlib <https://www.zlib.net/>
> 
> +        105 - 111:  Padding, leave as zero.
> 

Looking on this in separate, I'd make a software which will zero this padding unconditionally. However, if it's an existing image which we just open, we should keep the content we read.. On the other hand, of course, if read the whole spec, everything is clear.


-- 
Best regards,
Vladimir

Re: [PATCH for-5.0? v2] qcow2: Explicit mention of padding bytes
Posted by Eric Blake 4 years, 1 month ago
On 4/6/20 3:50 AM, Vladimir Sementsov-Ogievskiy wrote:
> 03.04.2020 21:19, 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 rest of the text for implicitly covering 7 padding
>> bytes.  For consistency with other parts of the header (such as the
>> header extension format listing padding from n - m, or the snapshot
>> table entry mentioning variable padding), we might as well call out
>> the remaining 7 bytes as padding until such time (as any) as they gain
>> another meaning.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>
>> v2: Call out explicit byte range rather than '105 - m' [Max]
>>
>> Safe for 5.0 as it is just a doc fix, but only if we actually want it.
>>
>>   docs/interop/qcow2.txt | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
>> index 640e0eca4000..80728bc2008d 100644
>> --- a/docs/interop/qcow2.txt
>> +++ b/docs/interop/qcow2.txt
>> @@ -210,3 +210,4 @@ version 2.
>>                       Available compression type values:
>>                           0: zlib <https://www.zlib.net/>
>>
>> +        105 - 111:  Padding, leave as zero.
>>
> 
> Looking on this in separate, I'd make a software which will zero this 
> padding unconditionally. However, if it's an existing image which we 
> just open, we should keep the content we read.. On the other hand, of 
> course, if read the whole spec, everything is clear.

Maybe:
   105 - 111: Padding, contents defined below

rather than an explicit mention of setting to 0?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


Re: [PATCH for-5.0? v2] qcow2: Explicit mention of padding bytes
Posted by Vladimir Sementsov-Ogievskiy 4 years ago
06.04.2020 16:32, Eric Blake wrote:
> On 4/6/20 3:50 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 03.04.2020 21:19, 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 rest of the text for implicitly covering 7 padding
>>> bytes.  For consistency with other parts of the header (such as the
>>> header extension format listing padding from n - m, or the snapshot
>>> table entry mentioning variable padding), we might as well call out
>>> the remaining 7 bytes as padding until such time (as any) as they gain
>>> another meaning.
>>>
>>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>> ---
>>>
>>> v2: Call out explicit byte range rather than '105 - m' [Max]
>>>
>>> Safe for 5.0 as it is just a doc fix, but only if we actually want it.
>>>
>>>   docs/interop/qcow2.txt | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
>>> index 640e0eca4000..80728bc2008d 100644
>>> --- a/docs/interop/qcow2.txt
>>> +++ b/docs/interop/qcow2.txt
>>> @@ -210,3 +210,4 @@ version 2.
>>>                       Available compression type values:
>>>                           0: zlib <https://www.zlib.net/>
>>>
>>> +        105 - 111:  Padding, leave as zero.
>>>
>>
>> Looking on this in separate, I'd make a software which will zero this padding unconditionally. However, if it's an existing image which we just open, we should keep the content we read.. On the other hand, of course, if read the whole spec, everything is clear.
> 
> Maybe:
>    105 - 111: Padding, contents defined below

Works for me, no objections)

> 
> rather than an explicit mention of setting to 0?
> 


-- 
Best regards,
Vladimir