[PULL 0/2] Block layer patches

[PULL 0/2] Block layer patches

[Hanna Czenczek]

The following changes since commit 559919ce54927d59b215a4665eda7ab6118a48aa:

  Merge tag 'pull-trivial-patches' of https://gitlab.com/mjt0k/qemu into staging (2026-03-16 13:07:33 +0000)

are available in the Git repository at:

  https://gitlab.com/hreitz/qemu.git tags/pull-block-2026-03-17

for you to fetch changes up to 9ac85f4cc7995217db8f736733b990d6addcb036:

  block/mirror: fix assertion failure upon duplicate complete for job using 'replaces' (2026-03-17 12:31:54 +0100)

----------------------------------------------------------------
Block layer patches for rc0

- Fix race condition in throttle-group code triggering an assertion
  failure
- Fix assertion failure in mirror job when issuing job-complete twice

----------------------------------------------------------------
Alberto Garcia (1):
  throttle-group: Fix race condition in throttle_group_restart_queue()

Fiona Ebner (1):
  block/mirror: fix assertion failure upon duplicate complete for job
    using 'replaces'

 block/mirror.c          | 28 ++++++++-------
 block/throttle-groups.c | 79 +++++++++++++++++++++++++++++++----------
 2 files changed, 75 insertions(+), 32 deletions(-)

-- 
2.53.0

[PULL 1/2] throttle-group: Fix race condition in throttle_group_restart_queue()

[Hanna Czenczek]

From: Alberto Garcia <berto@igalia.com>

When a timer is fired a pending I/O request is restarted and
tg->any_timer_armed is reset so other requests can be scheduled.

However we're resetting any_timer_armed first in timer_cb() before
the request is actually restarted, and there's a window between both
moments in which another thread can arm the same timer, hitting an
assertion in throttle_group_restart_queue().

This can be solved by deferring the reset of tg->any_timer_armed to
the moment when the queue is actually restarted, which is protected by
tg->lock, preventing other threads from arming the timer before that.

In addition to that, throttle_group_restart_tgm() is also updated to
hold tg->lock while the timer is being inspected. Here we consider
three different scenarios:

- If the tgm has a timer set, fire it immediately
- If another tgm has a timer set, restart the queue anyway
- If there is no timer set in this group then simulate a timer that
  fires immediately, by setting tg->any_timer_armed in order to
  prevent other threads from arming a timer in the meantime.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3194
Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-Id: <825598ef34ad384d936da19d634eda75598508f7.1773316842.git.berto@igalia.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
---
 block/throttle-groups.c | 79 +++++++++++++++++++++++++++++++----------
 1 file changed, 60 insertions(+), 19 deletions(-)

diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 5329ff1fdb..4b1b1944c2 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -391,6 +391,7 @@ void coroutine_fn throttle_group_co_io_limits_intercept(ThrottleGroupMember *tgm
 typedef struct {
     ThrottleGroupMember *tgm;
     ThrottleDirection direction;
+    bool reset_timer_armed;
 } RestartData;
 
 static void coroutine_fn throttle_group_restart_queue_entry(void *opaque)
@@ -403,6 +404,9 @@ static void coroutine_fn throttle_group_restart_queue_entry(void *opaque)
     bool empty_queue;
 
     qemu_mutex_lock(&tg->lock);
+    if (data->reset_timer_armed) {
+        tg->any_timer_armed[direction] = false;
+    }
     empty_queue = !throttle_group_co_restart_queue(tgm, direction);
 
     /* If the request queue was empty then we have to take care of
@@ -419,18 +423,23 @@ static void coroutine_fn throttle_group_restart_queue_entry(void *opaque)
 }
 
 static void throttle_group_restart_queue(ThrottleGroupMember *tgm,
-                                        ThrottleDirection direction)
+                                         ThrottleDirection direction,
+                                         bool reset_timer_armed)
 {
     Coroutine *co;
     RestartData *rd = g_new0(RestartData, 1);
 
     rd->tgm = tgm;
     rd->direction = direction;
+    rd->reset_timer_armed = reset_timer_armed;
 
-    /* This function is called when a timer is fired or when
-     * throttle_group_restart_tgm() is called. Either way, there can
+    /* If reset_timer_armed is set then this means that this function
+     * was called when a timer was fired (either from timer_cb() or
+     * from throttle_group_restart_tgm()). In this case there can
      * be no timer pending on this tgm at this point */
-    assert(!timer_pending(tgm->throttle_timers.timers[direction]));
+    if (reset_timer_armed) {
+        assert(!timer_pending(tgm->throttle_timers.timers[direction]));
+    }
 
     qatomic_inc(&tgm->restart_pending);
 
@@ -444,15 +453,50 @@ void throttle_group_restart_tgm(ThrottleGroupMember *tgm)
 
     if (tgm->throttle_state) {
         for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++) {
-            QEMUTimer *t = tgm->throttle_timers.timers[dir];
+            QEMUTimer *t;
+            ThrottleState *ts = tgm->throttle_state;
+            ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
+            bool reset_timer_armed;
+
+            /*
+             * This function restarts the tgm's queue immediately.
+             * This is used for example for callers to drain all requests.
+             * There are three different scenarios depending on whether
+             * a timer is armed for this tg and which tgm owns the timer.
+             */
+
+            qemu_mutex_lock(&tg->lock);
+
+            t = tgm->throttle_timers.timers[dir];
             if (timer_pending(t)) {
-                /* If there's a pending timer on this tgm, fire it now */
+                /*
+                 * Case 1: this tgm has a pending timer.
+                 * We can fire the timer immediately.
+                 */
                 timer_del(t);
-                timer_cb(tgm, dir);
+                reset_timer_armed = true;
+            } else if (tg->any_timer_armed[dir]) {
+                /*
+                 * Case 2: another tgm has a pending timer.
+                 * In this case we can still restart the queue but we
+                 * have to leave any_timer_armed untouched so the
+                 * other tgm's timer is not disrupted.
+                 */
+                reset_timer_armed = false;
             } else {
-                /* Else run the next request from the queue manually */
-                throttle_group_restart_queue(tgm, dir);
+                /*
+                 * Case 3: there is no timer set for this group.
+                 * Here we can simulate a timer that fires immediately,
+                 * so the queue is restarted but no other thread
+                 * can arm a timer in the meantime.
+                 */
+                tg->any_timer_armed[dir] = true;
+                reset_timer_armed = true;
             }
+
+            qemu_mutex_unlock(&tg->lock);
+
+            throttle_group_restart_queue(tgm, dir, reset_timer_armed);
         }
     }
 }
@@ -499,16 +543,13 @@ void throttle_group_get_config(ThrottleGroupMember *tgm, ThrottleConfig *cfg)
  */
 static void timer_cb(ThrottleGroupMember *tgm, ThrottleDirection direction)
 {
-    ThrottleState *ts = tgm->throttle_state;
-    ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
-
-    /* The timer has just been fired, so we can update the flag */
-    qemu_mutex_lock(&tg->lock);
-    tg->any_timer_armed[direction] = false;
-    qemu_mutex_unlock(&tg->lock);
-
-    /* Run the request that was waiting for this timer */
-    throttle_group_restart_queue(tgm, direction);
+    /*
+     * Run the request that was waiting for this timer.
+     * tg->any_timer_armed needs to be cleared, but we'll do it later
+     * when the queue is restarted in order to prevent another thread
+     * from arming the timer before that.
+     */
+    throttle_group_restart_queue(tgm, direction, true);
 }
 
 static void read_timer_cb(void *opaque)
-- 
2.53.0

[PULL 2/2] block/mirror: fix assertion failure upon duplicate complete for job using 'replaces'

[Hanna Czenczek]

From: Fiona Ebner <f.ebner@proxmox.com>

If s->replace_blocker was already set by an earlier invocation of
mirror_complete(), then there will be an assertion failure when
error_setg() is called for it a second time. The bdrv_op_block_all()
and bdrv_ref() operations should only be done a single time too.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-Id: <20260311145717.668492-2-f.ebner@proxmox.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
---
 block/mirror.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index fa1d975eb9..2fcded9e93 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1276,23 +1276,25 @@ static void mirror_complete(Job *job, Error **errp)
         return;
     }
 
