[PULL 1/6] job: keep job paused across overlapping pause requests

From: "Denis V. Lunev" <[email protected]>

job_pause_point_locked() sets job->paused before yielding and clears it
unconditionally on wake, before re-checking whether a pause is still
pending. job_pause() re-enters a parked job only while it is not yet
paused, so the wake that resumes one comes from a drain *ending*
(job_resume() -> job_enter_cond()). If the next drain begins before that
wake runs, the woken coroutine clears job->paused while pause_count is
already > 0 again:

  AioContext change (BQL thread)     job coroutine (iothread)
  -----------------------------      ------------------------
                                     parked in job_pause_point():
                                       paused=1, pause_count=1, yielded
  drain ends -> job_resume():
    pause_count = 0
    job_enter_cond(): queue wake ..>   (wake pending)
  bdrv_try_change_aio_context():
    bdrv_drain_all_begin():
      job_pause() per node
      pause_count = N (> 0)
                                     wake runs, leaves job_do_yield():
                                       paused = 0  (pause_count == N)
    tran_commit -> job_set_aio_context():
      assert(paused || completed) --> abort: paused == 0

bdrv_try_change_aio_context() drains precisely to quiesce the job before
changing its AioContext, but that brief paused==0 window trips the
assertion. It is guest-triggerable: a virtio-blk reset
(virtio_blk_stop_ioeventfd() -> blk_set_aio_context()) racing a running
mirror/blockCopy job hits it, as do x-blockdev-set-iothread, blockdev
hot-plug/unplug and job completion.

Keep job->paused set while a pause is still pending: loop the yield
until job_should_pause_locked() is false (or the job is cancelled), and
only then clear job->paused. Drained-state consumers then never observe
a pending-pause job as unpaused.

Signed-off-by: Denis V. Lunev <[email protected]>
Message-ID: <[email protected]>
Reviewed-by: Vladimir Sementsov-Ogievskiy <[email protected]>
Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
---
 job.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/job.c b/job.c
index e7479084726..d7220aaf30c 100644
--- a/job.c
+++ b/job.c
@@ -629,7 +629,14 @@ static void coroutine_fn job_pause_point_locked(Job *job)
                                     ? JOB_STATUS_STANDBY
                                     : JOB_STATUS_PAUSED);
         job->paused = true;
-        job_do_yield_locked(job, -1);
+        /*
+         * Stay paused across back-to-back pause requests: a transient
+         * paused == false while pause_count > 0 would be observed as
+         * "not paused" by job_set_aio_context() and other drain consumers.
+         */
+        do {
+            job_do_yield_locked(job, -1);
+        } while (job_should_pause_locked(job) && !job_is_cancelled_locked(job));
         job->paused = false;
         job_state_transition_locked(job, status);
     }
-- 
2.43.0