Based-on: <20260724152315.234183-1-hreitz@redhat.com>
          [PATCH 0/6] hw/[block]: Fix missing accounting
          Fri, 24 Jul 2026 17:23:09 +0200

Hi,

I’m told there are installations where storage is very slow, and some
would like the VM stack to report this proactively.  To do so, we should
(via QAPI events) report on extremely slow I/O requests.

We already have the latency histogram, but this is not deemed sufficient
because it is not proactively reporting and would require repeated
querying.  Therefore, this series introduces the event still.

Now, from a user's perspective, it would be nice if this event could be
raised exactly when an I/O request crosses the user-defined threshold,
but this would require keeping all active requests in a list and
checking it periodically.  Now, if we used latency cookies for this
(which makes sense), then that would require that every cookie set up is
also finalized when the request is done, because if we don't, results
could well be catastrophic:
- Either we use cookies as-is, which are often allocated on the stack or
  in some other structure managed by the device; then this would result
  in use-after-free,
- Or we allocate something specifically for this checking, so lingering
  requests would at most create spurious latency events and memory
  leaks, but this would require an additional heap allocation per
  request that we would probably want to avoid.

So ideally we could use latency cookies and could statically verify that
they are always finalized when the request is done, but doing this in C
may well be impossible.


So, because it is basically impossible (or at least it would be very
hard, and presumably require a large refactoring) to guarantee, without
additional heap allocations, that a list of active requests won’t run
into catastrophic use-after-frees, this series does the much simpler
version first, which is to just raise an event when a request *finishes*
and took more than a user-defined latency threshold.


(PS: The nice thing about throwing an alert while the request is still
going on would be that it could allow us to also stop the VM in case of
excessive latency, before the request completes, so the guest would be
shielded from such excessive latency.  This might be useful for Windows
guests that just have a maximum request lantency before throwing a
BSOD.)


Hanna Czenczek (9):
  block/accounting: Add offset to BlockAcctCookie
  qapi/block: Add IoAccountingOperation enum
  qapi/block: Add BLOCK_IO_DELAY event
  block-backend: Public blk_get_attached_dev_path()
  block/accounting: Add BB field to latency checker
  block/accounting: Emit BLOCK_IO_DELAY event
  block: Add delay-alert-ms property
  block/accounting: Move latency_ns override down
  iotests: Add delay-alert test

 qapi/block.json                          |  52 +++++++++
 include/block/accounting.h               |  20 +++-
 include/hw/block/block.h                 |   5 +-
 include/system/block-backend-io.h        |   9 ++
 include/system/dma.h                     |   2 +-
 block/accounting.c                       |  60 ++++++++--
 block/block-backend.c                    |   8 +-
 blockdev.c                               |  16 ++-
 hw/block/block.c                         |   4 +-
 hw/block/dataplane/xen-block.c           |   4 +-
 hw/block/virtio-blk.c                    |  15 +--
 hw/ide/ahci.c                            |   6 +-
 hw/ide/atapi.c                           |   9 +-
 hw/ide/core.c                            |   9 +-
 hw/ide/macio.c                           |  15 ++-
 hw/nvme/ctrl.c                           |  43 ++++---
 hw/nvme/dif.c                            |   8 +-
 hw/scsi/scsi-disk.c                      |  28 +++--
 qemu-io-cmds.c                           |  14 +--
 system/dma-helpers.c                     |   4 +-
 tests/unit/test-block-accounting.c       |   2 +-
 tests/qemu-iotests/172.out               |  38 +++++++
 tests/qemu-iotests/tests/delay-alert     | 136 +++++++++++++++++++++++
 tests/qemu-iotests/tests/delay-alert.out |  46 ++++++++
 24 files changed, 472 insertions(+), 81 deletions(-)
 create mode 100755 tests/qemu-iotests/tests/delay-alert
 create mode 100644 tests/qemu-iotests/tests/delay-alert.out

-- 
2.55.0

On Mon, Aug 31, 2026 at 03:51:56PM +0200, Hanna Czenczek wrote:
> Based-on: <20260724152315.234183-1-hreitz@redhat.com>
>           [PATCH 0/6] hw/[block]: Fix missing accounting
>           Fri, 24 Jul 2026 17:23:09 +0200
> 
> Hi,
> 
> I’m told there are installations where storage is very slow, and some
> would like the VM stack to report this proactively.  To do so, we should
> (via QAPI events) report on extremely slow I/O requests.
> 
> We already have the latency histogram, but this is not deemed sufficient
> because it is not proactively reporting and would require repeated
> querying.  Therefore, this series introduces the event still.
> 
> Now, from a user's perspective, it would be nice if this event could be
> raised exactly when an I/O request crosses the user-defined threshold,
> but this would require keeping all active requests in a list and
> checking it periodically.  Now, if we used latency cookies for this

Did you consider a per-request timer? The QEMUTimerList active_timers
sorted list does not support efficient insertion, but improving it would
benefit all timer API users.

> (which makes sense), then that would require that every cookie set up is
> also finalized when the request is done, because if we don't, results
> could well be catastrophic:
> - Either we use cookies as-is, which are often allocated on the stack or
>   in some other structure managed by the device; then this would result
>   in use-after-free,
> - Or we allocate something specifically for this checking, so lingering
>   requests would at most create spurious latency events and memory
>   leaks, but this would require an additional heap allocation per
>   request that we would probably want to avoid.
> 
> So ideally we could use latency cookies and could statically verify that
> they are always finalized when the request is done, but doing this in C
> may well be impossible.

I think you are saying that the cookie API is unsafe because cookie
lifetime is not bounded by the request lifetime?

Maybe the block_acct_*() API can be integrated into the actual request
so there is no way to leak the cookie. In other words, directly
associate requests with a BlockAcctStats and stop requiring the user to
manually manage a separate BlockAcctCookie.

The API is already weird because devices use:

  block_acct_failed(blk_get_stats(s->blk), &req->acct);

i.e. why does the device have to reach into s->blk to access the stats?
If the stats belong to s->blk, then s->blk should do the accounting
during the request lifetime.

This would require a redesign of not just the cookie API, but also the
error policy API. There is also a wrinkle in that virtio-blk merges I/O
requests and accounts the merges.

> 
> 
> So, because it is basically impossible (or at least it would be very
> hard, and presumably require a large refactoring) to guarantee, without
> additional heap allocations, that a list of active requests won’t run
> into catastrophic use-after-frees, this series does the much simpler
> version first, which is to just raise an event when a request *finishes*
> and took more than a user-defined latency threshold.

Does this achieve the goal of warning when requests exceed a threshold?
When an I/O request hangs for a long time, the management tool will be
unable to detect that the threshold has been exceeded in a timely
manner.

