Re: [PATCH 0/9] block: BLOCK_IO_DELAY event

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 5858 bytes --]

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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]