From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59564) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XlZsw-0005vj-15 for qemu-devel@nongnu.org; Tue, 04 Nov 2014 03:55:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XlZsi-0007kX-2Z for qemu-devel@nongnu.org; Tue, 04 Nov 2014 03:54:41 -0500 Received: from mx1.redhat.com ([209.132.183.28]:34426) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XlZsh-0007ix-IS for qemu-devel@nongnu.org; Tue, 04 Nov 2014 03:54:27 -0500 Message-ID: <5458943E.3050805@redhat.com> Date: Tue, 04 Nov 2014 09:54:22 +0100 From: Max Reitz MIME-Version: 1.0 References: <1414726377-13063-1-git-send-email-famz@redhat.com> <1414726377-13063-5-git-send-email-famz@redhat.com> In-Reply-To: <1414726377-13063-5-git-send-email-famz@redhat.com> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 4/4] qmp: Add optional switch "query-nodes" in query-blockstats List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: Kevin Wolf , Benoit Canet , Stefan Hajnoczi , Markus Armbruster On 2014-10-31 at 04:32, Fam Zheng wrote: > This bool option will allow query all the node names. It iterates all > the BDSes that are assigned a name, also in this case don't query up the > backing chain. > > Signed-off-by: Fam Zheng > --- > block/qapi.c | 20 +++++++++++++------- > hmp.c | 2 +- > qapi/block-core.json | 11 ++++++++++- > qmp-commands.hx | 2 +- > 4 files changed, 25 insertions(+), 10 deletions(-) > > diff --git a/block/qapi.c b/block/qapi.c > index a4d1a20..a0a50b9 100644 > --- a/block/qapi.c > +++ b/block/qapi.c > @@ -322,7 +322,8 @@ static void bdrv_query_info(BlockBackend *blk, BlockInfo **p_info, > qapi_free_BlockInfo(info); > } > > -static BlockStats *bdrv_query_stats(const BlockDriverState *bs) > +static BlockStats *bdrv_query_stats(const BlockDriverState *bs, > + bool query_backing) > { > BlockStats *s; > > @@ -352,12 +353,12 @@ static BlockStats *bdrv_query_stats(const BlockDriverState *bs) > > if (bs->file) { > s->has_parent = true; > - s->parent = bdrv_query_stats(bs->file); > + s->parent = bdrv_query_stats(bs->file, query_backing); > } > > - if (bs->backing_hd) { > + if (query_backing && bs->backing_hd) { > s->has_backing = true; > - s->backing = bdrv_query_stats(bs->backing_hd); > + s->backing = bdrv_query_stats(bs->backing_hd, query_backing); > } > > return s; > @@ -388,17 +389,22 @@ BlockInfoList *qmp_query_block(Error **errp) > return NULL; > } > > -BlockStatsList *qmp_query_blockstats(Error **errp) > +BlockStatsList *qmp_query_blockstats(bool has_query_nodes, > + bool query_nodes, > + Error **errp) > { > BlockStatsList *head = NULL, **p_next = &head; > BlockDriverState *bs = NULL; > > - while ((bs = bdrv_next(bs))) { > + /* Just to be safe if query_nodes is not always initialized */ > + query_nodes = has_query_nodes && query_nodes; > + > + while ((bs = query_nodes ? bdrv_next_node(bs) : bdrv_next(bs))) { > BlockStatsList *info = g_malloc0(sizeof(*info)); > AioContext *ctx = bdrv_get_aio_context(bs); > > aio_context_acquire(ctx); > - info->value = bdrv_query_stats(bs); > + info->value = bdrv_query_stats(bs, !query_nodes); > aio_context_release(ctx); > > *p_next = info; > diff --git a/hmp.c b/hmp.c > index 63d7686..94b27df 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -403,7 +403,7 @@ void hmp_info_blockstats(Monitor *mon, const QDict *qdict) > { > BlockStatsList *stats_list, *stats; > > - stats_list = qmp_query_blockstats(NULL); > + stats_list = qmp_query_blockstats(false, false, NULL); > > for (stats = stats_list; stats; stats = stats->next) { > if (!stats->value->has_device) { > diff --git a/qapi/block-core.json b/qapi/block-core.json > index 6b4b96c..db4e8e9 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -430,11 +430,20 @@ > # > # Query the @BlockStats for all virtual block devices. > # > +# @query-nodes: #optional If true, the command will query all the block nodes > +# that have a node name, in a list which will include "parent" > +# information, but not "backing". > +# If false or omitted, the behavior is as before - query all the > +# device backends, recursively including their "parent" and # Is there a reason for the # at the end of the line? Fixable by a maintainer, though. > +# "backing". (Since 2.3) > +# > # Returns: A list of @BlockStats for each virtual block devices. > # > # Since: 0.14.0 > ## > -{ 'command': 'query-blockstats', 'returns': ['BlockStats'] } > +{ 'command': 'query-blockstats', > + 'data': { '*query-nodes': 'bool' }, > + 'returns': ['BlockStats'] } > > ## > # @BlockdevOnError: > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 1abd619..44bfb9e 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -2347,7 +2347,7 @@ EQMP > > { > .name = "query-blockstats", > - .args_type = "", > + .args_type = "query-nodes:b?", > .mhandler.cmd_new = qmp_marshal_input_query_blockstats, > }, > As written in the reply I forgot to give to v1, I'm still concerned some nodes may appear twice (named protocol BDS) and maybe we don't want that. But it should not hurt. With the # removed: Reviewed-by: Max Reitz