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

[Dong Xu Wang <wdongxu@linux.vnet.ibm.com>]

On 2013/4/27 6:45, Eric Blake wrote:
> On 04/10/2013 02:11 AM, Dong Xu Wang wrote:
>> Document for add-cow format, the usage and spec of add-cow are
>> introduced.
>>
>> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
>> ---
>> V17->V18:
>> 1) remove version field.
>> 2) header size is maximum value and cluster size value.
>> 3) fix type.
>>   docs/specs/add-cow.txt | 165 +++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 165 insertions(+)
>>   create mode 100644 docs/specs/add-cow.txt
>>
>> diff --git a/docs/specs/add-cow.txt b/docs/specs/add-cow.txt
>> new file mode 100644
>> index 0000000..151028b
>> --- /dev/null
>> +++ b/docs/specs/add-cow.txt
>> @@ -0,0 +1,165 @@
>> +== General ==
>
> No copyright notice?  Not necessarily your fault, since many other files
> in this directory suffer from the same problem.
>
Yep, all documents in docs/specs have no copyright, so I omit it.
>> +
>> +The raw file format does not support backing files or copy on write
>> +feature. The add-cow image format makes it possible to use backing
>> +files with a image by keeping a separate .add-cow metadata file.
>> +Once all clusters have been written into the image it is safe to
>> +discard the .add-cow and backing files, then we can use the image
>> +directly.
>> +
>> +An example usage of add-cow would look like:
>> +(ubuntu.img is a disk image which has an installed OS.)
>> +    1)  Create a image, such as raw format, with the same size of
>> +        ubuntu.img:
>> +            qemu-img create -f raw test.raw 8G
>> +    2)  Create an add-cow image which will store dirty bitmap
>> +            qemu-img create -f add-cow test.add-cow \
>> +                -o backing_file=ubuntu.img,image_file=test.raw
>> +    3)  Run qemu with add-cow image
>> +            qemu -drive if=virtio,file=test.add-cow
>> +
>> +test.raw may be larger than ubuntu.img, in that case, the size of
>> +test.add-cow will be calculated from the size of test.raw.
>> +
>> +image_fmt can be omitted, in that case image_fmt is assumed to be
>> +"raw". backing_fmt can also be omitted, add-cow should do a probe
>> +operation and determine what the backing file's format is.
>
> In general, probing a raw file is a security hole (we just plugged a CVE
> with NBD probing); you probably ought to mention that it is recommended
> to always specify the format for any raw file, so that probing doesn't
> misinterpret the contents of the file as some other format.
>
Okay, will mention.
>> +
>> +=Specification=
>> +
>> +The file format looks like this:
>> +
>> + +---------------+-------------------------------+
>> + |     Header    |           COW bitmap          |
>> + +---------------+-------------------------------+
>> +
>> +All numbers in add-cow are stored in Little Endian byte order.
>> +
>> +== Header ==
>> +
>> +The Header is included in the first bytes:
>> +(HEADER_SIZE is defined in 40-43 bytes.)
>> +    Byte    0  -  3:    magic
>> +                        add-cow magic string ("ACOW").
>
> Probably ought to mention that this magic string is in the ASCII
> encoding (those characters map to different bytes on EBCDIC, although I
> doubt qemu will ever really been ported to EBCDIC)
>
Okay.
>> +
>> +            4  -  7:    backing file name offset
>> +                        Offset in the add-cow file at which the backing
>> +                        file name is stored (NB: The string is not
>> +                        lNUL-terminated).
>
> s/lNUL/NUL/
>
Okay.
>> +                        If backing file name does NOT exist, this field
>> +                        will be 0. Must be between 76 and [HEADER_SIZE
>> +                        - 2](a file name must be at least 1 byte).
>> +
>
>> +
>> +            40 - 43:    HEADER_SIZE
>> +                        The header field is variable-sized. This field
>> +                        indicates how many bytes will be used to store
>> +                        add-cow header. By default, it is maximum value
>> +                        of 4096 and cluster size value.
>
> Should it be required to be a multiple of 4096, for efficient alignment
> of clusters?
>
I think I can make cluster size be multiple of 4096..
>> +
>> +            44 - 59:    backing file format
>> +                        Format of backing file. It will be filled with
>> +                        0 if backing file name offset is 0. If backing
>> +                        file name offset is non-empty, it must be
>> +                        non-empty. It is coded in free-form ASCII, and
>> +                        is not NUL-terminated. Zero padded on the right.
>
> Requiring this to be non-empty if a backing file is named contradicts
> your earlier statement that backing format is probed (I actually like
> mandating the backing format, though).

I think logic can be:
1) if "-o backing_fmt = raw" then do not probe.
2) else even if "-o backing_fmt = $fmt" is used, also perform a probe 
operation and write backing_fmt in add_cow headers.
>
>> +
>> +            60 - 75:    image file format
>> +                        Format of image file. It must be non-empty. It
>> +                        is coded in free-form ASCII, and is not
>> +                        NUL-terminated. Zero padded on the right.
>
> Again, requiring a format contradicts the earlier statement about format
> probing.
>
> How does this compare with Paolo's efforts to design a persistent bitmap
> for drive-mirror/block-backup use?
>