11.02.2019 20:54, Kevin Wolf wrote: > Am 21.12.2018 um 17:53 hat Vladimir Sementsov-Ogievskiy geschrieben: >> Move to way of device selecting, however fall back to device name if >> path is not found. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >> --- >> qapi/block-core.json | 4 ++-- >> blockdev.c | 22 +++++++++++++++------- >> 2 files changed, 17 insertions(+), 9 deletions(-) >> >> diff --git a/qapi/block-core.json b/qapi/block-core.json >> index 762000f31f..bb70c51a57 100644 >> --- a/qapi/block-core.json >> +++ b/qapi/block-core.json >> @@ -489,7 +489,7 @@ >> # If only @device parameter is specified, remove all present latency >> histograms >> # for the device. Otherwise, add/reset some of (or all) latency histograms. >> # >> -# @device: device name to set latency histogram for. >> +# @id: The QOM path or name of the guest device. >> # >> # @boundaries: list of interval boundary values (see description in >> # BlockLatencyHistogramInfo definition). If specified, all >> @@ -547,7 +547,7 @@ >> # <- { "return": {} } >> ## >> { 'command': 'x-block-latency-histogram-set', >> - 'data': {'device': 'str', >> + 'data': {'id': 'str', >> '*boundaries': ['uint64'], >> '*boundaries-read': ['uint64'], >> '*boundaries-write': ['uint64'], >> diff --git a/blockdev.c b/blockdev.c >> index a6f71f9d83..ff0d8ded5e 100644 >> --- a/blockdev.c >> +++ b/blockdev.c >> @@ -4397,21 +4397,29 @@ void qmp_x_blockdev_set_iothread(const char >> *node_name, StrOrNull *iothread, >> } >> >> void qmp_x_block_latency_histogram_set( >> - const char *device, >> + const char *id, >> bool has_boundaries, uint64List *boundaries, >> bool has_boundaries_read, uint64List *boundaries_read, >> bool has_boundaries_write, uint64List *boundaries_write, >> bool has_boundaries_flush, uint64List *boundaries_flush, >> Error **errp) >> { >> - BlockBackend *blk = blk_by_name(device); >> BlockAcctStats *stats; >> int ret; >> + Error *local_err = NULL; >> + BlockBackend *blk = blk_by_qdev_id(id, &local_err); >> >> if (!blk) { >> - error_setg(errp, "Device '%s' not found", device); >> - return; >> + blk = blk_by_name(id); >> + if (!blk) { >> + error_propagate(errp, local_err); >> + return; >> + } else { >> + error_free(local_err); >> + local_err = NULL; >> + } >> } > > Why don't you use qmp_get_blk() here? >
qmp_get_blk is used for cases when we have both @device and @id parameters, where @device is deprecated. And one of previous iteration was a try to do it in this way (have both @id and @device). But it was considered to be bad idea for a new command, which don't need any backward compatibility. And we decided to merge two variants in @id. So, do you suggest to drop blk_by_name() variant at all, and go through qmp_get_blk(NULL, id), which will call blk_by_qdev_id() and fail if not found? -- Best regards, Vladimir