> 
> 
> (PS: The nice thing about throwing an alert while the request is still
> going on would be that it could allow us to also stop the VM in case of
> excessive latency, before the request completes, so the guest would be
> shielded from such excessive latency.  This might be useful for Windows
> guests that just have a maximum request lantency before throwing a
> BSOD.)
> 
> 
> Hanna Czenczek (9):
>   block/accounting: Add offset to BlockAcctCookie
>   qapi/block: Add IoAccountingOperation enum
>   qapi/block: Add BLOCK_IO_DELAY event
>   block-backend: Public blk_get_attached_dev_path()
>   block/accounting: Add BB field to latency checker
>   block/accounting: Emit BLOCK_IO_DELAY event
>   block: Add delay-alert-ms property
>   block/accounting: Move latency_ns override down
>   iotests: Add delay-alert test
> 
>  qapi/block.json                          |  52 +++++++++
>  include/block/accounting.h               |  20 +++-
>  include/hw/block/block.h                 |   5 +-
>  include/system/block-backend-io.h        |   9 ++
>  include/system/dma.h                     |   2 +-
>  block/accounting.c                       |  60 ++++++++--
>  block/block-backend.c                    |   8 +-
>  blockdev.c                               |  16 ++-
>  hw/block/block.c                         |   4 +-
>  hw/block/dataplane/xen-block.c           |   4 +-
>  hw/block/virtio-blk.c                    |  15 +--
>  hw/ide/ahci.c                            |   6 +-
>  hw/ide/atapi.c                           |   9 +-
>  hw/ide/core.c                            |   9 +-
>  hw/ide/macio.c                           |  15 ++-
>  hw/nvme/ctrl.c                           |  43 ++++---
>  hw/nvme/dif.c                            |   8 +-
>  hw/scsi/scsi-disk.c                      |  28 +++--
>  qemu-io-cmds.c                           |  14 +--
>  system/dma-helpers.c                     |   4 +-
>  tests/unit/test-block-accounting.c       |   2 +-
>  tests/qemu-iotests/172.out               |  38 +++++++
>  tests/qemu-iotests/tests/delay-alert     | 136 +++++++++++++++++++++++
>  tests/qemu-iotests/tests/delay-alert.out |  46 ++++++++
>  24 files changed, 472 insertions(+), 81 deletions(-)
>  create mode 100755 tests/qemu-iotests/tests/delay-alert
>  create mode 100644 tests/qemu-iotests/tests/delay-alert.out
> 
> -- 
> 2.55.0
> 

On 03.09.26 16:08, Stefan Hajnoczi wrote:
> On Mon, Aug 31, 2026 at 03:51:56PM +0200, Hanna Czenczek wrote:
>> Based-on:<20260724152315.234183-1-hreitz@redhat.com>
>>            [PATCH 0/6] hw/[block]: Fix missing accounting
>>            Fri, 24 Jul 2026 17:23:09 +0200
>>
>> Hi,
>>
>> I’m told there are installations where storage is very slow, and some
>> would like the VM stack to report this proactively.  To do so, we should
>> (via QAPI events) report on extremely slow I/O requests.
>>
>> We already have the latency histogram, but this is not deemed sufficient
>> because it is not proactively reporting and would require repeated
>> querying.  Therefore, this series introduces the event still.
>>
>> Now, from a user's perspective, it would be nice if this event could be
>> raised exactly when an I/O request crosses the user-defined threshold,
>> but this would require keeping all active requests in a list and
>> checking it periodically.  Now, if we used latency cookies for this
> Did you consider a per-request timer? The QEMUTimerList active_timers
> sorted list does not support efficient insertion, but improving it would
> benefit all timer API users.

I'm not sure how that would address the problem. Does that not just move 
the list elsewhere?

I.e. it still has the problem that every request needs to be put into a 
list (starting a timer), and be removed when the request is done, or the 
timer will fire and the below problems would occur if the request is 
done but we failed to remove it (either use-after-free; or spurious 
events plus memory leaks on top of a heap allocation per request).

>> (which makes sense), then that would require that every cookie set up is
>> also finalized when the request is done, because if we don't, results
>> could well be catastrophic:
>> - Either we use cookies as-is, which are often allocated on the stack or
>>    in some other structure managed by the device; then this would result
>>    in use-after-free,
>> - Or we allocate something specifically for this checking, so lingering
>>    requests would at most create spurious latency events and memory
>>    leaks, but this would require an additional heap allocation per
>>    request that we would probably want to avoid.
>>
>> So ideally we could use latency cookies and could statically verify that
>> they are always finalized when the request is done, but doing this in C
>> may well be impossible.
> I think you are saying that the cookie API is unsafe because cookie
> lifetime is not bounded by the request lifetime?

Yes, because it is not statically proven to be so.

> Maybe the block_acct_*() API can be integrated into the actual request
> so there is no way to leak the cookie. In other words, directly
> associate requests with a BlockAcctStats and stop requiring the user to
> manually manage a separate BlockAcctCookie.

I don't follow what you mean concretely. If you are suggesting a list of 
requests in BlockAcctStats, then that is exactly what I had.

The problem is that if the destructor has to be called explicitly, we 
may forget to do so; and accounting is done on the device emulation 
level, so there is no central place where the pairing of constructor and 
destructor would be obvious and trivial to verify.

I.e. it’s not like our functions current look like

