[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v6 9/9] qapi: query-blockstat: add
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v6 9/9] qapi: query-blockstat: add driver specific file-posix stats |
Date: |
Thu, 21 Feb 2019 13:36:56 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
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>
>>> ---
>>> qapi/block-core.json | 38 ++++++++++++++++++++++++++++++++++++++
>>> include/block/block.h | 1 +
>>> include/block/block_int.h | 1 +
>>> block.c | 9 +++++++++
>>> block/file-posix.c | 32 ++++++++++++++++++++++++++++++++
>>> block/qapi.c | 5 +++++
>>> 6 files changed, 86 insertions(+)
>>>
>>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>>> index 959358ccc4..b100e852c7 100644
>>> --- a/qapi/block-core.json
>>> +++ b/qapi/block-core.json
>>> @@ -877,6 +877,41 @@
>>> '*x_wr_latency_histogram': 'BlockLatencyHistogramInfo',
>>> '*x_flush_latency_histogram': 'BlockLatencyHistogramInfo' } }
>>>
>>> +##
>>> +# @BlockStatsSpecificFile:
>>> +#
>>> +# File driver statistics
>>> +#
>>> +# @discard-nb-ok: The number of succeeded discard operations performed by
>>
>> successful discard operations
>>
>
> Fixed.
>
>>> +# the driver.
>>> +#
>>> +# @discard-nb-failed: The number of failed discard operations performed by
>>> +# the driver.
>>> +#
>>> +# @discard-bytes-ok: The number of bytes discarded by the driver.
>>> +#
>>> +# Since: 4.0
>>> +##
>>> +{ 'struct': 'BlockStatsSpecificFile',
>>> + 'data': {
>>> + 'discard-nb-ok': 'int',
>>> + 'discard-nb-failed': 'int',
>>> + 'discard-bytes-ok': 'int' } }
>>
>> Should these be unsigned?
>>
>> For what it's worth, similar counters nearby are also 'int'.
>>
>
> And I just added these symmetrically.
> Probably shouldn't have - let these be uint64.
>
>>> +
>>> +##
>>> +# @BlockStatsSpecific:
>>> +#
>>> +# Block driver specific statistics
>>> +#
>>> +# Since: 4.0
>>> +##
>>> +{ 'union': 'BlockStatsSpecific',
>>> + 'base': { 'driver': 'BlockdevDriver' },
>>> + 'discriminator': 'driver',
>>> + 'data': {
>>> + 'file': 'BlockStatsSpecificFile',
>>> + 'host_device': 'BlockStatsSpecificFile' } }
>>> +
>>> ##
>>> # @BlockStats:
>>> #
>>> @@ -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
With the comment tweak:
Acked-by: Markus Armbruster <address@hidden>
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-block] [Qemu-devel] [PATCH v6 9/9] qapi: query-blockstat: add driver specific file-posix stats,
Markus Armbruster <=