From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54769) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aQLej-0004wl-Qw for qemu-devel@nongnu.org; Mon, 01 Feb 2016 16:05:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aQLef-0004Zo-PM for qemu-devel@nongnu.org; Mon, 01 Feb 2016 16:05:05 -0500 Received: from mx1.redhat.com ([209.132.183.28]:34520) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aQLef-0004ZS-ID for qemu-devel@nongnu.org; Mon, 01 Feb 2016 16:05:01 -0500 References: <1453909960-77997-1-git-send-email-vsementsov@virtuozzo.com> From: Max Reitz Message-ID: <56AFC877.90506@redhat.com> Date: Mon, 1 Feb 2016 22:04:55 +0100 MIME-Version: 1.0 In-Reply-To: <1453909960-77997-1-git-send-email-vsementsov@virtuozzo.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="X59kVMpmIGFIdNGeGnN7Q3hS1B2QqUkKm" Subject: Re: [Qemu-devel] [PATCH v8] 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) --X59kVMpmIGFIdNGeGnN7Q3hS1B2QqUkKm Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 27.01.2016 16:52, Vladimir Sementsov-Ogievskiy wrote: > The new feature for qcow2: storing bitmaps. >=20 > 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. >=20 > 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. >=20 > Signed-off-by: Vladimir Sementsov-Ogievskiy > --- > The semantics are good, I just have more grammar nitpicks from here on. := -) [...] >=20 > docs/specs/qcow2.txt | 225 +++++++++++++++++++++++++++++++++++++++++++= +++++++- > 1 file changed, 224 insertions(+), 1 deletion(-) >=20 > diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt > index f236d8c..7b0ebef 100644 > --- a/docs/specs/qcow2.txt > +++ b/docs/specs/qcow2.txt [...] > + 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'. > + > + 2: extra_data_compatible > + This flags is meaningful when extra data is u= nknown for s/for/to/, and probably also "the extra data". > + the software (currently any extra data is unk= nown for s/for/to/ > + Qemu). > + If it is set, the bitmap may be used as expec= ted, extra > + data must be left as is. > + If it is not set, the bitmap must not be used= , but left > + as is with extra data. Maybe s/with/along with its/ would sound better; or "but both it and its extra data be left as is". > + > + Bits 3 - 31 are reserved and must be 0. [...] > +=3D=3D=3D Dirty tracking bitmaps =3D=3D=3D > + > +Bitmaps with 'type' field equal to one are dirty tracking bitmaps. > + > +When the virtual disk is in use dirty tracking bitmap may be 'enabled'= or > +'disabled'. While the bitmap is 'enabled', all writes to the virtual d= isk > +should be reflected in the bitmap. Set bit in the bitmap means that th= e s/Set bit/A set bit/ > +corresponding range of the virtual disk (see above) was written while = the Maybe s/written/written to/, but that's optional ("written" sounds to me like an allocating write, or as if everything in that range was overwritten). > +bitmap was 'enabled'. Unset bit means that this range was not written.= s/Unset bit/An unset bit/, and again maybe s/written/written to/. > + > +The software should not sync the bitmap in the image file with its > +representation in RAM after each write. Flag 'in_use' should be set wh= ile the > +bitmap is not synced. > + > +In the image file the 'enabled' state is reflected by 'auto' flag. If = this flag s/'auto' flag/the 'auto' flag/ > +is set, the software must consider the bitmap as 'enabled' and start t= racking > +virtual disk changes to this bitmap from the first write to the virtua= l disk. If > +this flag is not set then the bitmap is constant. s/constant/'disabled'/? It's basically the same, but "disabled" is what you used above. > + > +To maintain the bitmap consistent, the only software is allowed to cha= nge the > +value of 'auto' flag: the software which was created the bitmap. I'd prefer: To maintain bitmap consistency, the only software which is allowed to change the value of the 'auto' flag is the one which has created the bitm= ap. > The o= ther > +software must not change this flag, it only tracks changes to this bit= map, if > +'auto' flag is set and ignores the bitmap otherwise. I'd drop the second part and shorten it to: Any other software must not change this flag. Or just drop it completely. The previous sentence completely suffices to tell that no other program is allowed to modify it; and I found the second part ("it only tracks...") confusing because I had to wonder about what the "it" referred to, and because it's superfluous. It's just repeating how any program is supposed to handle such a bitmap anyway. Max --X59kVMpmIGFIdNGeGnN7Q3hS1B2QqUkKm 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 iQEcBAEBCAAGBQJWr8h3AAoJEDuxQgLoOKytgr4IAKDg63mQy74q3kQx+RM+QKj4 ELSW7J4pMOv7/tWhC9d8SBFb4vX2uyq0F59tT6jGEE3uZjNB5sYTMQs8aSeat8Wv 3eEQtNkC1Yc2S7WkUorvdmrL/BDlt3D/wESDXj+oQewllOEqIK525O/9TH+cFnUY C8JzBW0wHKMXrcMc5dm5Gzxez/UtHsHqiFMFG1FhGksrxCovQrA9Mr2VmucTCa5l cwA7/t4NbhLn/0QRiBzxG0DmwwxoJiE7mYpzOBHqQ31n+asUCeOqZ3lAKRQHA24u 24qGa+vz/N+aqMOdCCsU7oCKuyQvPtAlbaRfgmOo28jL8e6FURxEFQmHXK6NinQ= =JuKz -----END PGP SIGNATURE----- --X59kVMpmIGFIdNGeGnN7Q3hS1B2QqUkKm--