````
run_co_ide_request() {
     start_cookie();
     run_co_request();
     finalize_cookie();
}
```

Where the pairing of start and finalize are obvious; instead, start and 
stop are in very different parts of the code, in all device emulation code.

> The API is already weird because devices use:
>
>    block_acct_failed(blk_get_stats(s->blk), &req->acct);
>
> i.e. why does the device have to reach into s->blk to access the stats?
> If the stats belong to s->blk, then s->blk should do the accounting
> during the request lifetime.
>
> This would require a redesign of not just the cookie API, but also the
> error policy API. There is also a wrinkle in that virtio-blk merges I/O
> requests and accounts the merges.
>
>> So, because it is basically impossible (or at least it would be very
>> hard, and presumably require a large refactoring) to guarantee, without
>> additional heap allocations, that a list of active requests won’t run
>> into catastrophic use-after-frees, this series does the much simpler
>> version first, which is to just raise an event when a request *finishes*
>> and took more than a user-defined latency threshold.
> Does this achieve the goal of warning when requests exceed a threshold?

According to 
https://redhat.atlassian.net/browse/RHEL-141617?focusedCommentId=18127333 
(and the following two comments), yes.

> When an I/O request hangs for a long time, the management tool will be
> unable to detect that the threshold has been exceeded in a timely
> manner.

Yes. But why would that be a real problem?

Given the request so far is only to notify users of their storage block 
showing problematic latencies at all, there is no need to provide the 
alerts in actual real time.

Hanna

>> (PS: The nice thing about throwing an alert while the request is still
>> going on would be that it could allow us to also stop the VM in case of
>> excessive latency, before the request completes, so the guest would be
>> shielded from such excessive latency.  This might be useful for Windows
>> guests that just have a maximum request lantency before throwing a
>> BSOD.)
>>
>>
>> Hanna Czenczek (9):
>>    block/accounting: Add offset to BlockAcctCookie
>>    qapi/block: Add IoAccountingOperation enum
>>    qapi/block: Add BLOCK_IO_DELAY event
>>    block-backend: Public blk_get_attached_dev_path()
>>    block/accounting: Add BB field to latency checker
>>    block/accounting: Emit BLOCK_IO_DELAY event
>>    block: Add delay-alert-ms property
>>    block/accounting: Move latency_ns override down
>>    iotests: Add delay-alert test
>>
>>   qapi/block.json                          |  52 +++++++++
>>   include/block/accounting.h               |  20 +++-
>>   include/hw/block/block.h                 |   5 +-
>>   include/system/block-backend-io.h        |   9 ++
>>   include/system/dma.h                     |   2 +-
>>   block/accounting.c                       |  60 ++++++++--
>>   block/block-backend.c                    |   8 +-
>>   blockdev.c                               |  16 ++-
>>   hw/block/block.c                         |   4 +-
>>   hw/block/dataplane/xen-block.c           |   4 +-
>>   hw/block/virtio-blk.c                    |  15 +--
>>   hw/ide/ahci.c                            |   6 +-
>>   hw/ide/atapi.c                           |   9 +-
>>   hw/ide/core.c                            |   9 +-
>>   hw/ide/macio.c                           |  15 ++-
>>   hw/nvme/ctrl.c                           |  43 ++++---
>>   hw/nvme/dif.c                            |   8 +-
>>   hw/scsi/scsi-disk.c                      |  28 +++--
>>   qemu-io-cmds.c                           |  14 +--
>>   system/dma-helpers.c                     |   4 +-
>>   tests/unit/test-block-accounting.c       |   2 +-
>>   tests/qemu-iotests/172.out               |  38 +++++++
>>   tests/qemu-iotests/tests/delay-alert     | 136 +++++++++++++++++++++++
>>   tests/qemu-iotests/tests/delay-alert.out |  46 ++++++++
>>   24 files changed, 472 insertions(+), 81 deletions(-)
>>   create mode 100755 tests/qemu-iotests/tests/delay-alert
>>   create mode 100644 tests/qemu-iotests/tests/delay-alert.out
>>
>> -- 
>> 2.55.0
>>

On Wed, Sep 16, 2026 at 10:04:51AM +0200, Hanna Czenczek wrote:
> On 03.09.26 16:08, Stefan Hajnoczi wrote:
> > On Mon, Aug 31, 2026 at 03:51:56PM +0200, Hanna Czenczek wrote:
> > > Based-on:<20260724152315.234183-1-hreitz@redhat.com>
> > >            [PATCH 0/6] hw/[block]: Fix missing accounting
> > >            Fri, 24 Jul 2026 17:23:09 +0200
> > > 
> > > Hi,
> > > 
> > > I’m told there are installations where storage is very slow, and some
> > > would like the VM stack to report this proactively.  To do so, we should
> > > (via QAPI events) report on extremely slow I/O requests.
> > > 
> > > We already have the latency histogram, but this is not deemed sufficient
> > > because it is not proactively reporting and would require repeated
> > > querying.  Therefore, this series introduces the event still.
> > > 
> > > Now, from a user's perspective, it would be nice if this event could be
> > > raised exactly when an I/O request crosses the user-defined threshold,
> > > but this would require keeping all active requests in a list and
> > > checking it periodically.  Now, if we used latency cookies for this
> > Did you consider a per-request timer? The QEMUTimerList active_timers
> > sorted list does not support efficient insertion, but improving it would
> > benefit all timer API users.
> 
> I'm not sure how that would address the problem. Does that not just move the
> list elsewhere?
> 
> I.e. it still has the problem that every request needs to be put into a list
> (starting a timer), and be removed when the request is done, or the timer
> will fire and the below problems would occur if the request is done but we
> failed to remove it (either use-after-free; or spurious events plus memory
> leaks on top of a heap allocation per request).

I wasn't thinking about the lifecycle here, but about the current
limitation that users aren't notified until request completion. Requests
can be stuck for a very long time or forever. Using a timer solves the
timeliness problem.

> > > (which makes sense), then that would require that every cookie set up is
> > > also finalized when the request is done, because if we don't, results
> > > could well be catastrophic:
> > > - Either we use cookies as-is, which are often allocated on the stack or
> > >    in some other structure managed by the device; then this would result
> > >    in use-after-free,
> > > - Or we allocate something specifically for this checking, so lingering
> > >    requests would at most create spurious latency events and memory
> > >    leaks, but this would require an additional heap allocation per
> > >    request that we would probably want to avoid.
> > > 
> > > So ideally we could use latency cookies and could statically verify that
> > > they are always finalized when the request is done, but doing this in C
> > > may well be impossible.
> > I think you are saying that the cookie API is unsafe because cookie
> > lifetime is not bounded by the request lifetime?
> 
> Yes, because it is not statically proven to be so.
> 
> > Maybe the block_acct_*() API can be integrated into the actual request
> > so there is no way to leak the cookie. In other words, directly
> > associate requests with a BlockAcctStats and stop requiring the user to
> > manually manage a separate BlockAcctCookie.
> 
> I don't follow what you mean concretely. If you are suggesting a list of
> requests in BlockAcctStats, then that is exactly what I had.

I meant that each I/O request should contain its own cookie and there is
never a need to create a cookie separately from the request. That way
the lifetime issue is solved.

This would require API changes because device emulation code currently
has some of the logic for cookies. Devices would no longer call
block_acct_done() once they have called blk_aio_pwritev(), for
example. I haven't looked in detail and am not sure if it's feasible.

> The problem is that if the destructor has to be called explicitly, we may
> forget to do so; and accounting is done on the device emulation level, so
> there is no central place where the pairing of constructor and destructor
> would be obvious and trivial to verify.

This is the part I'm asking about: can accounting be done by the block
layer? There might be cases that are purely handled in device emulation
code without a call into the block layer. In that case the accounting
still needs to be done in device emulation code. But when device
emulation calls blk_aio_*(), it should not do accounting itself.

> I.e. it’s not like our functions current look like
> 
> ````
> run_co_ide_request() {
>     start_cookie();
>     run_co_request();
>     finalize_cookie();
> }
> ```
> 
> Where the pairing of start and finalize are obvious; instead, start and stop
> are in very different parts of the code, in all device emulation code.
> 
> > The API is already weird because devices use:
> > 
> >    block_acct_failed(blk_get_stats(s->blk), &req->acct);
> > 
> > i.e. why does the device have to reach into s->blk to access the stats?
> > If the stats belong to s->blk, then s->blk should do the accounting
> > during the request lifetime.
> > 
> > This would require a redesign of not just the cookie API, but also the
> > error policy API. There is also a wrinkle in that virtio-blk merges I/O
> > requests and accounts the merges.
> > 
> > > So, because it is basically impossible (or at least it would be very
> > > hard, and presumably require a large refactoring) to guarantee, without
> > > additional heap allocations, that a list of active requests won’t run
> > > into catastrophic use-after-frees, this series does the much simpler
> > > version first, which is to just raise an event when a request *finishes*
> > > and took more than a user-defined latency threshold.
> > Does this achieve the goal of warning when requests exceed a threshold?
> 
> According to
> https://redhat.atlassian.net/browse/RHEL-141617?focusedCommentId=18127333
> (and the following two comments), yes.

The issue says one purpose of this feature is to pause the guest to
avoid BSODs. This won't work if the request has to finish first, because
the BSOD occurs sometime after the treshold is reached but before QEMU
or any other component can react to the QMP event.

> 
> > When an I/O request hangs for a long time, the management tool will be
> > unable to detect that the threshold has been exceeded in a timely
> > manner.
> 
> Yes. But why would that be a real problem?
> 
> Given the request so far is only to notify users of their storage block
> showing problematic latencies at all, there is no need to provide the alerts
> in actual real time.

