Re: [Qemu-block] [PATCH v2 8/8] qapi: query-blockstat: add driver specific file-posix stats
On 01/23/2018 05:28 AM, Anton Nefedov wrote:
>>> +
>>> +##
>>> +# @BlockDriverStats:
>>> +#
>>> +# Statistics of a block driver (driver-specific)
>>> +#
>>> +# Since: 2.12
>>> +##
>>> +{ 'union': 'BlockDriverStats',
>>> + 'data': {
>>> + 'file': 'BlockDriverStatsFile'
>>> + } }
>>
>> Markus has been adamant that we add no new "simple unions" (unions with
>> a 'discriminator' field) - because they are anything but simple in the
>> long run.
>
> Right, forgot about those unions.. Will fix.
>
> (I guess I will need an extra enum, like BlockdevDriverWithStats with a
> single 'file' member, otherwise it seems to require to define data for
> each BlockdevDriver type)
Kevin also recently requested an easier way to make a flat union that
uses only a subset of a larger enum. Maybe it's worth investing some
time in a QAPI generator patch to make this part easier (I have a
potential patch floating on one of my older branches that would allow
'branch':{} rather than having to declare a dummy type, but that's
slightly different than not even supplying the branches that you aren't
implementing).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-block] [PATCH v2 8/8] qapi: query-blockstat: add driver specific file-posix stats
On 22/1/2018 11:55 PM, Eric Blake wrote:
On 01/19/2018 06:50 AM, Anton Nefedov wrote:
A block driver can provide a callback to report driver-specific
statistics.
file-posix driver now reports discard statistics
Signed-off-by: Anton Nefedov
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
+++ b/qapi/block-core.json
@@ -774,6 +774,40 @@
'timed_stats': ['BlockDeviceTimedStats'] } }
##
+# @BlockDriverStatsFile:
+#
+# File driver statistics
+#
+# @discard_nb_ok: The number of succeeded discard operations performed by
+# 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 2.12
+##
+{ 'struct': 'BlockDriverStatsFile',
+ 'data': {
+ 'discard_nb_ok': 'int',
+ 'discard_nb_failed': 'int',
+ 'discard_bytes_ok': 'int'
+ } }
New interfaces should prefer '-' over '_', where possible (a reason for
using '_' is if the fields are alongside pre-existing fields that
already used '_'). Let's see how this gets used...[1]
+
+##
+# @BlockDriverStats:
+#
+# Statistics of a block driver (driver-specific)
+#
+# Since: 2.12
+##
+{ 'union': 'BlockDriverStats',
+ 'data': {
+ 'file': 'BlockDriverStatsFile'
+ } }
Markus has been adamant that we add no new "simple unions" (unions with
a 'discriminator' field) - because they are anything but simple in the
long run.
+
+##
# @BlockStats:
#
# Statistics of a virtual block device or a block backing device.
@@ -785,6 +819,8 @@
#
# @stats: A @BlockDeviceStats for the device.
#
+# @driver-stats: Optional driver-specific statistics. (Since 2.12)
+#
# @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
@@ -798,6 +834,7 @@
{ 'struct': 'BlockStats',
'data': {'*device': 'str', '*node-name': 'str',
'stats': 'BlockDeviceStats',
+ '*driver-stats': 'BlockDriverStats',
...[1] You are using it alongside a struct that already uses '-'
(node-name), so you should use dashes.
So, the difference between your proposal (a simple union) and using a
"flat union", on the wire, is yours:
"return": { ..., "driver-stats": { "type": "file", "data": {
"discard_nb_ok: ... } } }
vs. a flat union:
"return": { ..., "driver-stats": { "driver": "file", "discard-nb-ok":
... } }
where you can benefit from less nesting and a saner discriminator name.
Right, forgot about those unions.. Will fix.
(I guess I will need an extra enum, like BlockdevDriverWithStats with a
single 'file' member, otherwise it seems to require to define data for
each BlockdevDriver type)
/Anton
Re: [Qemu-block] [PATCH v2 8/8] qapi: query-blockstat: add driver specific file-posix stats
On 01/19/2018 06:50 AM, Anton Nefedov wrote:
> A block driver can provide a callback to report driver-specific
> statistics.
>
> file-posix driver now reports discard statistics
>
> Signed-off-by: Anton Nefedov
> Reviewed-by: Vladimir Sementsov-Ogievskiy
> ---
> +++ b/qapi/block-core.json
> @@ -774,6 +774,40 @@
> 'timed_stats': ['BlockDeviceTimedStats'] } }
>
> ##
> +# @BlockDriverStatsFile:
> +#
> +# File driver statistics
> +#
> +# @discard_nb_ok: The number of succeeded discard operations performed by
> +# 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 2.12
> +##
> +{ 'struct': 'BlockDriverStatsFile',
> + 'data': {
> + 'discard_nb_ok': 'int',
> + 'discard_nb_failed': 'int',
> + 'discard_bytes_ok': 'int'
> + } }
New interfaces should prefer '-' over '_', where possible (a reason for
using '_' is if the fields are alongside pre-existing fields that
already used '_'). Let's see how this gets used...[1]
> +
> +##
> +# @BlockDriverStats:
> +#
> +# Statistics of a block driver (driver-specific)
> +#
> +# Since: 2.12
> +##
> +{ 'union': 'BlockDriverStats',
> + 'data': {
> + 'file': 'BlockDriverStatsFile'
> + } }
Markus has been adamant that we add no new "simple unions" (unions with
a 'discriminator' field) - because they are anything but simple in the
long run.
> +
> +##
> # @BlockStats:
> #
> # Statistics of a virtual block device or a block backing device.
> @@ -785,6 +819,8 @@
> #
> # @stats: A @BlockDeviceStats for the device.
> #
> +# @driver-stats: Optional driver-specific statistics. (Since 2.12)
> +#
> # @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
> @@ -798,6 +834,7 @@
> { 'struct': 'BlockStats',
>'data': {'*device': 'str', '*node-name': 'str',
> 'stats': 'BlockDeviceStats',
> + '*driver-stats': 'BlockDriverStats',
...[1] You are using it alongside a struct that already uses '-'
(node-name), so you should use dashes.
So, the difference between your proposal (a simple union) and using a
"flat union", on the wire, is yours:
"return": { ..., "driver-stats": { "type": "file", "data": {
"discard_nb_ok: ... } } }
vs. a flat union:
"return": { ..., "driver-stats": { "driver": "file", "discard-nb-ok":
... } }
where you can benefit from less nesting and a saner discriminator name.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
