We're currently doing a full query-block just to enumerate the devices for qmp_nbd_server_add and then discarding the BlockInfoList afterwards. Alter hmp_nbd_server_start to instead iterate explicitly over the block_backends list.
This allows the removal of the dependency on qmp_query_block from hmp_nbd_server_start. This is desirable because we're about to move qmp_query_block into a coroutine and don't need to change the NBD code at the same time. Add the GRAPH_RDLOCK_GUARD_MAINLOOP macro because bdrv_skip_implicit_filters() needs the graph lock. Signed-off-by: Fabiano Rosas <[email protected]> --- - add a comment explaining some checks are done to preserve previous behavior; - we need the strdup when assigning .device to preserve const. Just add a matching g_free(); - about the possible leak at qmp_nbd_server_add() unrelated to this patch, commit 8675cbd68b ("nbd: Utilize QAPI_CLONE for type conversion") mentions that the QAPI visitor will already free arg->name. --- block/monitor/block-hmp-cmds.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c index d954bec6f1..9db587c661 100644 --- a/block/monitor/block-hmp-cmds.c +++ b/block/monitor/block-hmp-cmds.c @@ -387,10 +387,12 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict) bool writable = qdict_get_try_bool(qdict, "writable", false); bool all = qdict_get_try_bool(qdict, "all", false); Error *local_err = NULL; - BlockInfoList *block_list, *info; + BlockBackend *blk; SocketAddress *addr; NbdServerAddOptions export; + GRAPH_RDLOCK_GUARD_MAINLOOP(); + if (writable && !all) { error_setg(&local_err, "-w only valid together with -a"); goto exit; @@ -415,29 +417,43 @@ 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); + for (blk = blk_all_next(NULL); blk; blk = blk_all_next(blk)) { + BlockDriverState *bs = blk_bs(blk); - for (info = block_list; info; info = info->next) { - if (!info->value->inserted) { + if (!*blk_name(blk)) { + continue; + } + + /* + * Note: historically we used to call qmp_query_block() to get + * the list of block devices. The two 'continue' cases below + * are the same as used by that function and are here to + * preserve behavior. + */ + + if (!blk_get_attached_dev(blk)) { + continue; + } + + bs = bdrv_skip_implicit_filters(bs); + if (!bs || !bs->drv) { continue; } export = (NbdServerAddOptions) { - .device = info->value->device, + .device = g_strdup(blk_name(blk)), .has_writable = true, .writable = writable, }; qmp_nbd_server_add(&export, &local_err); - + g_free(export.device); if (local_err != NULL) { qmp_nbd_server_stop(NULL); break; } } - qapi_free_BlockInfoList(block_list); - exit: hmp_handle_error(mon, local_err); } -- 2.35.3
