Hanna Czenczek <[email protected]> writes:
> On 20.10.26 08:49, Markus Armbruster wrote:
>> Hanna Czenczek <[email protected]> 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 <[email protected]>
>>> ---
>>> 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].
>
> Sure. So… “block device model”, or “on -device”?
For PATCH 01, I suggested additional text that makes the meaning
obvious:
These are the properties "rerror" and "werror" certain block device
models have.
Here, you have something similar already. I think "on the block device
model" would do. Or maybe "the block device model's ``delay-report-ns``
/ ``delay-report-ns`` properties".
>>> +#
>>> +# 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.
>
> Sure!
>
>>> +# @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?
>
> No, because latency starts when the request begins, and the event (and
> action) happen only when that threshold is crossed.
So it can be triggered once for @delay-report-ns, and once for
@delay-stop-ns. Correct?
In sane usage, @delay-report-ns < @delay-stop-ns. Correct?
> I can try to be more clear.
>
>> The three possible values are ignore, report, and stop. You cover
>> report and stop. Can ignore happen?
>
> No, it cannot. (Also by definition of `ignore` meaning to… ignore something.
> So an event cannot carry this.)
Try mentioning this. If the text feels worse then, don't.
>> We should use ``literal`` markup for literal values.
>
> Sure.
>
> Hanna
>
>>> +#
>>> +# @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' } }
