On 01/14/2016 06:35 AM, Denis V. Lunev wrote: > On 01/12/2016 03:30 AM, John Snow wrote: >> >> On 01/11/2016 08: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 <vsement...@virtuozzo.com> >>> --- >>> >>> v7: >>> >>> - Rewordings, grammar. >>> Max, Eric, John, thank you very much. >>> >>> - add last paragraph: remaining bits in bitmap data clusters must be >>> zero. >>> >>> - s/Bitmap Directory/bitmap directory/ and other names like this at >>> the request of Max. >>> >>> v6: >>> >>> - reword bitmap_directory_size description >>> - bitmap type: make 0 reserved >>> - extra_data_size: resize to 4bytes >>> Also, I've marked this field as "must be zero". We can always change >>> it, if we decide allowing managing app to specify any extra data, by >>> defining some magic value as a top of user extra data.. So, for now >>> non zeor extra_data_size should be considered as an error. >>> - swap name and extra_data to give good alignment to extra_data. >>> >>> >>> v5: >>> >>> - 'Dirty bitmaps' renamed to 'Bitmaps', as we may have several types of >>> bitmaps. >>> - rewordings >>> - move upper bounds to "Notes about Qemu limits" >>> - s/should/must somewhere. (but not everywhere) >>> - move name_size field closer to name itself in bitmap header >>> - add extra data area to bitmap header >>> - move bitmap data description to separate section >>> >>> docs/specs/qcow2.txt | 172 >>> ++++++++++++++++++++++++++++++++++++++++++++++++++- >>> 1 file changed, 171 insertions(+), 1 deletion(-) >>> >>> diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt >>> index 121dfc8..997239d 100644 >>> --- a/docs/specs/qcow2.txt >>> +++ b/docs/specs/qcow2.txt >>> @@ -103,7 +103,18 @@ in the description of a field. >>> write to an image with unknown auto-clear >>> features if it >>> clears the respective bits from this field first. >>> - Bits 0-63: Reserved (set to 0) >>> + Bit 0: Bitmaps extension bit >>> + This bit indicates consistency for >>> the bitmaps >>> + extension data. >>> + >>> + It is an error if this bit is set >>> without the >>> + bitmaps extension present. >>> + >>> + If the bitmaps extension is present >>> but this >>> + bit is unset, the bitmaps extension >>> data is >>> + inconsistent. >>> + >>> + Bits 1-63: Reserved (set to 0) >>> 96 - 99: refcount_order >>> Describes the width of a reference count block >>> entry (width >>> @@ -123,6 +134,7 @@ be stored. Each extension has a structure like >>> the following: >>> 0x00000000 - End of the header extension area >>> 0xE2792ACA - Backing file format name >>> 0x6803f857 - Feature name table >>> + 0x23852875 - Bitmaps extension >>> other - Unknown header extension, can >>> be safely >>> ignored >>> @@ -166,6 +178,34 @@ the header extension data. Each entry look >>> like this: >>> terminated if it has full length) >>> +== Bitmaps extension == >>> + >>> +The bitmaps extension is an optional header extension. It provides >>> the ability >>> +to store bitmaps related to a virtual disk. For now, there is only >>> one bitmap >>> +type: the dirty tracking bitmap, which tracks virtual disk changes >>> from some >>> +point in time. >>> + >>> +The data of the extension should be considered consistent only if the >>> +corresponding auto-clear feature bit is set, see autoclear_features >>> above. >>> + >>> +The fields of the bitmaps extension are: >>> + >>> + 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. >>> + >>> + 8 - 15: bitmap_directory_offset >>> + Offset into the image file at which the bitmap >>> directory >>> + starts. Must be aligned to a cluster boundary. >>> + >>> + >>> == Host cluster management == >>> qcow2 manages the allocation of host clusters by maintaining a >>> reference count >>> @@ -360,3 +400,133 @@ Snapshot table entry: >>> variable: Padding to round up the snapshot table entry >>> size to the >>> next multiple of 8. >>> + >>> + >>> +== 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. >>> + >>> + 12 - 15: flags >>> + Bit >>> + 0: in_use >>> + The bitmap was not saved correctly and may be >>> + inconsistent. >>> + >>> + 1: auto >>> + The bitmap must reflect all changes of the >>> virtual >>> + disk by any application that would write to >>> this qcow2 >>> + file (including writes, snapshot switching, >>> etc.). The >>> + type of this bitmap must be 'dirty tracking >>> bitmap'. >>> + >>> + Bits 2 - 31 are reserved and must be 0. >>> + >>> + 16: type >>> + This field describes the sort of the bitmap. >>> + Values: >>> + 1: Dirty tracking bitmap >>> + >>> + Values 0, 2 - 255 are reserved. >>> + >>> + 17: granularity_bits >>> + Granularity bits. Valid values: 0 - 63. >>> + >>> + Note: Qemu currently doesn't support >>> granularity_bits >>> + greater than 31. >>> + >>> + Granularity is calculated as >>> + granularity = 1 << granularity_bits >>> + >>> + A bitmap's granularity is how many bytes of the >>> image >>> + accounts for one bit of the bitmap. >>> + >>> + 18 - 19: name_size >>> + Size of the bitmap name. Must be non-zero. >>> + >>> + Note: Qemu currently doesn't support values >>> greater than >>> + 1023. >>> + >>> + 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. >>> + >>> + 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. >>> + >>> +=== 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. >>> + >>> +Each bitmap table has a variable size (stored in the bitmap >>> directory Entry) >>> +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. >>> + >>> + 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 >>> +cluster of the bitmap data contains some unused tail bits. These >>> bits must be >>> +zero. >>> >> s/multiply/multiple/. >> >> Looks good otherwise, pending the results of our discussion about what >> to do about the extra_data_size / type-specific data fields. >> >> Thanks! >> > should we re-submit? Is this version good enough to be accepted? :) > > Den
For my money, I'm happy -- though depending on how Vladimir wishes to use his type-specific data field, it may require some clarification. (I had a question over whether or not we needed versioning for this field if the plan was to add type-specific data to a type later, or if the extra data was always defined directly alongside a new type.) The 2.6 window isn't close to over yet, though, so I would personally be OK with follow-up patches. Max, Eric, any further input? --js