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

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

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