qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [Qemu-block] [PATCH 03/20] qcow2: Extend spec for exter


From: Eric Blake
Subject: Re: [Qemu-devel] [Qemu-block] [PATCH 03/20] qcow2: Extend spec for external data files
Date: Wed, 6 Mar 2019 06:43:30 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1

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

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]