From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37087) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aBAJi-0001Qj-EE for qemu-devel@nongnu.org; Mon, 21 Dec 2015 18:56:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aBAJe-00030M-Bq for qemu-devel@nongnu.org; Mon, 21 Dec 2015 18:56:38 -0500 Received: from mx1.redhat.com ([209.132.183.28]:37965) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aBAJe-00030F-1l for qemu-devel@nongnu.org; Mon, 21 Dec 2015 18:56:34 -0500 References: <1450711532-14196-1-git-send-email-vsementsov@virtuozzo.com> From: Max Reitz Message-ID: <567891AC.6070609@redhat.com> Date: Tue, 22 Dec 2015 00:56:28 +0100 MIME-Version: 1.0 In-Reply-To: <1450711532-14196-1-git-send-email-vsementsov@virtuozzo.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="6tdEnwUBv5MRrH60EFPlb3SCwSD55nMrc" 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: Vladimir Sementsov-Ogievskiy , qemu-devel@nongnu.org Cc: kwolf@redhat.com, famz@redhat.com, stefanha@redhat.com, den@openvz.org, jsnow@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --6tdEnwUBv5MRrH60EFPlb3SCwSD55nMrc Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 21.12.2015 16:25, Vladimir Sementsov-Ogievskiy wrote: > The new feature for qcow2: storing bitmaps. >=20 > Only bitmaps, relative to the virtual disk, stored in qcow2 file, shoul= d > be stored in this qcow2 file. >=20 > Strings started from +# are RFC-strings, not to be commited of course >=20 > Signed-off-by: Vladimir Sementsov-Ogievskiy > --- >=20 >=20 > v5: >=20 > - '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 >=20 >=20 >=20 > docs/specs/qcow2.txt | 160 +++++++++++++++++++++++++++++++++++++++++++= +++++++- > 1 file changed, 159 insertions(+), 1 deletion(-) Looks good! :-) 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. > =20 > - Bits 0-63: Reserved (set to 0) > + Bit 0: Bitmaps extension bit. > + This bit is responsible for Bitmaps ex= tension > + consistency. > + > + If it is set, but there is no Bitmaps > + extension, this should be considered a= s an > + error. > + > + If it is not set, but there is a Bitma= ps > + extension, its data should be consider= ed as > + inconsistent. > + > + Bits 1-63: Reserved (set to 0) > =20 > 96 - 99: refcount_order > Describes the width of a reference count block ent= ry (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 > =20 > @@ -166,6 +179,34 @@ the header extension data. Each entry look like th= is: > terminated if it has full length) > =20 > =20 > +=3D=3D Bitmaps extension =3D=3D > + > +Bitmaps extension is an optional header extension. It provides an abil= ity 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 ab= ove). > + > +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 bitm= aps per > + image. > + > + 4 - 7: bitmap_directory_size > + Size of the Bitmap Directory in bytes. It must be e= qual 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 Dire= ctory > + starts. Must be aligned to a cluster boundary. > + > + > =3D=3D Host cluster management =3D=3D > =20 > qcow2 manages the allocation of host clusters by maintaining a referen= ce count > @@ -360,3 +401,120 @@ Snapshot table entry: > =20 > variable: Padding to round up the snapshot table entry size = to the > next multiple of 8. > + > + > +=3D=3D Bitmaps =3D=3D > + > +The feature supports storing bitmaps in a qcow2 image. All bitmaps are= related > +to the virtual disk, stored in this image. > + > +=3D=3D=3D Bitmap Directory =3D=3D=3D > + > +Each bitmap saved in the image is described in a Bitmap Directory entr= y. Bitmap > +Directory is a contiguous area in the image file, whose starting offse= t and > +length are given by the header extension fields bitmap_directory_offse= t and > +bitmap_directory_size. The entries of the bitmap directory have variab= le > +length, depending on the length of the bitmap name and extra data. The= se > +entries are also called bitmap headers. > + > +Bitmap Directory Entry: > + > + Byte 0 - 7: bitmap_table_offset > + Offset into the image file at which the Bitmap Tab= le > + (described below) for the bitmap starts. Must be a= ligned to > + a cluster boundary. > + > + 8 - 11: bitmap_table_size > + Number of entries in the Bitmap Table of the bitma= p. > + > + 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 vi= rtual > + disk by any application that would write to t= his qcow2 > + file (including writes, snapshot switching, e= tc.). The > + type of this bitmap must be 'Dirty Tracking B= itmap'. > + > + 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_b= its > + greater than 31. > + > + Granularity is calculated as > + granularity =3D 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 e= xtension. > + > + 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) =3D=3D= 4, its alignment will be 4). Maybe that'll come in handy at some point. 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 siz= e to the > + next multiple of 8. > + > +=3D=3D=3D Bitmap Table =3D=3D=3D > + > +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 cl= usters. > +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 im= age file. > + > +Bitmap Table entry: > + > + Bit 0: Reserved and must be zero if bits 9 - 55 are non-z= ero. > + 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 aligne= d to a > + cluster boundary. If the offset is 0, the cluster = is > + unallocated, see bit 0 description. > + > + 56 - 63: Reserved and must be zero. > + > +=3D=3D=3D Bitmap Data =3D=3D=3D > + > +As noted above, bitmap data is stored in several (or may be one, exact= ly > +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 =3D > + bitmap_table[bitmap_data_offset / cluster_size] + > + (bitmap_data_offset % cluster_size) > + > +Taking into account the granularity of the bitmap, an offset in bits i= nto the > +image file, corresponding to byte number byte_nr of the virtual disk c= an be > +calculated like this: > + > + bit_offset =3D > + image_offset(byte_nr / granularity / 8) * 8 + > + (byte_nr / granularity) % 8 >=20 --6tdEnwUBv5MRrH60EFPlb3SCwSD55nMrc Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJWeJGsAAoJEDuxQgLoOKytvzEH/iU3hsqJnaUvzHw4rmG/FcMc /8nq64TUggxBsrJYB3dBhsx7fKiFUmJ81/c+KWzlbJfgQPzH7lpVXDSllG31PPtp lVcErGSdFUoJc6c4W5G5cAyflYoFlz9m+dloYQ+KhMGS7IxeB/XzerIqI8RYFJZc VhpL5FnXPaJWtJdtlwo0mO/tNyAc90UFeA24IBTMB/wv3pAc/Mg3KrVTtb3uKquj x5jp3WXYjxIbpyq7fYyax9pZ5mS3QyT514x2zMvPDwPU6XLh4pOBeXW2oFHgZcAY t7ZzSFvJD3fGrXxw3uLOBy5wl8Xr+QR7ElhuVkZHVbp6y1N9u7GlBHSiLa7Vvk0= =0hV7 -----END PGP SIGNATURE----- --6tdEnwUBv5MRrH60EFPlb3SCwSD55nMrc--