-    /* block all operations on to_replace bs */
-    if (s->replaces) {
-        s->to_replace = bdrv_find_node(s->replaces);
-        if (!s->to_replace) {
-            error_setg(errp, "Node name '%s' not found", s->replaces);
-            return;
+    if (!s->should_complete) {
+        /* block all operations on to_replace bs */
+        if (s->replaces) {
+            s->to_replace = bdrv_find_node(s->replaces);
+            if (!s->to_replace) {
+                error_setg(errp, "Node name '%s' not found", s->replaces);
+                return;
+            }
+
+            /* TODO Translate this into child freeze system. */
+            error_setg(&s->replace_blocker,
+                       "block device is in use by block-job-complete");
+            bdrv_op_block_all(s->to_replace, s->replace_blocker);
+            bdrv_ref(s->to_replace);
         }
 
-        /* TODO Translate this into child freeze system. */
-        error_setg(&s->replace_blocker,
-                   "block device is in use by block-job-complete");
-        bdrv_op_block_all(s->to_replace, s->replace_blocker);
-        bdrv_ref(s->to_replace);
+        s->should_complete = true;
     }
 
-    s->should_complete = true;
-
     /* If the job is paused, it will be re-entered when it is resumed */
     WITH_JOB_LOCK_GUARD() {
         if (!job->paused) {
-- 
2.53.0

Re: [PULL 0/2] Block layer patches

[Peter Maydell]

On Tue, 17 Mar 2026 at 13:45, Hanna Czenczek <hreitz@redhat.com> wrote:
>
> The following changes since commit 559919ce54927d59b215a4665eda7ab6118a48aa:
>
>   Merge tag 'pull-trivial-patches' of https://gitlab.com/mjt0k/qemu into staging (2026-03-16 13:07:33 +0000)
>
> are available in the Git repository at:
>
>   https://gitlab.com/hreitz/qemu.git tags/pull-block-2026-03-17
>
> for you to fetch changes up to 9ac85f4cc7995217db8f736733b990d6addcb036:
>
>   block/mirror: fix assertion failure upon duplicate complete for job using 'replaces' (2026-03-17 12:31:54 +0100)
>
> ----------------------------------------------------------------
> Block layer patches for rc0
>
> - Fix race condition in throttle-group code triggering an assertion
>   failure
> - Fix assertion failure in mirror job when issuing job-complete twice
>



Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/11.0
for any user-visible changes.

-- PMM

Re: [PULL 0/2] Block layer patches

[Michael Tokarev]

On 17.03.2026 16:45, Hanna Czenczek wrote:
[]
> Block layer patches for rc0
> 
> - Fix race condition in throttle-group code triggering an assertion
>    failure
> - Fix assertion failure in mirror job when issuing job-complete twice
> 
> ----------------------------------------------------------------
> Alberto Garcia (1):
>    throttle-group: Fix race condition in throttle_group_restart_queue()
> 
> Fiona Ebner (1):
>    block/mirror: fix assertion failure upon duplicate complete for job
>      using 'replaces'
> 
>   block/mirror.c          | 28 ++++++++-------
>   block/throttle-groups.c | 79 +++++++++++++++++++++++++++++++----------
>   2 files changed, 75 insertions(+), 32 deletions(-)

Is it not qemu-stable material?
The amount of changes is a bit too high though.

I'm picking it up for the currently active stable branches, please
let me know if I shouldn't.

Thanks,

/mjt

Re: [PULL 0/2] Block layer patches

[Hanna Czenczek]

On 18.03.26 21:35, Michael Tokarev wrote:
> On 17.03.2026 16:45, Hanna Czenczek wrote:
> []
>> Block layer patches for rc0
>>
>> - Fix race condition in throttle-group code triggering an assertion
>>    failure
>> - Fix assertion failure in mirror job when issuing job-complete twice
>>
>> ----------------------------------------------------------------
>> Alberto Garcia (1):
>>    throttle-group: Fix race condition in throttle_group_restart_queue()
>>
>> Fiona Ebner (1):
>>    block/mirror: fix assertion failure upon duplicate complete for job
>>      using 'replaces'
>>
>>   block/mirror.c          | 28 ++++++++-------
>>   block/throttle-groups.c | 79 +++++++++++++++++++++++++++++++----------
>>   2 files changed, 75 insertions(+), 32 deletions(-)
>
> Is it not qemu-stable material?
> The amount of changes is a bit too high though.

The mirror one should be simple enough, the throttle-groups one… I think 
it still is good.

> I'm picking it up for the currently active stable branches, please
> let me know if I shouldn't.

Yes, thanks!

Hanna