Re: [PATCH v2 06/12] qapi/block: Add BLOCK_IO_DELAY event

[Markus Armbruster <armbru@redhat.com>]

Hanna Czenczek <hreitz@redhat.com> writes:

> This event will be emitted when a disk operation takes longer than a
> pre-defined threshold (as set on the block device).
>
> Note that the description refers to the "delay-report-ns" and
> "delay-stop-ns" block device properties, which are not yet available.
> A future commit in this series will add them.
>
> Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
> ---
>  qapi/block.json | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>
> diff --git a/qapi/block.json b/qapi/block.json
> index 8f4a71733c1..5f9f5c28050 100644
> --- a/qapi/block.json
> +++ b/qapi/block.json
> @@ -649,3 +649,40 @@
>  ##
>  { 'enum': 'IoAccountingOperation',
>    'data': [ 'read', 'write', 'flush', 'zone-append', 'unmap' ] }
> +
> +##
> +# @BLOCK_IO_DELAY:
> +#
> +# Emitted when a disk operation takes longer than the pre-defined
> +# threshold (as set via the "delay-report-ns" / "delay-stop-ns"
> +# properties on the block device).

As in PATCH 01, "block device" could be confusing, because we use the
term both for frontends and backends.  I believe "delay-report-ns" and
"delay-stop-ns" are the qdev properties every block device model will
have [PATCH 11].

> +#
> +# Note that delays deliberately incurred by throttling do count
> +# towards the operation duration.
> +#

Please use the revised syntax for introductions, like you did in PATCH
01:

   # @name: introduction, best to keep it brief, but you may continue it
   #     like this if necessary.  If your text need to refer to
   #     arguments, it's almost certainly not brief introduction
   #     anymore.
   #
   # Additional text can go here for now.

I apologize for the inconvenience.  We need this to eliminate certain
syntactic ambiguities that plagued the doc generator.  We'll need it
some more for the doc inliner.

> +# @qom-path: path to the block device object in the QOM tree
> +#
> +# @operation: I/O operation
> +#
> +# @latency-ns: Ongoing duration of the operation (in nanoseconds)
> +#
> +# @action: Action that has been taken ("report" for "delay-report-ns"
> +#     or "stop" for "delay-stop-ns").  A "stop" action means the VM
> +#     will be stopped with the "io-hang" `RunState`, which does not
> +#     drain the block devices attached to the VM, because draining
> +#     would be counterproductive in case of a hanging I/O request.

This feels less than clear.  What exactly happens?  My best guess:

* When an operation takes longer than delay-report-ns, BLOCK_IO_DELAY is
  emitted with "action": "report" (but only once for this operation).

* When an operation takes longer than delay-stop-ns, the guest is
  stopped (run state becomes io-hang), and BLOCK_IO_DELAY is emitted
  with "action": "stop".

  Say we continue the guest.  Can the even be triggered again for this
  operation?

The three possible values are ignore, report, and stop.  You cover
report and stop.  Can ignore happen?

We should use ``literal`` markup for literal values.

> +#
> +# @offset: Request offset in the guest disk (in bytes), if available
> +#
> +# @bytes: Request length
> +#
> +# Since: 11.2
> +##
> +{ 'event': 'BLOCK_IO_DELAY',
> +  'data': {
> +      'qom-path': 'str',
> +      'operation': 'IoAccountingOperation',
> +      'latency-ns': 'uint64',
> +      'action': 'BlockErrorAction',
> +      '*offset': 'uint64',
> +      'bytes': 'uint64' } }