What does the current approach solve that is not already possible with
the block latency histogram?

Libvirt could add an API to notify when a particular threshold is
reached without any QEMU changes.

Stefan

> 
> Hanna
> 
> > > (PS: The nice thing about throwing an alert while the request is still
> > > going on would be that it could allow us to also stop the VM in case of
> > > excessive latency, before the request completes, so the guest would be
> > > shielded from such excessive latency.  This might be useful for Windows
> > > guests that just have a maximum request lantency before throwing a
> > > BSOD.)
> > > 
> > > 
> > > Hanna Czenczek (9):
> > >    block/accounting: Add offset to BlockAcctCookie
> > >    qapi/block: Add IoAccountingOperation enum
> > >    qapi/block: Add BLOCK_IO_DELAY event
> > >    block-backend: Public blk_get_attached_dev_path()
> > >    block/accounting: Add BB field to latency checker
> > >    block/accounting: Emit BLOCK_IO_DELAY event
> > >    block: Add delay-alert-ms property
> > >    block/accounting: Move latency_ns override down
> > >    iotests: Add delay-alert test
> > > 
> > >   qapi/block.json                          |  52 +++++++++
> > >   include/block/accounting.h               |  20 +++-
> > >   include/hw/block/block.h                 |   5 +-
> > >   include/system/block-backend-io.h        |   9 ++
> > >   include/system/dma.h                     |   2 +-
> > >   block/accounting.c                       |  60 ++++++++--
> > >   block/block-backend.c                    |   8 +-
> > >   blockdev.c                               |  16 ++-
> > >   hw/block/block.c                         |   4 +-
> > >   hw/block/dataplane/xen-block.c           |   4 +-
> > >   hw/block/virtio-blk.c                    |  15 +--
> > >   hw/ide/ahci.c                            |   6 +-
> > >   hw/ide/atapi.c                           |   9 +-
> > >   hw/ide/core.c                            |   9 +-
> > >   hw/ide/macio.c                           |  15 ++-
> > >   hw/nvme/ctrl.c                           |  43 ++++---
> > >   hw/nvme/dif.c                            |   8 +-
> > >   hw/scsi/scsi-disk.c                      |  28 +++--
> > >   qemu-io-cmds.c                           |  14 +--
> > >   system/dma-helpers.c                     |   4 +-
> > >   tests/unit/test-block-accounting.c       |   2 +-
> > >   tests/qemu-iotests/172.out               |  38 +++++++
> > >   tests/qemu-iotests/tests/delay-alert     | 136 +++++++++++++++++++++++
> > >   tests/qemu-iotests/tests/delay-alert.out |  46 ++++++++
> > >   24 files changed, 472 insertions(+), 81 deletions(-)
> > >   create mode 100755 tests/qemu-iotests/tests/delay-alert
> > >   create mode 100644 tests/qemu-iotests/tests/delay-alert.out
> > > 
> > > -- 
> > > 2.55.0
> > > 
> 

On 21.09.26 22:41, Stefan Hajnoczi wrote:
> On Wed, Sep 16, 2026 at 10:04:51AM +0200, Hanna Czenczek wrote:
>> On 03.09.26 16:08, Stefan Hajnoczi wrote:
>>> On Mon, Aug 31, 2026 at 03:51:56PM +0200, Hanna Czenczek wrote:
>>>> Based-on:<20260724152315.234183-1-hreitz@redhat.com>
>>>>             [PATCH 0/6] hw/[block]: Fix missing accounting
>>>>             Fri, 24 Jul 2026 17:23:09 +0200
>>>>
>>>> Hi,
>>>>
>>>> I’m told there are installations where storage is very slow, and some
>>>> would like the VM stack to report this proactively.  To do so, we should
>>>> (via QAPI events) report on extremely slow I/O requests.
>>>>
>>>> We already have the latency histogram, but this is not deemed sufficient
>>>> because it is not proactively reporting and would require repeated
>>>> querying.  Therefore, this series introduces the event still.
>>>>
>>>> Now, from a user's perspective, it would be nice if this event could be
>>>> raised exactly when an I/O request crosses the user-defined threshold,
>>>> but this would require keeping all active requests in a list and
>>>> checking it periodically.  Now, if we used latency cookies for this
>>> Did you consider a per-request timer? The QEMUTimerList active_timers
>>> sorted list does not support efficient insertion, but improving it would
>>> benefit all timer API users.
>> I'm not sure how that would address the problem. Does that not just move the
>> list elsewhere?
>>
>> I.e. it still has the problem that every request needs to be put into a list
>> (starting a timer), and be removed when the request is done, or the timer
>> will fire and the below problems would occur if the request is done but we
>> failed to remove it (either use-after-free; or spurious events plus memory
>> leaks on top of a heap allocation per request).
> I wasn't thinking about the lifecycle here, but about the current
> limitation that users aren't notified until request completion. Requests
> can be stuck for a very long time or forever. Using a timer solves the
> timeliness problem.

I still don’t follow. The lifecycle *is* the problem if the timeliness. 
How to implement repeated querying is not the problem.

FWIW, the series that I had with timely waking did use a timer. But the 
timer is just not the problem.

>>>> (which makes sense), then that would require that every cookie set up is
>>>> also finalized when the request is done, because if we don't, results
>>>> could well be catastrophic:
>>>> - Either we use cookies as-is, which are often allocated on the stack or
>>>>     in some other structure managed by the device; then this would result
>>>>     in use-after-free,
>>>> - Or we allocate something specifically for this checking, so lingering
>>>>     requests would at most create spurious latency events and memory
>>>>     leaks, but this would require an additional heap allocation per
>>>>     request that we would probably want to avoid.
>>>>
>>>> So ideally we could use latency cookies and could statically verify that
>>>> they are always finalized when the request is done, but doing this in C
>>>> may well be impossible.
>>> I think you are saying that the cookie API is unsafe because cookie
>>> lifetime is not bounded by the request lifetime?
>> Yes, because it is not statically proven to be so.
>>
>>> Maybe the block_acct_*() API can be integrated into the actual request
>>> so there is no way to leak the cookie. In other words, directly
>>> associate requests with a BlockAcctStats and stop requiring the user to
>>> manually manage a separate BlockAcctCookie.
>> I don't follow what you mean concretely. If you are suggesting a list of
>> requests in BlockAcctStats, then that is exactly what I had.
> I meant that each I/O request should contain its own cookie and there is
> never a need to create a cookie separately from the request. That way
> the lifetime issue is solved.

Okay, I understood with your other email, because to me, “I/O request” 
sounded like some kind of object and I couldn’t think of any. My mind 
did not jump to the fact that you meant the actual execution thread of 
the request, i.e. code, not structure.

> This would require API changes because device emulation code currently
> has some of the logic for cookies. Devices would no longer call
> block_acct_done() once they have called blk_aio_pwritev(), for
> example. I haven't looked in detail and am not sure if it's feasible.

As said in the other email, doing this would most likely change behavior 
because we can’t or don’t want to reproduce all currently existing 
quirks. Which may be good or bad. Bad for me in any case because it 
would be more work, but, well…

>> The problem is that if the destructor has to be called explicitly, we may
>> forget to do so; and accounting is done on the device emulation level, so
>> there is no central place where the pairing of constructor and destructor
>> would be obvious and trivial to verify.
> This is the part I'm asking about: can accounting be done by the block
> layer? There might be cases that are purely handled in device emulation
> code without a call into the block layer. In that case the accounting
> still needs to be done in device emulation code. But when device
> emulation calls blk_aio_*(), it should not do accounting itself.
>
>> I.e. it’s not like our functions current look like
>>
>> ````
>> run_co_ide_request() {
>>      start_cookie();
>>      run_co_request();
>>      finalize_cookie();
>> }
>> ```
>>
>> Where the pairing of start and finalize are obvious; instead, start and stop
>> are in very different parts of the code, in all device emulation code.
>>
>>> The API is already weird because devices use:
>>>
>>>     block_acct_failed(blk_get_stats(s->blk), &req->acct);
>>>
>>> i.e. why does the device have to reach into s->blk to access the stats?
>>> If the stats belong to s->blk, then s->blk should do the accounting
>>> during the request lifetime.
>>>
>>> This would require a redesign of not just the cookie API, but also the
>>> error policy API. There is also a wrinkle in that virtio-blk merges I/O
>>> requests and accounts the merges.
>>>
>>>> So, because it is basically impossible (or at least it would be very
>>>> hard, and presumably require a large refactoring) to guarantee, without
>>>> additional heap allocations, that a list of active requests won’t run
>>>> into catastrophic use-after-frees, this series does the much simpler
>>>> version first, which is to just raise an event when a request *finishes*
>>>> and took more than a user-defined latency threshold.
>>> Does this achieve the goal of warning when requests exceed a threshold?
>> According to
>> https://redhat.atlassian.net/browse/RHEL-141617?focusedCommentId=18127333
>> (and the following two comments), yes.
> The issue says one purpose of this feature is to pause the guest to
> avoid BSODs. This won't work if the request has to finish first, because
> the BSOD occurs sometime after the treshold is reached but before QEMU
> or any other component can react to the QMP event.

I know. I linked to a specific comment chain that says it is enough for now.

Whether the pausing is something anyone actually would ever want is a 
completely different story, honestly. It is an idea I had for the 
original design, but nothing that has actually ever been requested.

>>> When an I/O request hangs for a long time, the management tool will be
>>> unable to detect that the threshold has been exceeded in a timely
>>> manner.
>> Yes. But why would that be a real problem?
>>
>> Given the request so far is only to notify users of their storage block
>> showing problematic latencies at all, there is no need to provide the alerts
>> in actual real time.
> What does the current approach solve that is not already possible with
> the block latency histogram?

See what I linked above. Apparently having to repeatedly query the 
histogram repeatedly is not deemed nice enough.

Hanna

> Libvirt could add an API to notify when a particular threshold is
> reached without any QEMU changes.
>
> Stefan

On Wed, Sep 23, 2026 at 12:48:36PM +0200, Hanna Czenczek wrote:
> On 21.09.26 22:41, Stefan Hajnoczi wrote:
> > On Wed, Sep 16, 2026 at 10:04:51AM +0200, Hanna Czenczek wrote:
> > > On 03.09.26 16:08, Stefan Hajnoczi wrote:
> > > > On Mon, Aug 31, 2026 at 03:51:56PM +0200, Hanna Czenczek wrote:
> > > > > Based-on:<20260724152315.234183-1-hreitz@redhat.com>
> > > > >             [PATCH 0/6] hw/[block]: Fix missing accounting
> > > > >             Fri, 24 Jul 2026 17:23:09 +0200
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > I’m told there are installations where storage is very slow, and some
> > > > > would like the VM stack to report this proactively.  To do so, we should
> > > > > (via QAPI events) report on extremely slow I/O requests.
> > > > > 
> > > > > We already have the latency histogram, but this is not deemed sufficient
> > > > > because it is not proactively reporting and would require repeated
> > > > > querying.  Therefore, this series introduces the event still.
> > > > > 
> > > > > Now, from a user's perspective, it would be nice if this event could be
> > > > > raised exactly when an I/O request crosses the user-defined threshold,
> > > > > but this would require keeping all active requests in a list and
> > > > > checking it periodically.  Now, if we used latency cookies for this
> > > > Did you consider a per-request timer? The QEMUTimerList active_timers
> > > > sorted list does not support efficient insertion, but improving it would
> > > > benefit all timer API users.
> > > I'm not sure how that would address the problem. Does that not just move the
> > > list elsewhere?
> > > 
> > > I.e. it still has the problem that every request needs to be put into a list
> > > (starting a timer), and be removed when the request is done, or the timer
> > > will fire and the below problems would occur if the request is done but we
> > > failed to remove it (either use-after-free; or spurious events plus memory
> > > leaks on top of a heap allocation per request).
> > I wasn't thinking about the lifecycle here, but about the current
> > limitation that users aren't notified until request completion. Requests
> > can be stuck for a very long time or forever. Using a timer solves the
> > timeliness problem.
> 
> I still don’t follow. The lifecycle *is* the problem if the timeliness. How
> to implement repeated querying is not the problem.
> 
> FWIW, the series that I had with timely waking did use a timer. But the
> timer is just not the problem.

Regarding the lifecycle, I don't see a fundamental problem. It's
possible to add a timer to block layer I/O requests and know for certain
that leaks, use-after-free, etc are not possible because the caller
doesn't need to juggle anything. Even if the caller has to juggle
something, this is a C codebase where APIs are not always safe. That's
not a blocker as long as they API design allows disciplined users to use
it correctly. If I misunderstood your concerns and there is a
fundamental reason why the lifetime cannot be made correct, maybe you
can explain?

> > > > > (which makes sense), then that would require that every cookie set up is
> > > > > also finalized when the request is done, because if we don't, results
> > > > > could well be catastrophic:
> > > > > - Either we use cookies as-is, which are often allocated on the stack or
> > > > >     in some other structure managed by the device; then this would result
> > > > >     in use-after-free,
> > > > > - Or we allocate something specifically for this checking, so lingering
> > > > >     requests would at most create spurious latency events and memory
> > > > >     leaks, but this would require an additional heap allocation per
> > > > >     request that we would probably want to avoid.
> > > > > 
> > > > > So ideally we could use latency cookies and could statically verify that
> > > > > they are always finalized when the request is done, but doing this in C
> > > > > may well be impossible.
> > > > I think you are saying that the cookie API is unsafe because cookie
> > > > lifetime is not bounded by the request lifetime?
> > > Yes, because it is not statically proven to be so.
> > > 
> > > > Maybe the block_acct_*() API can be integrated into the actual request
> > > > so there is no way to leak the cookie. In other words, directly
> > > > associate requests with a BlockAcctStats and stop requiring the user to
> > > > manually manage a separate BlockAcctCookie.
> > > I don't follow what you mean concretely. If you are suggesting a list of
> > > requests in BlockAcctStats, then that is exactly what I had.
> > I meant that each I/O request should contain its own cookie and there is
> > never a need to create a cookie separately from the request. That way
> > the lifetime issue is solved.
> 
> Okay, I understood with your other email, because to me, “I/O request”
> sounded like some kind of object and I couldn’t think of any. My mind did
> not jump to the fact that you meant the actual execution thread of the
> request, i.e. code, not structure.
> 
> > This would require API changes because device emulation code currently
> > has some of the logic for cookies. Devices would no longer call
> > block_acct_done() once they have called blk_aio_pwritev(), for
> > example. I haven't looked in detail and am not sure if it's feasible.
> 
> As said in the other email, doing this would most likely change behavior
> because we can’t or don’t want to reproduce all currently existing quirks.
> Which may be good or bad. Bad for me in any case because it would be more
> work, but, well…
> 
> > > The problem is that if the destructor has to be called explicitly, we may
> > > forget to do so; and accounting is done on the device emulation level, so
> > > there is no central place where the pairing of constructor and destructor
> > > would be obvious and trivial to verify.
> > This is the part I'm asking about: can accounting be done by the block
> > layer? There might be cases that are purely handled in device emulation
> > code without a call into the block layer. In that case the accounting
> > still needs to be done in device emulation code. But when device
> > emulation calls blk_aio_*(), it should not do accounting itself.
> > 
> > > I.e. it’s not like our functions current look like
> > > 
> > > ````
> > > run_co_ide_request() {
> > >      start_cookie();
> > >      run_co_request();
> > >      finalize_cookie();
> > > }
> > > ```
> > > 
> > > Where the pairing of start and finalize are obvious; instead, start and stop
> > > are in very different parts of the code, in all device emulation code.
> > > 
> > > > The API is already weird because devices use:
> > > > 
> > > >     block_acct_failed(blk_get_stats(s->blk), &req->acct);
> > > > 
> > > > i.e. why does the device have to reach into s->blk to access the stats?
> > > > If the stats belong to s->blk, then s->blk should do the accounting
> > > > during the request lifetime.
> > > > 
> > > > This would require a redesign of not just the cookie API, but also the
> > > > error policy API. There is also a wrinkle in that virtio-blk merges I/O
> > > > requests and accounts the merges.
> > > > 
> > > > > So, because it is basically impossible (or at least it would be very
> > > > > hard, and presumably require a large refactoring) to guarantee, without
> > > > > additional heap allocations, that a list of active requests won’t run
> > > > > into catastrophic use-after-frees, this series does the much simpler
> > > > > version first, which is to just raise an event when a request *finishes*
> > > > > and took more than a user-defined latency threshold.
> > > > Does this achieve the goal of warning when requests exceed a threshold?
> > > According to
> > > https://redhat.atlassian.net/browse/RHEL-141617?focusedCommentId=18127333
> > > (and the following two comments), yes.
> > The issue says one purpose of this feature is to pause the guest to
> > avoid BSODs. This won't work if the request has to finish first, because
> > the BSOD occurs sometime after the treshold is reached but before QEMU
> > or any other component can react to the QMP event.
> 
> I know. I linked to a specific comment chain that says it is enough for now.
> 
> Whether the pausing is something anyone actually would ever want is a
> completely different story, honestly. It is an idea I had for the original
> design, but nothing that has actually ever been requested.
>
> > > > When an I/O request hangs for a long time, the management tool will be
> > > > unable to detect that the threshold has been exceeded in a timely
> > > > manner.
> > > Yes. But why would that be a real problem?
> > > 
> > > Given the request so far is only to notify users of their storage block
> > > showing problematic latencies at all, there is no need to provide the alerts
> > > in actual real time.
> > What does the current approach solve that is not already possible with
> > the block latency histogram?
> 
> See what I linked above. Apparently having to repeatedly query the histogram
> repeatedly is not deemed nice enough.

