From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33297) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XjPIB-00048B-SR for qemu-devel@nongnu.org; Wed, 29 Oct 2014 05:11:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XjPI5-0006YU-Jq for qemu-devel@nongnu.org; Wed, 29 Oct 2014 05:11:47 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44295) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XjPI5-0006YH-5R for qemu-devel@nongnu.org; Wed, 29 Oct 2014 05:11:41 -0400 Message-ID: <5450AF47.8010408@redhat.com> Date: Wed, 29 Oct 2014 10:11:35 +0100 From: Max Reitz MIME-Version: 1.0 References: <1414559044-14501-1-git-send-email-famz@redhat.com> <1414559044-14501-5-git-send-email-famz@redhat.com> In-Reply-To: <1414559044-14501-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 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 , Markus Armbruster , Luiz Capitulino , Stefan Hajnoczi On 2014-10-29 at 06:04, 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 | 4 +++- > qmp-commands.hx | 2 +- > 4 files changed, 18 insertions(+), 10 deletions(-) > > diff --git a/block/qapi.c b/block/qapi.c > index a4d1a20..e26033e 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); > } Is there a specific reason why you're not querying the backing chain but still recurse to bs->file? > 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 intialized */ > + query_nodes = query_nodes && has_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 c4beb17..a662137 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -434,7 +434,9 @@ > # > # Since: 0.14.0 > ## > -{ 'command': 'query-blockstats', 'returns': ['BlockStats'] } > +{ 'command': 'query-blockstats', > + 'data': {'*query-nodes': 'bool' }, No space after the opening shwoopy bracket (took that from Wikipedia, I love it), but a space before the closing one. Also, the new parameter should be documented, I think (and the documentation should include that the backing chain is not queried recursively). Max > + '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, > }, >