On 3/6/19 3:51 AM, Stefan Hajnoczi wrote: > On Fri, Mar 01, 2019 at 10:32:27AM -0600, Eric Blake wrote: >> On 3/1/19 10:17 AM, Stefan Hajnoczi wrote: >>> On Wed, Feb 27, 2019 at 06:22:39PM +0100, Kevin Wolf wrote: >>>> @@ -148,6 +170,7 @@ be stored. Each extension has a structure like the >>>> following: >>>> 0x6803f857 - Feature name table >>>> 0x23852875 - Bitmaps extension >>>> 0x0537be77 - Full disk encryption header pointer >>>> + 0x44415441 - External data file name >>> >>> This new header extension isn't described in this patch? >> >> I asked the same on v1, and the answer (which remains valid) is that >> neither is 0xe2792aca Backing file format name. (In other words, both >> extensions are simple enough as a single file name to be implicitly >> described by the reference to the header in the earlier text). Making >> both explicit wouldn't hurt my feelings, but I don't see it as a >> showstopper to the patch as-is. > > The spec should make the representation clear. Is it a NUL-terminated > string or is the length dictated by the header extension length field?
My understanding is length determined by the header field, with optional NUL padding out to the alignment boundary (but that also means that it does NOT necessarily have a trailing NUL on disk if sizing matches alignment). But yes, being explicit never hurts. > > Otherwise implementors are forced to look at the QEMU source code or > guess based on hex dumping example files :(. Indeed, cleaning up the existing Backing file format name is worth doing (at which point this should follow suit). But it still sounds like a separate patch, at which point it becomes a question of ordering - if the cleanup lands first, then this needs to rebase to do the same; if this lands first, then the cleanup does both headers at once. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
