[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v3 1/2] qapi: move to QOM path for x-block-laten
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [Qemu-block] [PATCH v3 1/2] qapi: move to QOM path for x-block-latency-histogram-set |
Date: |
Mon, 11 Feb 2019 18:30:26 +0000 |
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 <address@hidden>
>> ---
>> 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