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