From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36905) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yl32e-0007eS-Rj for qemu-devel@nongnu.org; Wed, 22 Apr 2015 18:22:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yl32d-00013w-QI for qemu-devel@nongnu.org; Wed, 22 Apr 2015 18:22:48 -0400 Message-ID: <55381F34.7080908@redhat.com> Date: Wed, 22 Apr 2015 18:22:44 -0400 From: John Snow MIME-Version: 1.0 References: <1429314609-29776-1-git-send-email-jsnow@redhat.com> <1429314609-29776-13-git-send-email-jsnow@redhat.com> <55381E1B.1060507@redhat.com> In-Reply-To: <55381E1B.1060507@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v6 12/21] qmp: Add dirty bitmap status field in query-block List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-block@nongnu.org Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com, vsementsov@parallels.com, stefanha@redhat.com, mreitz@redhat.com On 04/22/2015 06:18 PM, Eric Blake wrote: > On 04/17/2015 05:50 PM, John Snow wrote: >> Add the "frozen" status booleans, to inform clients >> when a bitmap is occupied doing a task. >> >> Signed-off-by: Fam Zheng >> Signed-off-by: John Snow >> Reviewed-by: Max Reitz >> Reviewed-by: Stefan Hajnoczi >> Reviewed-by: Eric Blake >> --- >> block.c | 1 + >> qapi/block-core.json | 5 ++++- >> 2 files changed, 5 insertions(+), 1 deletion(-) > >> +++ b/qapi/block-core.json >> @@ -336,10 +336,13 @@ >> # >> # @granularity: granularity of the dirty bitmap in bytes (since 1.4) >> # >> +# @frozen: whether the dirty bitmap is frozen (Since 2.4) >> +# >> # Since: 1.3 >> ## >> { 'type': 'BlockDirtyInfo', >> - 'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32'} } >> + 'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32', >> + 'frozen': 'bool'} } > > Just thinking aloud here - internally, we have a tri-state situation > (enabled, disabled, or frozen). I know we aren't exposing disabled to > the end user yet (no use case for that), but would it be better to make > this output an enum type (two values for now, 'enabled' and 'frozen') to > make it easier to add a third value later, without having to add yet > another boolean? > > But it's not the end of the world to expose a single boolean now (we'd > just have to maintain it forever, even if we add later states), and > adding an enum now just adds complexity that we may not need, so: > > Reviewed-by: Eric Blake > Hmm..., you have a point! If there are no further objections to this series as-is, I will author a quick follow-up patch to implement that -- possibly when we merge in the bitmap migration code. --js