| Message ID | 1429314609-29776-13-git-send-email-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 <famz@redhat.com> > Signed-off-by: John Snow <jsnow@redhat.com> > Reviewed-by: Max Reitz <mreitz@redhat.com> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > --- > 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 <eblake@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 <famz@redhat.com> >> Signed-off-by: John Snow <jsnow@redhat.com> >> Reviewed-by: Max Reitz <mreitz@redhat.com> >> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> >> Reviewed-by: Eric Blake <eblake@redhat.com> >> --- >> 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 <eblake@redhat.com> > 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
diff --git a/block.c b/block.c index 679991b..ca73a6a 100644 --- a/block.c +++ b/block.c @@ -5633,6 +5633,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs) info->granularity = bdrv_dirty_bitmap_granularity(bm); info->has_name = !!bm->name; info->name = g_strdup(bm->name); + info->frozen = bdrv_dirty_bitmap_frozen(bm); entry->value = info; *plist = entry; plist = &entry->next; diff --git a/qapi/block-core.json b/qapi/block-core.json index fc9ca04..cae995b 100644 --- a/qapi/block-core.json +++ 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'} } ## # @BlockInfo: