Re: [PATCH v2 07/12] qapi/block: Add BLOCK_IO_DONE event

[Markus Armbruster <armbru@redhat.com>]

Hanna Czenczek <hreitz@redhat.com> writes:

> On 23.10.26 08:18, Markus Armbruster wrote:
>> Hanna Czenczek <hreitz@redhat.com> writes:
>>
>>> On 20.10.26 08:55, Markus Armbruster wrote:
>>>> Hanna Czenczek <hreitz@redhat.com> writes:
>>>>
>>>>> This event is emitted when an operation completes, for which a
>>>>> BLOCK_IO_DELAY event has previously been emitted.
>>>>>
>>>>> Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
>>>>> ---
>>>>>    qapi/block.json | 33 +++++++++++++++++++++++++++++++++
>>>>>    1 file changed, 33 insertions(+)
>>>>>
>>>>> diff --git a/qapi/block.json b/qapi/block.json
>>>>> index 5f9f5c28050..38f293dd32d 100644
>>>>> --- a/qapi/block.json
>>>>> +++ b/qapi/block.json

[...]

>>>>> @@ -686,3 +689,33 @@
>>>>>        'action': 'BlockErrorAction',
>>>>>        '*offset': 'uint64',
>>>>>        'bytes': 'uint64' } }
>>>>> +
>>>>> +##
>>>>> +# @BLOCK_IO_DONE:
>>>>> +#
>>>>> +# Emitted on completion for disk operations that previously emitted a
>>>>> +# `BLOCK_IO_DELAY` event.
>>>>
>>>> Can we have multiple operations in flight that previously emitted
>>>> BLOCK_IO_DELAY?
>>>>
>>>> If yes, might a management application need to connect the BLOCK_IO_DONE
>>>> back to the BLOCK_IO_DELAY?
>>>>
>>>> If yes, how could it do that?
>>>
>>> Well, the combination of device, operation, offset, bytes.
>>>
>>> Do you think there should be an ID like a u64 counter per
>>> BlockBackend?
>>
>> I'm not overly particular on how exactly the connection can be made.  If
>> the quadruple you suggested always suffices, that's good enough for me.
>> Documentation should point it out, though.
>>
>> Can multiple operations with the same (device, operation, offset, bytes)
>> be in flight?
>
> Technically, sure.
>
> Adding an ID is quite ugly, I feel like, which is why I would need your input in how it would be least intrusive in QAPI.

First step is to get a clearer idea on what exactly management
applications need here.

> (Caveat, at this point: I’m afraid I’ll be quite far from my notebook for around four weeks… Thank you for your help so far :))

No problem!

[...]

>>>>> +#
>>>>> +# @qom-path: path to the block device object in the QOM tree
>>>>> +#
>>>>> +# @operation: I/O operation
>>>>> +#
>>>>> +# @latency-ns: Full duration of the operation (in nanoseconds)
>>>>> +#
>>>>> +# @offset: Request offset in the guest disk (in bytes), if available
>>>>
>>>> Out of curiosity: when could @offset be unavailable?
>>>
>>> For flushes.
>>
>> Uh, what's @bytes then?
>
> 0. I found that natural. Any offset value is a valid offset, but a value of 0 for bytes sounded good to me.
>
> I now anticipate discomfort with this, so I can make @bytes optional, too, yes.

How much a flush writes and where is not known here.  We can use 0 for
"unknown" as long as we document it.  "Absent" feels like the more
obvious choice to me, though.  Advice, not a demand.

Same logic arguagbly applies to @offset, too.

Regardless, the special case "flush" needs to be documented.

[...]