From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37386) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1byj4j-0002Qo-5u for qemu-devel@nongnu.org; Mon, 24 Oct 2016 13:30:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1byj4h-0001EX-Uv for qemu-devel@nongnu.org; Mon, 24 Oct 2016 13:30:17 -0400 References: <1475232808-4852-1-git-send-email-vsementsov@virtuozzo.com> <1475232808-4852-17-git-send-email-vsementsov@virtuozzo.com> From: Max Reitz Message-ID: Date: Mon, 24 Oct 2016 19:30:04 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="5XsDLC1hRjAWqvsG88wpfST5Hkx0fST4I" Subject: Re: [Qemu-devel] [PATCH 16/22] qmp: add persistent flag to block-dirty-bitmap-add List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladimir Sementsov-Ogievskiy , qemu-block@nongnu.org, qemu-devel@nongnu.org Cc: kwolf@redhat.com, armbru@redhat.com, eblake@redhat.com, jsnow@redhat.com, famz@redhat.com, den@openvz.org, stefanha@redhat.com, pbonzini@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --5XsDLC1hRjAWqvsG88wpfST5Hkx0fST4I From: Max Reitz To: Vladimir Sementsov-Ogievskiy , qemu-block@nongnu.org, qemu-devel@nongnu.org Cc: kwolf@redhat.com, armbru@redhat.com, eblake@redhat.com, jsnow@redhat.com, famz@redhat.com, den@openvz.org, stefanha@redhat.com, pbonzini@redhat.com Message-ID: Subject: Re: [PATCH 16/22] qmp: add persistent flag to block-dirty-bitmap-add References: <1475232808-4852-1-git-send-email-vsementsov@virtuozzo.com> <1475232808-4852-17-git-send-email-vsementsov@virtuozzo.com> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 24.10.2016 17:12, Vladimir Sementsov-Ogievskiy wrote: > 10.10.2016 19:08, Max Reitz wrote: >> On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote: >> >=20 > [...] >=20 >>> + } >>> out: >>> aio_context_release(aio_context); >>> diff --git a/qapi/block-core.json b/qapi/block-core.json >>> index 31f9990..2bf56cd 100644 >>> --- a/qapi/block-core.json >>> +++ b/qapi/block-core.json >>> @@ -1235,10 +1235,15 @@ >>> # @granularity: #optional the bitmap granularity, default is 64k fo= r >>> # block-dirty-bitmap-add >>> # >>> +# @persistent: #optional the bitmap is persistent, i.e. it will be >>> saved to >>> +# corresponding block device on it's close. Default is >>> false. >>> +# For block-dirty-bitmap-add. (Since 2.8) >> I'm not sure what the "For block-dirty-bitmap-add." is supposed to mea= n, >> because this whole struct is for block-dirty-bitmap-add (and for >> block-dirty-bitmap-add transactions, to be exact, but @persistent will= >> surely work there, too, won't it?). >> >> Also, I'd say "will be saved to the corresponding block device image >> file" instead of just "block device", because in my understanding a >> block device and its image file are two separate things. >=20 > Hmm.. But 'its close' is block device close, not file close. In my understanding, it is the file close. > And we cal= l > common bdrv_* function to save it, so we actually save it to device, an= d > then the device puzzles out, how to actually save it. Well, OK, it depends on what you mean by "block device". There are many things we call a "block device", but nowadays I think it mostly refers to either a guest block device or a BlockBackend (and as of lately, we're more and more trying to hide the BlockBackend from the user, so you could argue that it's only the guest device now). The bdrv_* functions operate on block layer BDS nodes, and I don't think we call them "block devices" (at least not any more). In any case, I think for users the term "block device" refers to either the device presented to the guest or to all of the block layer stuff that's underneath, and it's not quite clear how you could save a bitmap to that, or what it's supposed to mean to "close" a block device (you can remove it, you can destroy it, you can delete it, but "close" it?). But saying that it will be saved to the image file that is attached to the block device will make it absolutely clear what we mean. So what you have called a "device" here is neither what I'd call a device (I'd call it a "node" or "BDS"), nor what I think users would call a device. Also, it's not the BDS that puzzles out how to save it but some block driver. >>> +# >>> # Since 2.4 >>> ## >>> { 'struct': 'BlockDirtyBitmapAdd', >>> - 'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32' }= } >>> + 'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32', >>> + '*persistent': 'bool' } } >> I think normally we'd align the new line so that the opening ' of >> '*persistent' is under the opening ' of 'node'. >> >>> ## >>> # @block-dirty-bitmap-add >>> diff --git a/qmp-commands.hx b/qmp-commands.hx >>> index ba2a916..434b418 100644 >>> --- a/qmp-commands.hx >>> +++ b/qmp-commands.hx >>> @@ -1441,7 +1441,7 @@ EQMP >>> { >>> .name =3D "block-dirty-bitmap-add", >>> - .args_type =3D "node:B,name:s,granularity:i?", >>> + .args_type =3D "node:B,name:s,granularity:i?,persistent:b?"= , >>> .mhandler.cmd_new =3D qmp_marshal_block_dirty_bitmap_add, >>> }, >>> @@ -1458,6 +1458,9 @@ Arguments: >>> - "node": device/node on which to create dirty bitmap (json-string)= >>> - "name": name of the new dirty bitmap (json-string) >>> - "granularity": granularity to track writes with (int, optional) >>> +- "persistent": bitmap will be saved to corresponding block device >>> + on it's close. Block driver should maintain >>> persistent bitmaps >>> + (json-bool, optional, default false) (Since 2.8) >> And I don't know what the user is supposed to make of the information >> that block drivers will take care of maintaining persistent bitmaps. A= ll >> they care about is that it will be stored in the corresponding image >> file, so in my opinion it would be better to just omit the last senten= ce >> here. >=20 > Users shoud know, that only qcow2 supports persistent bitmaps. Instead > of last sentence I can write "For now only Qcow2 disks supports > persistent bitmaps". s/supports/support/, but yes, that sounds preferable to me. Max --5XsDLC1hRjAWqvsG88wpfST5Hkx0fST4I Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQEvBAEBCAAZBQJYDkUdEhxtcmVpdHpAcmVkaGF0LmNvbQAKCRD0B9sAYdXPQFAz B/9l7lNnNXE9zlHUiRJ/jfa/epGmUDiCbrAdu9uJ79Z05K9kuH+lh5xei3bg1c64 1I0ZV4JBj2eDCiuL2p77vSODfBUCpFCD4dOEaa6bUAvpnq0mcFSR3MkX4R3a5JFX ZYd7i2/OlRvNP0nRqTEUOLwfxwh+1NhMX4b8npuW8GG8BwN2nQS2ifylhugmo6Lc p2Q2k+jcLc4nod82xOdDgmBj3YyuIb1xWyFCXN8O958yLVh4cQZN2Nq2AT71vPmm zqRZR10ChJaFRNTCowje/WW5D/sliz3QK1+EGH0/ocMXbiuuC7wsGYsnu5oIYHv9 q3RaJ95bkYGf14fxsfZa18Xr =sXqV -----END PGP SIGNATURE----- --5XsDLC1hRjAWqvsG88wpfST5Hkx0fST4I--