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

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 5530 bytes --]

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 <wdongxu@linux.vnet.ibm.com>
> ---
> +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?

> +
> +            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.
> 

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]