Am 06.03.2019 um 13:43 hat Eric Blake geschrieben: > 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.
Maybe add a new section "String header extensions" that covers both? If this remains the only patch in the series that would need a significant change, I'd prefer a follow-up patch indeed. Kevin
signature.asc
Description: PGP signature
