From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47497) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aBFIK-0003Bg-VS for qemu-devel@nongnu.org; Tue, 22 Dec 2015 00:15:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aBFIG-0006uM-GD for qemu-devel@nongnu.org; Tue, 22 Dec 2015 00:15:32 -0500 Received: from relay.parallels.com ([195.214.232.42]:57120) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aBFIG-0006uC-2f for qemu-devel@nongnu.org; Tue, 22 Dec 2015 00:15:28 -0500 Message-ID: <5678DC5E.5090501@virtuozzo.com> Date: Tue, 22 Dec 2015 08:15:10 +0300 From: Vladimir Sementsov-Ogievskiy MIME-Version: 1.0 References: <1450711532-14196-1-git-send-email-vsementsov@virtuozzo.com> <567891AC.6070609@redhat.com> In-Reply-To: <567891AC.6070609@redhat.com> Content-Type: text/plain; charset="iso-8859-15"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 RFC] spec: add qcow2 bitmaps extension specification List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz , qemu-devel@nongnu.org Cc: kwolf@redhat.com, famz@redhat.com, stefanha@redhat.com, den@openvz.org, jsnow@redhat.com On 22.12.2015 02:56, Max Reitz wrote: > On 21.12.2015 16:25, Vladimir Sementsov-Ogievskiy wrote: >> The new feature for qcow2: storing bitmaps. >> >> Only bitmaps, relative to the virtual disk, stored in qcow2 file, should >> be stored in this qcow2 file. >> >> Strings started from +# are RFC-strings, not to be commited of course >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy >> --- >> >> >> 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 | 160 ++++++++++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 159 insertions(+), 1 deletion(-) > Looks good! :-) Thank you! ) > > Some comments below, but I think the general design is good now. > >> diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt >> index 121dfc8..3d557ee 100644 >> --- a/docs/specs/qcow2.txt >> +++ b/docs/specs/qcow2.txt >> @@ -103,7 +103,19 @@ 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 is responsible for Bitmaps extension >> + consistency. >> + >> + If it is set, but there is no Bitmaps >> + extension, this should be considered as an >> + error. >> + >> + If it is not set, but there is a Bitmaps >> + extension, its data should be considered as >> + inconsistent. >> + >> + Bits 1-63: Reserved (set to 0) >> >> 96 - 99: refcount_order >> Describes the width of a reference count block entry (width >> @@ -123,6 +135,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 +179,34 @@ the header extension data. Each entry look like this: >> terminated if it has full length) >> >> >> +== Bitmaps extension == >> + >> +Bitmaps extension is an optional header 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. >> + >> +The data of the extension should be considered as consistent only if >> +corresponding auto-clear feature bit is set (see autoclear_features above). >> + >> +The fields of Bitmaps extension are: >> + >> + 0 - 3: nb_bitmaps >> + The number of bitmaps contained in the image. Must be >> + greater 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 must be equal to >> + sum of sizes of all (nb_bitmaps) bitmap headers. > I'd rather write this as: "Size of the Bitmap Directory in bytes, i.e. > the cumulative size of all (nb_bitmaps) bitmap headers." > > ("It must" sounds like it's an additional restriction while it's > actually just an explanation.) > >> + >> + 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 +401,120 @@ Snapshot table entry: >> >> variable: Padding to round up the snapshot table entry size to the >> next multiple of 8. >> + >> + >> +== Bitmaps == >> + >> +The feature supports storing bitmaps in a qcow2 image. 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. 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. >> + >> +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: >> + 0: Dirty Tracking Bitmap >> + >> + Values 1 - 255 are reserved. >> +# Is there any reason, to make it '1 - Dirty Tracking Bitmap, 0,2-255 - reserved?' > Maybe something arbitrary like "But 0 feels like 'unknown' to me!". But > I can't think of anything substantial. > >> + >> + 17: granularity_bits >> + Granularity bits. Valid values are: 0 - 63. >> + >> + Note: Qemu currently doesn't support granularity_bits >> + greater than 31. >> + >> + Granularity is calculated as >> + granularity = 1 << granularity_bits >> + >> + Granularity of the bitmap is how many bytes of the image >> + accounts for one bit of the bitmap. >> + >> + 18 - 19: name_size >> + Size of the bitmap name. Valid values: 1 - 1023. >> + >> + 20 - 21: extra_data_size >> + Size of type-specific extra data. >> +# 2 bytes or 4 bytes ? > I'd make it 4. I don't think those additional two bytes will hurt > anybody, and the simple fact that you're asking that question shows to > me that there is a possibility that 2 won't be enough. ;-) > >> + >> + variable: The name of the bitmap (not null terminated). Must be >> + unique among all bitmap names within the Bitmaps extension. >> + >> + variable: Type-specific extra data for the bitmap. > I'd swap name and type-specific data so we know the alignment of the > type-specific data (if they are swapped and sizeof(extra_data_size) == > 4, its alignment will be 4). Maybe that'll come in handy at some point. Good thing, thanks. It will be even 8) > > Maybe you should make a note that currently, no extra data is defined > for Dirty Tracking Bitmaps, but that this may change in the future. > > Max > >> + 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 (not two-level like refcounts and guest >> +clusters mapping) structure for the mapping of bitmaps data to host clusters. >> +It is called 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. >> + >> +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 host cluster offset. Must be aligned to a >> + cluster boundary. If the offset is 0, the cluster is >> + unallocated, see bit 0 description. >> + >> + 56 - 63: Reserved and must be zero. >> + >> +=== Bitmap Data === >> + >> +As noted above, bitmap data is stored in several (or may be one, exactly >> +bitmap_table_size) separate clusters, described by 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) >> + >> +Taking into account the granularity of the bitmap, an offset in bits into the >> +image file, corresponding to byte number byte_nr of the virtual disk can be >> +calculated like this: >> + >> + bit_offset = >> + image_offset(byte_nr / granularity / 8) * 8 + >> + (byte_nr / granularity) % 8 >> > -- Best regards, Vladimir * now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.