Re: [PATCH v2] block: Never drop BLOCK_IO_ERROR with action=stop for rate limiting

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Mar 04, 2026 at 01:28:00PM +0100, Kevin Wolf wrote:
> Commit 2155d2dd introduced rate limiting for BLOCK_IO_ERROR to emit an
> event only once a second. This makes sense for cases in which the guest
> keeps running and can submit more requests that would possibly also fail
> because there is a problem with the backend.
> 
> However, if the error policy is configured so that the VM is stopped on
> errors, this is both unnecessary because stopping the VM means that the
> guest can't issue more requests and in fact harmful because stopping the
> VM is an important state change that management tools need to keep track
> of even if it happens more than once in a given second. If an event is
> dropped, the management tool would see a VM randomly going to paused
> state without an associated error, so it has a hard time deciding how to
> handle the situation.
> 
> This patch disables rate limiting for action=stop by not relying on the
> event type alone any more in monitor_qapi_event_queue_no_reenter(), but
> checking action for BLOCK_IO_ERROR, too. If the error is reported to the
> guest or ignored, the rate limiting stays in place.
> 
> Fixes: 2155d2dd7f73 ('block-backend: per-device throttling of BLOCK_IO_ERROR reports')
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qapi/block-core.json |  2 +-
>  monitor/monitor.c    | 21 ++++++++++++++++++++-
>  2 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index b66bf316e2f..da0b36a3751 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -5794,7 +5794,7 @@
>  # .. note:: If action is "stop", a `STOP` event will eventually follow
>  #    the `BLOCK_IO_ERROR` event.
>  #
> -# .. note:: This event is rate-limited.
> +# .. note:: This event is rate-limited, except if action is "stop".
>  #
>  # Since: 0.13
>  #
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index 1273eb72605..37fa674cfe6 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -367,14 +367,33 @@ monitor_qapi_event_queue_no_reenter(QAPIEvent event, QDict *qdict)
>  {
>      MonitorQAPIEventConf *evconf;
>      MonitorQAPIEventState *evstate;
> +    bool throttled;
>  
>      assert(event < QAPI_EVENT__MAX);
>      evconf = &monitor_qapi_event_conf[event];
>      trace_monitor_protocol_event_queue(event, qdict, evconf->rate);
> +    throttled = evconf->rate;
> +
> +    /*
> +     * Rate limit BLOCK_IO_ERROR only for action != "stop".
> +     *
> +     * If the VM is stopped after an I/O error, this is important information
> +     * for the management tool to keep track of the state of QEMU and we can't
> +     * merge any events. At the same time, stopping the VM means that the guest
> +     * can't send additional requests and the number of events is already
> +     * limited, so we can do without rate limiting.
> +     */
> +    if (event == QAPI_EVENT_BLOCK_IO_ERROR) {
> +        QDict *data = qobject_to(QDict, qdict_get(qdict, "data"));
> +        const char *action = qdict_get_str(data, "action");
> +        if (!strcmp(action, "stop")) {
> +            throttled = false;
> +        }
> +    }

Can this be handled in the same way as other events viat he
qapi_event_throttle_hash & qapi_event_throttle_equal methods ?

eg if action is "stop", then ensure "equal" is always false ?
Possibly add a random token to the hash but might not be needed
if 'equal' is always false

>  
>      QEMU_LOCK_GUARD(&monitor_lock);
>  
> -    if (!evconf->rate) {
> +    if (!throttled) {
>          /* Unthrottled event */
>          monitor_qapi_event_emit(event, qdict);
>      } else {
> -- 
> 2.53.0
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|