Re: [Qemu-devel] [PATCH v2] persistent dirty bitmap: add QDB file spec.

[Eric Blake]

On 11/20/2014 03:34 AM, Vladimir Sementsov-Ogievskiy wrote:
> QDB file is for storing dirty bitmap. The specification is based on
> qcow2 specification.
> 
> Saving several bitmaps is necessary when server shutdowns during
> backup. In this case 2 tables for each disk are available. One
> collected for a previous period and one active. Though this feature
> is discussable.
> 
> Big endian format and Standard Cluster Descriptor are used to simplify
> integration with qcow2, to support internal bitmaps for qcow2 in future.
> 
> The idea is that the same procedure writing the data to QDB file could
> do the same for QCOW2. The only difference is cluster refcount table.
> Should we use it here or not is still questionable.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>  docs/specs/qdb.txt | 132 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 132 insertions(+)
>  create mode 100644 docs/specs/qdb.txt

No comment on whether the approach itself makes sense - just a
high-level review of this document in isolation.

> 
> diff --git a/docs/specs/qdb.txt b/docs/specs/qdb.txt
> new file mode 100644
> index 0000000..d570a69
> --- /dev/null
> +++ b/docs/specs/qdb.txt
> @@ -0,0 +1,132 @@
> +== General ==

Missing a copyright notice.  Yeah, you've got a lot of bad examples in
this directory (in docs/* in general), but there ARE a few of the newer
files that are starting to buck the trend and use a copyright/license blurb.

> +
> +"QDB" means "Qemu Dirty Bitmaps". QDB file can store several dirty bitmaps.
> +QDB file is organized in units of constant size, which are called clusters.
> +
> +All numbers in QDB are stored in Big Endian byte order.
> +
> +== Header ==
> +
> +The first cluster of a QDB image contains the file header:
> +
> +    Byte  0 -  3:   magic
> +                    QDB magic string ("QDB\0")
> +
> +          4 -  7:   version
> +                    Version number (valid value is 1)
> +
> +          8 - 11:   cluster_bits
> +                    Number of bits that are used for addressing an offset
> +                    within a cluster (1 << cluster_bits is the cluster size).
> +                    Must not be less than 9 (i.e. 512 byte clusters).

Is there a maximum?

> +
> +         12 - 15:   nb_bitmaps
> +                    Number of bitmaps contained in the file
> +
> +         16 - 23:   bitmaps_offset
> +                    Offset into the QDB file at which the bitmap table 
> starts.
> +                    Must be aligned to a cluster boundary.
> +
> +         24 - 27:   header_length
> +                    Length of the header structure in bytes.

does that include the length of all extensions?  Should we enforce a
maximum header length of one cluster?

> +
> +Like in qcow2, directly after the image header, optional sections called 
> header extensions can
> +be stored. Each extension has a structure like the following:
> +
> +    Byte  0 -  3:   Header extension type:
> +                        0x00000000 - End of the header extension area
> +                        other      - Unknown header extension, can be safely
> +                                     ignored
> +
> +          4 -  7:   Length of the header extension data
> +
> +          8 -  n:   Header extension data
> +
> +          n -  m:   Padding to round up the header extension size to the next
> +                    multiple of 8.
> +
> +Unless stated otherwise, each header extension type shall appear at most once
> +in the same image.

I like how qcow2 v3 has a header extension for listing the name of each
header extension, for nicer error messages.  Also, I think that
declaring all unknown extensions as ignorable may be dangerous, since
you lack a capability bitmask.  Maybe it would be wise to copy the qcow2
v3 capabilities (including flags for ignorable vs. mandatory support of
given features, where a client can sanely decide what to do if it does
not recognize a feature).

> +
> +        26 - 27:    Size of the bitmap name
> +
> +        36 - 39:    Size of extra data in the table entry (used for future
> +                    extensions of the format)
> +
> +        variable:   Extra data for future extensions. Unknown fields must be
> +                    ignored.

This block is width 0 if bytes 36-39 is 0?  How are extensions
identified?  Are they required to be done like overall file headers,
with an id, length, and then variable data, so that it is possible to
scan to the end of each unknown extension to see if the next extension
is known?  This is where capability bits in the overall header may make
more sense.

> +
> +        variable:   Name of the bitmap (not null terminated)

The length of this block is determined by bytes 26-27?

> +
> +        variable:   Padding to round up the bitmap table entry size to the
> +                    next multiple of 8.
> +
> +The fields "size", "granularity", "enabled" and "name" are corresponding with
> +the fields in struct BdrvDirtyBitmap.

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

signature.asc
Description: OpenPGP digital signature