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
