Kevin Wolf <kw...@redhat.com> writes: > This allows querying one specific BlockDriverState instead of a list of > all drives.
I've long resisted optional arguments to limit list-valued queries, because I believe the additional interface complexity isn't worth the additional utility. But resistance might be futile. We have one only query- with an optional limiting arguement[*]: query-rx-filter. If we go down this route, then I'd prefer to have such optional limiting for all query- commands returning lists of elements describing some aspect of a named object. Preference != demand. > > Signed-off-by: Kevin Wolf <kw...@redhat.com> > --- > block/qapi.c | 15 +++++++++++++-- > hmp.c | 6 +++--- > qapi/block-core.json | 8 ++++++-- > 3 files changed, 22 insertions(+), 7 deletions(-) > > diff --git a/block/qapi.c b/block/qapi.c > index 6b43bc3..2a060d3 100644 > --- a/block/qapi.c > +++ b/block/qapi.c > @@ -367,13 +367,24 @@ static BlockStats *bdrv_query_stats(const > BlockDriverState *bs) > return s; > } > > -BlockInfoList *qmp_query_block(Error **errp) > +BlockInfoList *qmp_query_block(bool has_device, const char *device, > + Error **errp) > { > BlockInfoList *head = NULL, **p_next = &head; > BlockDriverState *bs = NULL; > Error *local_err = NULL; > > - while ((bs = bdrv_next(bs))) { > + if (has_device) { > + bs = bdrv_find(device); > + if (bs == NULL) { > + error_setg(errp, "Device not found"); > + goto err; > + } > + } else { > + bs = bdrv_next(NULL); > + } > + > + for (; bs; (bs = has_device ? NULL : bdrv_next(bs))) { > BlockInfoList *info = g_malloc0(sizeof(*info)); > bdrv_query_info(bs, &info->value, &local_err); > if (local_err) { I'd find for (bs = bdrv_next(NULL); bs; bs == bdrv_next(bs)) { if (device && strcmp(device, bs->device)) { continue; } } simpler and clearer. It's how qmp_query_rx_filter() works. > diff --git a/hmp.c b/hmp.c > index c3846b8..02eee80 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -298,7 +298,7 @@ void hmp_info_block(Monitor *mon, const QDict *qdict) > const char *device = qdict_get_try_str(qdict, "device"); > bool verbose = qdict_get_try_bool(qdict, "verbose", 0); > > - block_list = qmp_query_block(NULL); > + block_list = qmp_query_block(false, NULL, NULL); > > for (info = block_list; info; info = info->next) { > if (device && strcmp(device, info->value->device)) { > @@ -847,7 +847,7 @@ void hmp_cont(Monitor *mon, const QDict *qdict) > BlockInfoList *bdev_list, *bdev; > Error *err = NULL; > > - bdev_list = qmp_query_block(NULL); > + bdev_list = qmp_query_block(false, NULL, NULL); > for (bdev = bdev_list; bdev; bdev = bdev->next) { > if (key_is_missing(bdev->value)) { > monitor_read_block_device_key(mon, bdev->value->device, > @@ -1588,7 +1588,7 @@ void hmp_nbd_server_start(Monitor *mon, const QDict > *qdict) > /* Then try adding all block devices. If one fails, close all and > * exit. > */ > - block_list = qmp_query_block(NULL); > + block_list = qmp_query_block(false, NULL, NULL); > > for (info = block_list; info; info = info->next) { > if (!info->value->has_inserted) { > diff --git a/qapi/block-core.json b/qapi/block-core.json > index c53b587..ddc3fe0 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -354,13 +354,17 @@ > ## > # @query-block: > # > -# Get a list of BlockInfo for all virtual block devices. > +# Get a list of BlockInfo for all or one specific virtual block device. > +# > +# @device: #optional The id of the block device to inspect. (Since 2.2) > # > # Returns: a list of @BlockInfo describing each virtual block device > # > # Since: 0.14.0 > ## This is how we document query-rx-filter: ## # @query-rx-filter: # # Return rx-filter information for all NICs (or for the given NIC). # # @name: #optional net client name # # Returns: list of @RxFilterInfo for all NICs (or for the given NIC). # Returns an error if the given @name doesn't exist, or given # NIC doesn't support rx-filter querying, or given net client # isn't a NIC. # # Since: 1.6 ## I find that slightly clearer. Perhaps you'd like to steal from it. > -{ 'command': 'query-block', 'returns': ['BlockInfo'] } > +{ 'command': 'query-block', > + 'data': { '*device': 'str' }, > + 'returns': ['BlockInfo'] } > > ## > # @BlockDeviceStats: [*] Yes, I objected to the argument, but I didn't insist. I won't insist now.