Re: [Qemu-devel] [PATCH v6 9/9] qapi: query-blockstat: add driver specific file-posix stats

[Markus Armbruster]

Anton Nefedov <address@hidden> writes:

> On 13/12/2018 3:20 PM, Markus Armbruster wrote:
>> I'm reviewing just the QAPI schema today.
>> 
>> Anton Nefedov <address@hidden> writes:
>> 
>>> A block driver can provide a callback to report driver-specific
>>> statistics.
>>>
>>> file-posix driver now reports discard statistics
>>>
>>> Signed-off-by: Anton Nefedov <address@hidden>
[...]
>>> @@ -892,6 +927,8 @@
>>>   #
>>>   # @stats:  A @BlockDeviceStats for the device.
>>>   #
>>> +# @driver-specific: Optional driver-specific stats. (Since 4.0)
>>> +#
>>>   # @parent: This describes the file block device if it has one.
>>>   #          Contains recursively the statistics of the underlying
>>>   #          protocol (e.g. the host file for a qcow2 image). If there is
>>> @@ -905,6 +942,7 @@
>>>   { 'struct': 'BlockStats',
>>>     'data': {'*device': 'str', '*qdev': 'str', '*node-name': 'str',
>>>              'stats': 'BlockDeviceStats',
>>> +           '*driver-specific': 'BlockStatsSpecific',
>>>              '*parent': 'BlockStats',
>>>              '*backing': 'BlockStats'} }
>>>   
>> 
>> Feels awkward.
>> 
>> When is @driver-specific present?  Exactly when the driver is 'file' or
>> 'host_device'?  If that's correct, then turning BlockStats into a union
>> would be clearer and reduce parenthesises on the wire:
>> 
>> { 'union': 'BlockStats',
>>    'base': {
>>        'driver': 'BlockdevDriver',
>>        ... all the other existing members of BlockStats ... }
>>    'discriminator': 'driver',
>>    'data': {
>>        'file': 'BlockStatsSpecificFile',
>>        'host_device': 'BlockStatsSpecificFile' } }
>> 
>> [...]
>> 
>
> this series drags for quite a while - we already discussed this :)
> In short: Blockdev does not always have driver, so it's either this
> or adding weird BlockdevDriver values like "none".
>
> http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg01845.html

You're right.