Why at the QEMU level though? Libvirt can offer an event based on the
block histogram.

The utility of this new feature is not clear to me. How is this QMP
event actually going to be used? Usually a feature has a clear use case
and the cost of committing to the QMP API and maintaining backwards
compatibility is easy to justify. In this case I'm not sure what the
real use case is or whether this is something someone thought might be
nice to have but may never use - we'll have to maintain it forever
either way.

(Timeliness makes the need for this feature clear to me. QEMU has to be
involved either by pausing the guest itself or by emitting a QMP event.
This is why I'm particularly interested in timeliness.)

Stefan

Am 24.09.2026 um 17:17 hat Stefan Hajnoczi geschrieben:
> On Wed, Sep 23, 2026 at 12:48:36PM +0200, Hanna Czenczek wrote:
> > On 21.09.26 22:41, Stefan Hajnoczi wrote:
> > > On Wed, Sep 16, 2026 at 10:04:51AM +0200, Hanna Czenczek wrote:
> > > > On 03.09.26 16:08, Stefan Hajnoczi wrote:
> > > > > On Mon, Aug 31, 2026 at 03:51:56PM +0200, Hanna Czenczek wrote:
> > > > > > Based-on:<20260724152315.234183-1-hreitz@redhat.com>
> > > > > >             [PATCH 0/6] hw/[block]: Fix missing accounting
> > > > > >             Fri, 24 Jul 2026 17:23:09 +0200
> > > > > > 
> > > > > > Hi,
> > > > > > 
> > > > > > I’m told there are installations where storage is very slow, and some
> > > > > > would like the VM stack to report this proactively.  To do so, we should
> > > > > > (via QAPI events) report on extremely slow I/O requests.
> > > > > > 
> > > > > > We already have the latency histogram, but this is not deemed sufficient
> > > > > > because it is not proactively reporting and would require repeated
> > > > > > querying.  Therefore, this series introduces the event still.
> > > > > > 
> > > > > > Now, from a user's perspective, it would be nice if this event could be
> > > > > > raised exactly when an I/O request crosses the user-defined threshold,
> > > > > > but this would require keeping all active requests in a list and
> > > > > > checking it periodically.  Now, if we used latency cookies for this
> > > > > Did you consider a per-request timer? The QEMUTimerList active_timers
> > > > > sorted list does not support efficient insertion, but improving it would
> > > > > benefit all timer API users.
> > > > I'm not sure how that would address the problem. Does that not just move the
> > > > list elsewhere?
> > > > 
> > > > I.e. it still has the problem that every request needs to be put into a list
> > > > (starting a timer), and be removed when the request is done, or the timer
> > > > will fire and the below problems would occur if the request is done but we
> > > > failed to remove it (either use-after-free; or spurious events plus memory
> > > > leaks on top of a heap allocation per request).
> > > I wasn't thinking about the lifecycle here, but about the current
> > > limitation that users aren't notified until request completion. Requests
> > > can be stuck for a very long time or forever. Using a timer solves the
> > > timeliness problem.
> > 
> > I still don’t follow. The lifecycle *is* the problem if the timeliness. How
> > to implement repeated querying is not the problem.
> > 
> > FWIW, the series that I had with timely waking did use a timer. But the
> > timer is just not the problem.
> 
> Regarding the lifecycle, I don't see a fundamental problem. It's
> possible to add a timer to block layer I/O requests and know for certain
> that leaks, use-after-free, etc are not possible because the caller
> doesn't need to juggle anything. Even if the caller has to juggle
> something, this is a C codebase where APIs are not always safe. That's
> not a blocker as long as they API design allows disciplined users to use
> it correctly. If I misunderstood your concerns and there is a
> fundamental reason why the lifetime cannot be made correct, maybe you
> can explain?

I don't think there is a fundamental reason other than that humans are
bad at writing correct C code, but that this is based more on Hanna's
finding that our existing code has bugs in this respect and she isn't
sure if she caught all of them.

The current consequence of missing the end of a request is that it isn't
accounted for, and that's it. After the change, it means that a timer
callback will fire long after the request is gone, and therefore work on
a stale pointer, so it may turn into memory corruption and crashes.

Maybe this can be mitigated by not referencing data bound to the request
lifetime, but just keeping a copy of type/offset/length or whatever is
needed for emitting the QAPI event, at the cost of a heap allocation as
Hanna explained in the cover letter. It would still mean that a bogus
timeout is reported.

So I agree that we'd want to have those cases fixed before depending on
correct block_acct_start()/block_account_one_io() pairing.

And I'm afraid that Hanna is also right that it might be impossible to
get this done by static analysis in our callback-heavy device
implementations. If the full request control flow were in a coroutine,
TSA could probably do it, but I don't think it can work with our actual
code. Maybe simple cases like virtio-blk could be covered with the
latest TSA improvements coming from the kernel (I would have to check if
they cover callbacks well enough), but the scsi-disk state machine looks
too complicated for this.

> > > > > When an I/O request hangs for a long time, the management tool will be
> > > > > unable to detect that the threshold has been exceeded in a timely
> > > > > manner.
> > > > Yes. But why would that be a real problem?
> > > > 
> > > > Given the request so far is only to notify users of their storage block
> > > > showing problematic latencies at all, there is no need to provide the alerts
> > > > in actual real time.
> > > What does the current approach solve that is not already possible with
> > > the block latency histogram?
> > 
> > See what I linked above. Apparently having to repeatedly query the histogram
> > repeatedly is not deemed nice enough.
> 
> Why at the QEMU level though? Libvirt can offer an event based on the
> block histogram.

At what intervals should libvirt poll QEMU? And why should polling be
better than using an event?

> The utility of this new feature is not clear to me. How is this QMP
> event actually going to be used? Usually a feature has a clear use case
> and the cost of committing to the QMP API and maintaining backwards
> compatibility is easy to justify. In this case I'm not sure what the
> real use case is or whether this is something someone thought might be
> nice to have but may never use - we'll have to maintain it forever
> either way.

The idea is to show a warning to the user that their storage has latency
issues and they need to look into that. Ideally before the latency
reaches critical values that result in errors.

> (Timeliness makes the need for this feature clear to me. QEMU has to be
> involved either by pausing the guest itself or by emitting a QMP event.
> This is why I'm particularly interested in timeliness.)

If you want to prevent a in-flight request from timing out in the guest,
then yes, QEMU would have pause the guest. I think emitting a QMP event
alone wouldn't be good enough for this. (Even more so considering that a
'stop' command actually involves draining all requests.)

Kevin

Am 23.09.2026 um 12:48 hat Hanna Czenczek geschrieben:
> > > > When an I/O request hangs for a long time, the management tool will be
> > > > unable to detect that the threshold has been exceeded in a timely
> > > > manner.
> > > Yes. But why would that be a real problem?
> > > 
> > > Given the request so far is only to notify users of their storage block
> > > showing problematic latencies at all, there is no need to provide the alerts
> > > in actual real time.
> > What does the current approach solve that is not already possible with
> > the block latency histogram?
> 
> See what I linked above. Apparently having to repeatedly query the histogram
> repeatedly is not deemed nice enough.

It's the difference between an event and active polling. I would agree
that active polling isn't the right answer in most cases.

Kevin

Am 21.09.2026 um 22:41 hat Stefan Hajnoczi geschrieben:
> On Wed, Sep 16, 2026 at 10:04:51AM +0200, Hanna Czenczek wrote:
> > On 03.09.26 16:08, Stefan Hajnoczi wrote:
> > > On Mon, Aug 31, 2026 at 03:51:56PM +0200, Hanna Czenczek wrote:
> > > > Based-on:<20260724152315.234183-1-hreitz@redhat.com>
> > > >            [PATCH 0/6] hw/[block]: Fix missing accounting
> > > >            Fri, 24 Jul 2026 17:23:09 +0200
> > > > 
> > > > Hi,
> > > > 
> > > > I’m told there are installations where storage is very slow, and some
> > > > would like the VM stack to report this proactively.  To do so, we should
> > > > (via QAPI events) report on extremely slow I/O requests.
> > > > 
> > > > We already have the latency histogram, but this is not deemed sufficient
> > > > because it is not proactively reporting and would require repeated
> > > > querying.  Therefore, this series introduces the event still.
> > > > 
> > > > Now, from a user's perspective, it would be nice if this event could be
> > > > raised exactly when an I/O request crosses the user-defined threshold,
> > > > but this would require keeping all active requests in a list and
> > > > checking it periodically.  Now, if we used latency cookies for this
> > > Did you consider a per-request timer? The QEMUTimerList active_timers
> > > sorted list does not support efficient insertion, but improving it would
> > > benefit all timer API users.
> > 
> > I'm not sure how that would address the problem. Does that not just move the
> > list elsewhere?
> > 
> > I.e. it still has the problem that every request needs to be put into a list
> > (starting a timer), and be removed when the request is done, or the timer
> > will fire and the below problems would occur if the request is done but we
> > failed to remove it (either use-after-free; or spurious events plus memory
> > leaks on top of a heap allocation per request).
> 
> I wasn't thinking about the lifecycle here, but about the current
> limitation that users aren't notified until request completion. Requests
> can be stuck for a very long time or forever. Using a timer solves the
> timeliness problem.

I think we'll need to do this eventually. There was also talk about
stopping the VM if a request hangs for too long, which will definitely
need a timer.

But I think this specific series can work without it for now. Once we do
have the timer anyway, reporting the latency right when the threshold is
crossed can still be done.

We should be careful with the wording in the documentation to allow both
behaviours so we can make this change in the future.

> > > > (which makes sense), then that would require that every cookie set up is
> > > > also finalized when the request is done, because if we don't, results
> > > > could well be catastrophic:
> > > > - Either we use cookies as-is, which are often allocated on the stack or
> > > >    in some other structure managed by the device; then this would result
> > > >    in use-after-free,
> > > > - Or we allocate something specifically for this checking, so lingering
> > > >    requests would at most create spurious latency events and memory
> > > >    leaks, but this would require an additional heap allocation per
> > > >    request that we would probably want to avoid.
> > > > 
> > > > So ideally we could use latency cookies and could statically verify that
> > > > they are always finalized when the request is done, but doing this in C
> > > > may well be impossible.
> > > I think you are saying that the cookie API is unsafe because cookie
> > > lifetime is not bounded by the request lifetime?
> > 
> > Yes, because it is not statically proven to be so.
> > 
> > > Maybe the block_acct_*() API can be integrated into the actual request
> > > so there is no way to leak the cookie. In other words, directly
> > > associate requests with a BlockAcctStats and stop requiring the user to
> > > manually manage a separate BlockAcctCookie.
> > 
> > I don't follow what you mean concretely. If you are suggesting a list of
> > requests in BlockAcctStats, then that is exactly what I had.
> 
> I meant that each I/O request should contain its own cookie and there is
> never a need to create a cookie separately from the request. That way
> the lifetime issue is solved.
> 
> This would require API changes because device emulation code currently
> has some of the logic for cookies. Devices would no longer call
> block_acct_done() once they have called blk_aio_pwritev(), for
> example. I haven't looked in detail and am not sure if it's feasible.
> 
> > The problem is that if the destructor has to be called explicitly, we may
> > forget to do so; and accounting is done on the device emulation level, so
> > there is no central place where the pairing of constructor and destructor
> > would be obvious and trivial to verify.
> 
> This is the part I'm asking about: can accounting be done by the block
> layer? There might be cases that are purely handled in device emulation
> code without a call into the block layer. In that case the accounting
> still needs to be done in device emulation code. But when device
> emulation calls blk_aio_*(), it should not do accounting itself.

Apart from cases where requests are completed entirely within the
device (like for all block_acct_invalid() callers), there are also cases
where a single device-level requests involves multiple backend-level
requests. I was thinking of IDE TRIM initially, but actually I think
splitting can happen for any request that uses the DMA helpers.

Conversely, virtio-blk can merge requests, so you get a single request
in the backend that covers multiple requests in the device.

Kevin

On Tue, Sep 22, 2026 at 03:31:49PM +0200, Kevin Wolf wrote:
> Am 21.09.2026 um 22:41 hat Stefan Hajnoczi geschrieben:
> > On Wed, Sep 16, 2026 at 10:04:51AM +0200, Hanna Czenczek wrote:
> > > On 03.09.26 16:08, Stefan Hajnoczi wrote:
> > > > On Mon, Aug 31, 2026 at 03:51:56PM +0200, Hanna Czenczek wrote:
> > > The problem is that if the destructor has to be called explicitly, we may
> > > forget to do so; and accounting is done on the device emulation level, so
> > > there is no central place where the pairing of constructor and destructor
> > > would be obvious and trivial to verify.
> > 
> > This is the part I'm asking about: can accounting be done by the block
> > layer? There might be cases that are purely handled in device emulation
> > code without a call into the block layer. In that case the accounting
> > still needs to be done in device emulation code. But when device
> > emulation calls blk_aio_*(), it should not do accounting itself.
> 
> Apart from cases where requests are completed entirely within the
> device (like for all block_acct_invalid() callers), there are also cases
> where a single device-level requests involves multiple backend-level
> requests. I was thinking of IDE TRIM initially, but actually I think
> splitting can happen for any request that uses the DMA helpers.
> 
> Conversely, virtio-blk can merge requests, so you get a single request
> in the backend that covers multiple requests in the device.

Sticking to the requests as seen by the device seems like the cleanest
solution rather than cheating and counting host requests in some places.

The idea to move the accounting into blk_aio_*() doesn't work well in
light of this.

Stefan

On 22.09.26 19:18, Stefan Hajnoczi wrote:
> On Tue, Sep 22, 2026 at 03:31:49PM +0200, Kevin Wolf wrote:
>> Am 21.09.2026 um 22:41 hat Stefan Hajnoczi geschrieben:
>>> On Wed, Sep 16, 2026 at 10:04:51AM +0200, Hanna Czenczek wrote:
>>>> On 03.09.26 16:08, Stefan Hajnoczi wrote:
>>>>> On Mon, Aug 31, 2026 at 03:51:56PM +0200, Hanna Czenczek wrote:
>>>> The problem is that if the destructor has to be called explicitly, we may
>>>> forget to do so; and accounting is done on the device emulation level, so
>>>> there is no central place where the pairing of constructor and destructor
>>>> would be obvious and trivial to verify.
>>> This is the part I'm asking about: can accounting be done by the block
>>> layer? There might be cases that are purely handled in device emulation
>>> code without a call into the block layer. In that case the accounting
>>> still needs to be done in device emulation code. But when device
>>> emulation calls blk_aio_*(), it should not do accounting itself.
>> Apart from cases where requests are completed entirely within the
>> device (like for all block_acct_invalid() callers), there are also cases
>> where a single device-level requests involves multiple backend-level
>> requests. I was thinking of IDE TRIM initially, but actually I think
>> splitting can happen for any request that uses the DMA helpers.
>>
>> Conversely, virtio-blk can merge requests, so you get a single request
>> in the backend that covers multiple requests in the device.
> Sticking to the requests as seen by the device seems like the cleanest
> solution rather than cheating and counting host requests in some places.
>
> The idea to move the accounting into blk_aio_*() doesn't work well in
> light of this.

Does this mean you would be against separating delay monitoring from the 
rest of accounting?

Because to me that still sounds reasonable: To do delay monitoring in 
the BB layer, separate from accounting cookies, to have a simple 
lifecycle and timely reporting.

(And as you did say, if one wants the guest-device-request-level delay 
information, the histogram is there O:) )

