Re: [Qemu-devel] [PATCH 1/6 v10] docs: spec for add-cow file format

[Dong Xu Wang]

On Thu, Jun 14, 2012 at 6:47 PM, Kevin Wolf <address@hidden> wrote:
> Am 14.06.2012 05:06, schrieb Dong Xu Wang:
>> On Wed, Jun 13, 2012 at 11:10 PM, Eric Blake <address@hidden> wrote:
>>> On 06/13/2012 08:36 AM, Dong Xu Wang wrote:
>>>> Introduce a new file format:add-cow. The usage can be found at this patch.
>>>>
>>>> Signed-off-by: Dong Xu Wang <address@hidden>
>>>> ---
>>>>  docs/specs/add-cow.txt |   87 
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  1 files changed, 87 insertions(+), 0 deletions(-)
>>>>  create mode 100644 docs/specs/add-cow.txt
>
>>>> +
>>>> +== Header ==
>>>> +
>>>> +The Header is included in the first bytes:
>>>> +
>>>> +    Byte    0 -  7:     magic
>>>> +                        add-cow magic string ("ADD_COW\xff")
>>>> +
>>>> +            8 -  11:    version
>>>> +                        Version number (only valid value is 1 now)
>>>> +
>>>> +            12 - 15:    backing_filename_offset
>>>> +                        Offset in the add-cow file at which the backing 
>>>> file name
>>>> +                        is stored (NB: The string is not null 
>>>> terminated). 0 if the
>>>> +                        image doesn't have a backing file.
>>>
>>> Mention that if this is not 0, then it must be between 36 and 4094 (a
>>> file name must be at least 1 byte).  What are the semantics if the
>>> filename is relative?
>>
>> relative filename is ok, I tested it just now.
>
> I believe Eric wanted to know what a relative path means, i.e. that it's
> relative to the image file rather than relative to the working directory.
>
Now it is relative to working directory.
>>>> +
>>>> +            16 - 19:    backing_filename_size
>>>> +                        Length of the backing file name in bytes. 
>>>> Undefined if the
>>>> +                        image doesn't have a backing file.
>>>
>>> Better to require 0 if backing_filename_offset is 0, than to leave this
>>> field undefined; also if backing_filename_offset is non-zero, then this
>>> must be non-zero.  Must be less than 4096-36 to fit in the reserved part
>>> of the header.
>>>
>>
>> Okay.
>
> Does an add-cow image without a backing file even make sense?
>

Okay, I think so, it will be v11.

>>>> +            28 - 35:    features
>>>> +                        Currently only 2 feature bits are used:
>>>> +                        Feature bits:
>>>> +                            The image uses a backing file:
>>>> +                            * ADD_COW_F_BACKING_FILE = 0x01.
>>>> +                            The backing file's format is raw:
>>>> +                            * ADD_COW_F_BACKING_FORMAT_NO_PROBE = 0x02.
>>>
>>> Should this follow the qcow2v3 proposal of splitting into mandatory vs.
>>> optional feature bits?
>>>
>>> I agree that ADD_COW_F_BACKING_FORMAT_NO_PROBE is sufficient to avoid
>>> security implications, but do we want the extra flexibility of
>>> specifying the backing format file format rather than just requiring
>>> probes on all but raw?
>>
>> Kevin, or Stefan, can you give some comments for this? thanks.
>
> I tend to agree that a format name is better than relying on probing.
>
> Also, I think we need the same thing for image_file. add-cow is not only
> useful for raw images, but also for other image format types for which
> we don't support backing files.
>

Okay.

>>>> +== Reserved ==
>>>> +
>>>> +    Byte    36 - 4095:  Reserved field:
>>>> +                        It is used to make sure COW bitmap field starts 
>>>> at the
>>>> +                        4096th byte, backing_file name and image_file 
>>>> name will
>>>> +                        be stored here.
>>>
>>> Do we want to keep a fixed-size header, or should we be planning on the
>>> possibility of future extensions requiring enough other header
>>> extensions that a variable-sized header would be wiser?  That is, I'm
>>> fine with requiring that the header be a multiple of 4k, but maybe it
>>> would make sense to have a mandatory header field that states how many
>>> header pages are present before the COW bitmap begins.  In the first
>>> round of implementation, this header field can be required to be 1 (that
>>> is, for now, we require exactly 4k header), but having the field would
>>> let us change in the future to a design with an 8k header to hold more
>>> metadata as needed.
>
> I have the impression that this simple add-cow hack is starting to get
> seriously overengineered... :-)
>
> Kevin
>