Re: [Qemu-block] [Qemu-devel] [PATCH v3 1/2] qapi: move to QOM path for x-block-latency-histogram-set

[Vladimir Sementsov-Ogievskiy]

08.01.2019 16:20, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <address@hidden> writes:
> 
>> 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
> 
> Is such overloaded semantics what we want in new interfaces?
> 
> Hmm, looks like there's ample precedence for it.  Escaped my grep at
> first because its commonly documented as "The name or QOM path of the
> guest device".  Suggest to stick to that for consistency.


Interesting, that in cases you mean, documentation seems wrong:
it goes through qmp_get_blk, which works like @id may be only QOM path, not 
name,
so for the it should be @id: The QOM path.

And I'm afraid that only my case merges both into one field. And it is more 
correct
to put QOM path in first place, as it is checked first, and if it fails, 
searches by
name. So, I think we should keep it as is, and fix other command documentation 
(but
not in these series, of course)

> 
>> @@ -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'],
> [...]
> 


-- 
Best regards,
Vladimir