Hanna

On Wed, Sep 23, 2026 at 12:52:17PM +0200, Hanna Czenczek wrote:
> On 22.09.26 19:18, Stefan Hajnoczi wrote:
> > On Tue, Sep 22, 2026 at 03:31:49PM +0200, Kevin Wolf wrote:
> > > Am 21.09.2026 um 22:41 hat Stefan Hajnoczi geschrieben:
> > > > On Wed, Sep 16, 2026 at 10:04:51AM +0200, Hanna Czenczek wrote:
> > > > > On 03.09.26 16:08, Stefan Hajnoczi wrote:
> > > > > > On Mon, Aug 31, 2026 at 03:51:56PM +0200, Hanna Czenczek wrote:
> > > > > The problem is that if the destructor has to be called explicitly, we may
> > > > > forget to do so; and accounting is done on the device emulation level, so
> > > > > there is no central place where the pairing of constructor and destructor
> > > > > would be obvious and trivial to verify.
> > > > This is the part I'm asking about: can accounting be done by the block
> > > > layer? There might be cases that are purely handled in device emulation
> > > > code without a call into the block layer. In that case the accounting
> > > > still needs to be done in device emulation code. But when device
> > > > emulation calls blk_aio_*(), it should not do accounting itself.
> > > Apart from cases where requests are completed entirely within the
> > > device (like for all block_acct_invalid() callers), there are also cases
> > > where a single device-level requests involves multiple backend-level
> > > requests. I was thinking of IDE TRIM initially, but actually I think
> > > splitting can happen for any request that uses the DMA helpers.
> > > 
> > > Conversely, virtio-blk can merge requests, so you get a single request
> > > in the backend that covers multiple requests in the device.
> > Sticking to the requests as seen by the device seems like the cleanest
> > solution rather than cheating and counting host requests in some places.
> > 
> > The idea to move the accounting into blk_aio_*() doesn't work well in
> > light of this.
> 
> Does this mean you would be against separating delay monitoring from the
> rest of accounting?

I'm not against it. I just don't think we can push accounting down into
the block layer since accounting operates at the emulated device's
request level and there isn't a 1:1 correspondence between block layer
I/O requests and device level requests.

> Because to me that still sounds reasonable: To do delay monitoring in the BB
> layer, separate from accounting cookies, to have a simple lifecycle and
> timely reporting.

Yes.

Stefan