From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38814) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bygvK-0000Lu-8L for qemu-devel@nongnu.org; Mon, 24 Oct 2016 11:12:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bygvJ-0004Sg-7G for qemu-devel@nongnu.org; Mon, 24 Oct 2016 11:12:26 -0400 Received: from mailhub.sw.ru ([195.214.232.25]:13191 helo=relay.sw.ru) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bygvI-0004Pm-Se for qemu-devel@nongnu.org; Mon, 24 Oct 2016 11:12:25 -0400 References: <1475232808-4852-1-git-send-email-vsementsov@virtuozzo.com> <1475232808-4852-17-git-send-email-vsementsov@virtuozzo.com> From: Vladimir Sementsov-Ogievskiy Message-ID: Date: Mon, 24 Oct 2016 18:12:11 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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: Max Reitz , 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 10.10.2016 19:08, Max Reitz wrote: > On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote: > [...] >> + } >> >> 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 for >> # 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 mean, > 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. Hmm.. But 'its close' is block device close, not file close. And we call common bdrv_* function to save it, so we actually save it to device, and then the device puzzles out, how to actually save it. > >> +# >> # 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 = "block-dirty-bitmap-add", >> - .args_type = "node:B,name:s,granularity:i?", >> + .args_type = "node:B,name:s,granularity:i?,persistent:b?", >> .mhandler.cmd_new = 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. All > 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 sentence > here. Users shoud know, that only qcow2 supports persistent bitmaps. Instead of last sentence I can write "For now only Qcow2 disks supports persistent bitmaps". -- Best regards, Vladimir