On Mon, Aug 31, 2026 at 03:51:56PM +0200, Hanna Czenczek wrote: > Based-on: <[email protected]> > [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 >
signature.asc
Description: PGP signature
