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

[Dong Xu Wang]

On Thu, Oct 25, 2012 at 10:56 PM, Eric Blake <address@hidden> wrote:
> On 10/25/2012 07:36 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 <address@hidden>
>> ---
>> +An example usage of add-cow would look like::
>> +(ubuntu.img is a disk image which has an installed OS.)
>> +    1)  Create a raw image 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
>
> This example does not specify the file format of either the backing_file
> or the image_file, yet[1]...
>
>>
>> +            8  - 11:    backing file name offset
>> +                        Offset in the add-cow file at which the backing file
>> +                        name is stored (NB: The string is not 
>> nul-terminated).
>
> Correct spelling of nul-terminated, but...[2]
>
>>
>> +            16 - 19:    image file name offset
>> +                        Offset in the add-cow file at which the image file 
>> name
>> +                        is stored (NB: The string is not null terminated). 
>> It
>
> [2]...here, you used the wrong spelling...
>
>> +            28 - 35:    features
>> +                        Bitmask of features. If a feature bit set that can 
>> not
>> +                        be recognized, the add-cow file should be droped. 
>> They are not
>> +                        used in v1.
>
> If a feature bit is set but not recognized, the add-cow file should be
> dropped.
>
>> +
>> +            48 - 63:    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.
>
> [2]...and here, you used different capitalization.  I think I prefer
> NUL-terminated in all cases.
>
>> +
>> +            64 - 79:    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.
>
> [1]...here you claim that backing and image file format are mandatory
> (must not be empty).  Shouldn't you allow the file format to be empty,
> in which case qemu will probe?  And why do you even need image file
> format - isn't the whole point of add-cow to wrap a raw image file, or
> are you planning on also being able to wrap non-raw files?  Are there
> other non-raw file formats that lack backing file support, where add-cow
> can be used to give it a backing file?
>

Kevin or Stefan, can you give me some opinion? Thanks.

>> +
>> +            80 - [HEADER_SIZE - 1]:
>> +                        It is used to make sure COW bitmap field starts at 
>> the
>> +                        HEADER_SIZE byte, backing file name and image file 
>> name
>> +                        will be stored here. The bytes that is not pointing 
>> to
>
> s/is/are/
>
>> +                        backing file and image file names must be set to 0.
>> +
>> +== COW bitmap ==
>> +
>> +The "COW bitmap" field starts at offset HEADER_SIZE, stores a bitmap 
>> related to
>> +backing file and image file. The bitmap will track whether the sector in
>> +backing file is dirty or not.
>
> Rather, it is tracking whether the sector in image file is allocated or not.
>
>> +
>> +Each bit in the bitmap tracks one cluster's status. For example, if cluster
>> +bit is 16, then each bit tracks one cluster, (1 << 16) = 65536 bytes. The
>> +image file size is rounded up to cluster size (where any bytes in the
>> +last cluster that do not fit in the image are ignored), then if the
>> +number of clusters is not a multiple of 8, then remaining bits in the
>> +bitmap will be set to 0.
>> +
>> +The size of bitmap is calculated according to virtual size of image file,and
>
> s/file,and/file, and/
>
>> +the size of bitmap should be multiple of add-cow file's cluster size, the 
>> bits
>> +not used will be set to 0. Within each byte, the least significant bit 
>> covers
>> +the first cluster. Bit orders in one byte look like:
>> + +----+----+----+----+----+----+----+----+
>> + | b7 | b6 | b5 | b4 | b3 | b2 | b1 | b0 |
>> + +----+----+----+----+----+----+----+----+
>> +
>> +If the bit is 0, it indicates the sector has not been allocated in image 
>> file,
>> +data should be loaded from backing file while reading; if the bit is 1, it
>> +indicates the related sector has been dirty, should be loaded from image 
>> file
>> +while reading. Writing to a sector causes the corresponding bit to be set 
>> to 1.
>> +If there is no backing file, or if the image file is larger than the backing
>> +file and the offset is beyond the end of the backing file, then the data 
>> should
>> +be read as all zero bytes instead.
>> +
>> +If raw image is not an even multiple of cluster bytes, bits that correspond 
>> to
>> +bytes beyond the raw file size in add-cow must be written as 0 and must be
>> +ignored when reading.
>> +
>> +Image file name and backing file name must NOT be the same, we prevent this
>> +while creating add-cow files via qemu-img. If image file name and backing 
>> file
>> +name are the same, the add-cow image must be treated as invalid.
>>
>

Thank you Eric, I will correct these in next version.

> --
> Eric Blake   address@hidden    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>