Re: [Qemu-devel] [PATCH v7] spec: add qcow2 bitmaps extension specification

[Eric Blake <eblake@redhat.com>]

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

On 01/11/2016 06:05 AM, Vladimir Sementsov-Ogievskiy wrote:
> The new feature for qcow2: storing bitmaps.
> 
> This patch adds new header extension to qcow2 - Bitmaps Extension. It
> provides an ability to store virtual disk related bitmaps in a qcow2
> image. For now there is only one type of such bitmaps: Dirty Tracking
> Bitmap, which just tracks virtual disk changes from some moment.
> 
> Note: Only bitmaps, relative to the virtual disk, stored in qcow2 file,
> should be stored in this qcow2 file. The size of each bitmap
> (considering its granularity) is equal to virtual disk size.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> 

> @@ -166,6 +178,34 @@ the header extension data. Each entry look like this:
>                      terminated if it has full length)
>  
>  
> +== Bitmaps extension ==

> +          0 -  3:  nb_bitmaps
> +                   The number of bitmaps contained in the image. Must be
> +                   greater than or equal to 1.
> +
> +                   Note: Qemu currently only supports up to 65535 bitmaps per
> +                   image.
> +
> +          4 -  7:  bitmap_directory_size
> +                   Size of the bitmap directory in bytes. It is the cumulative
> +                   size of all (nb_bitmaps) bitmap headers.

Only 4 bytes - if we ever raise our 64k entry restriction (nb_bitmaps),
could we run into an image that has so many directory entries as to make
the directory itself spill past 4G?  But I don't think it is likely, so
I can live with your choice.

> +
> +== Bitmaps ==
> +
> +As mentioned above, the bitmaps extension provides the ability to store bitmaps
> +related a virtual disk. This section describes how these bitmaps are stored.
> +
> +Note: all bitmaps are related to the virtual disk stored in this image.
> +
> +=== Bitmap directory ===
> +
> +Each bitmap saved in the image is described in a bitmap directory entry. The
> +bitmap directory is a contiguous area in the image file, whose starting offset
> +and length are given by the header extension fields bitmap_directory_offset and
> +bitmap_directory_size. The entries of the bitmap directory have variable
> +length, depending on the length of the bitmap name and extra data. These
> +entries are also called bitmap headers.
> +
> +Structure of a bitmap directory entry:
> +
> +    Byte 0 -  7:    bitmap_table_offset
> +                    Offset into the image file at which the bitmap table
> +                    (described below) for the bitmap starts. Must be aligned to
> +                    a cluster boundary.
> +
> +         8 - 11:    bitmap_table_size
> +                    Number of entries in the bitmap table of the bitmap.

Should this be the size in bytes, instead of the number of entries? But
at least the entries are fixed width of 8 bytes each, so this lets you
get a bitmap table up to 32G bytes rather than just 4G in size.  (Let's
see here - if we have 32G bytes in the bitmap table, that means 4G
clusters occupied by the bitmap itself; in the worst case of 512-byte
clusters and granularity 0, that is a maximum bitmap size of 2T
describing 16T of virtual guest image; with larger cluster size and/or
larger granularity, we cover a lot more virtual guest space with less
bitmap size; so I guess we aren't too worried about running out of space?).

> +        20 - 23:    extra_data_size
> +                    Size of type-specific extra data.
> +
> +                    For now, as no extra data is defined, extra_data_size is
> +                    reserved and must be zero.
> +
> +        variable:   Type-specific extra data for the bitmap.

I'd write this as:
           variable:   extra_data
                       Type-specific extra data for the bitmap,
                       occupying extra_data_size bytes.

> +
> +        variable:   The name of the bitmap (not null terminated). Must be
> +                    unique among all bitmap names within the bitmaps extension.
> +
> +        variable:   Padding to round up the bitmap directory entry size to the
> +                    next multiple of 8.

Should we require the padding to be all NUL bytes?  (We aren't
consistent on whether we require that for other locations of padding in
the spec, so that could be a followup patch).

> +
> +=== Bitmap table ===
> +
> +Bitmaps are stored using a one-level structure (as opposed to two-level
> +structure like for refcounts and guest clusters mapping) for the mapping of
> +bitmap data to host clusters. This structure is called the bitmap table.

Possible wording tweak:
Bitmaps are stored using a one-level structure (as opposed to the
two-level structures for refcounts and guest cluster mapping), and are
used for the mapping of bitmap data to host clusters

> +
> +Each bitmap table has a variable size (stored in the bitmap directory Entry)

Does 'Entry' still need to be capitalized?

> +and may use multiple clusters, however, it must be contiguous in the image
> +file.
> +
> +Structure of a bitmap table entry:
> +
> +    Bit       0:    Reserved and must be zero if bits 9 - 55 are non-zero.
> +                    If bits 9 - 55 are zero:
> +                      0: Cluster should be read as all zeros.
> +                      1: Cluster should be read as all ones.
> +
> +         1 -  8:    Reserved and must be zero.
> +
> +         9 - 55:    Bits 9 - 55 of the host cluster offset. Must be aligned to
> +                    a cluster boundary. If the offset is 0, the cluster is
> +                    unallocated; in that case, bit 0 determines how this
> +                    cluster should be treated when read from.

Possible wording tweak:
s/when read from/during reads/.

> +
> +        56 - 63:    Reserved and must be zero.
> +
> +=== Bitmap data ===
> +
> +As noted above, bitmap data is stored in separate clusters, described by the
> +bitmap table. Given an offset (in bytes) into the bitmap data, the offset into
> +the image file can be obtained as follows:
> +
> +    image_offset =
> +        bitmap_table[bitmap_data_offset / cluster_size] +
> +            (bitmap_data_offset % cluster_size)
> +
> +This offset is not defined if bits 9 - 55 of bitmap table entry are zero (see
> +above).
> +
> +Given an offset byte_nr into the virtual disk and the bitmap's granularity, the
> +bit offset into the bitmap can be calculated like this:
> +
> +    bit_offset =
> +        image_offset(byte_nr / granularity / 8) * 8 +
> +            (byte_nr / granularity) % 8
> +
> +If the size of the bitmap data is not a multiply of cluster size then the last

s/multiply of cluster size/multiple of the cluster size,/

> +cluster of the bitmap data contains some unused tail bits. These bits must be
> +zero.
> 

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