Re: [PATCH v7 1/2] docs: improve qcow2 spec about extending image header

[Vladimir Sementsov-Ogievskiy]

18.10.2019 16:39, Eric Blake wrote:
> On 10/18/19 4:27 AM, Vladimir Sementsov-Ogievskiy wrote:
> 
>>>>> 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.
>>>>
>>>> Hmm, if we imply that software will have to add some padding, than 
>>>> requirement above about zero === feature-absence
>>>> becomes necessary. [*]
>>>>
>>>> Still I have two questions:
>>>> 1. Do we really need all fields to be 4 or 8 bytes? Why not use 1 byte for 
>>>> compression?
> 
> No, fields can be smaller if that makes sense; but I still think it's 
> important that fields don't straddle natural alignment boundaries. When 
> adding just one field at a time, it's easier to add a wide field and less 
> padding than a narrow field and lots of padding, if we're still striving for 
> alignment.
> 
>>>> 2. What is the benefit of padding, which you propose?
> 
> The biggest benefit to keeping large fields from straddling alignment 
> boundaries is that you can mmap() a qcow2 file and do naturally-aligned reads 
> of those large fields, rather than byte-by-byte reads, without risking SIGBUS 
> on some architectures.  (You still have to worry about endianness, but that's 
> true regardless of alignment)
> 
> 
>>> So, it looks inconsistent, if we pad all header extensions to  8 bytes 
>>> except for the start of the first extension.
>>>
>>> I'll resend with padding soon.
>>
>>
>> Still, we have to make an exception at least for header_length = 104, which 
>> is not a multiply of 8.
> 
> Huh?  104 == 8 * 13, so our current v3 header size is 8-byte aligned. 
> Likewise for 72 == 8 * 9 for v2 header size.

Ahahahaha, shame on my head:) I should go to school again.

> 
>>
>> Also, is requiring alignment is an incompatible change of specification?
> 
> No. I think it is just clarifying what was implicitly already  the case.  
> Remember, immediately after the header comes header extensions, and THOSE are 
> explicitly required to be multiple-of-8 in size.  That requirement makes no 
> sense unless the header itself ends on an 8-byte alignment (which it does for 
> all existing v2 and v3 images prior to our first official v3 header 
> extension).
> 

OK, let's go with it, I'll resend again.


-- 
Best regards,
Vladimir