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

[Eric Blake <eblake@redhat.com>]

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

On 07/31/2012 10:51 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 <wdongxu@linux.vnet.ibm.com>
> ---
> Now add-cow is still using QEMUOptionParameter, not QemuOpts,  I will send a
> seperate patch series to convert.

s/seperate/separate/

> 
>  docs/specs/add-cow.txt |  128 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 128 insertions(+), 0 deletions(-)
>  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..4793a3e
> --- /dev/null
> +++ b/docs/specs/add-cow.txt
> @@ -0,0 +1,128 @@
> +== General ==
> +
> +Raw file format does not support backing file and copy on write feature.

grammar:
The raw file format does not support backing files or the copy-on-write
feature.

> +The add-cow image format makes it possible to use backing files with raw
> +image by keeping a separate .add-cow metadata file. Once all sectors
> +have been written into the raw image it is safe to discard the .add-cow
> +and backing files, then we can use the raw image directly.

I'm still not sure how this series fits in with the recent discussions
on adding drive-mirror with the capability of creating a raw file mirror
of a qcow2 snapshot.


> +
> +While using add-cow, procedures may like this:

grammar:
An example usage of add-cow would look like:

> +(ubuntu.img is a disk image which has been 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
> +    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 by the size of ubuntu.img, test.raw will be used from the
> +1st byte, the rest part can be used for other purpose.

Grammar was off here, but I'm not sure what you meant to suggest a
replacement.  Maybe:

the size of test.add-cow will be calculated from the size of ubuntu.img,
and extra space at the tail of test.raw can be used for other purposes.

> +(#define HEADER_SIZE (4096 * header_pages_size))
> +    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 file name offset
> +                        Offset in the add-cow file at which the backing file
> +                        name is stored (NB: The string is not null terminated).

Nit: this should be nul-terminated (NUL is the one-byte all-0 character
in single byte encodings that ends a string, and NULL is the four- or
eight-byte value, typically all-0, for a pointer to nowhere).


> +
> +            28 - 35:    features
> +                        Currently only 3 feature bit is used:
> +                        Feature bits:
> +                            The image uses a backing file:
> +                            * ADD_COW_F_BACKING_FILE    = 0x01.

Isn't this bit redundant with the earlier field at byte 12 stating
whether a backing file is present?

> +                            The image uses a image file:
> +                            * ADD_COW_F_IMAGE_FILE      = 0x02.

The field at byte 20 implies that an image file name is mandatory,
meaning this bit is always 1 and therefore pointless.

> +                            All bits in bitmap have been set to 1, add-cow wrapper
> +                            can be discarded.
> +                            * ADD_COW_F_All_ALLOCATED   = 0x04.
> +
> +            36 - 43:    optional features
> +                        Not used now. Researved for future use.

s/Researved/Reserved/, mention that it must be set to 0

> +
> +            44 - 47:    header pages size
> +                        The header field is variable-sized. This field indicates
> +                        how many pages(4k) will be used to store add-cow header.
> +                        In add-cow v1, it is fixed to 1, so the header size will
> +                        be 4k * 1 = 4096 bytes.
> +
> +Image file name and backing file name must NOT be the same, we prevent this
> +while creating add-cow files.
> +
> +Image file and backing file are interpreted relative to the qcow2 file, not
> +to the current working directory of the process that opened the qcow2 file.

Either indent this description to match the field it is describing, or
sink it down until after you have called out the header layout.

> +
> +== Reserved ==
> +
> +    Byte    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 none-zero, it must be non-zero.

s/none-zero/non-zero/

Why are you defining these byte offsets in the Reserved section?  This
text should occur earlier in the mandatory header format.  Is this field
free-form ASCII?  Must the field be NUL-terminated?  For that matter, I
think you can just delete the ==Reserved== header, as you are calling
out every possible offset.

> +
> +            64 - 79:    image file format
> +                        format of image file. It must be non-zero.

Same question about whether this field is ASCII, and must be NUL-terminated.

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

Is it required that bytes not pointed to by backing file and image names
must have any particular value?

> +
> +== COW bitmap ==
> +
> +The "COW bitmap" field starts at the 4096th byte, stores a bitmap related to
> +backing file and image file. The bitmap will track whether the sector in
> +backing file is dirty or not.
> +
> +Each bit in the bitmap indicates one cluster's status. One cluster includes 128
> +sectors, then each bit indicates 512 * 128 = 64k bytes. the size of bitmap is
> +calculated according to virtual size of backing file. In each byte, bit 0 to 7
> +will track the 1st to 7th cluster in sequence, bit orders in one byte look like:

1st to 7th is only 7 clusters.  You mean either '0th to 7th' or '1st to
8th'.  Or just simplify to:

Within each byte, the least significant bit covers the first cluster.

> + +----+----+----+----+----+----+----+----+
> + | b7 | b6 | b5 | b4 | b3 | b2 | b1 | b0 |
> + +----+----+----+----+----+----+----+----+
> +
> +If the bit is 0, indicates the sector has not been allocated in image file, data
> +should be loaded from backing file while reading; if the bit is 1, 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 raw image is not an even multiple of cluster bytes, bits that correspond to
> +bytes beyond the raw file size in add-cow will be 0.

Will this affect the use of the ADD_COW_F_ALL_ALLOCATED feature bit in
the header?

-- 
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: 620 bytes --]