[PATCH 0/2] job: don't drop job->paused while a pause is pending

Denis V. Lunev posted 2 patches 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260623152406.1180235-1-den@openvz.org
Maintainers: John Snow <jsnow@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
job.c                      |   9 +++-
tests/unit/test-blockjob.c | 100 +++++++++++++++++++++++++++++++++++++
2 files changed, 108 insertions(+), 1 deletion(-)
[PATCH 0/2] job: don't drop job->paused while a pause is pending
Posted by Denis V. Lunev 1 month ago
A guest can abort QEMU with

  qemu-system-x86_64: ../job.c:379: job_set_aio_context: Assertion
  `job->paused || job_is_completed_locked(job)' failed.

when it touches a virtio-blk device whose BlockBackend lives in an
iothread while a block job (drive-mirror / blockCopy) runs on the same
node. It is guest-triggerable and reproduces on current master. It was
reported against the qemu-kvm 10.0 downstream, hit by the live-migration
validation suite when a blockdev-mirror (libvirt blockCopy) ran while
the guest reset its virtio-blk device during boot.

Under overlapping drains job_pause_point_locked() clears job->paused on
wake while a pause is still pending (pause_count > 0), and the
concurrent AioContext change reads paused == false and asserts. Patch 1
carries the full interleaving and the fix; being in the job layer it
also covers the other bdrv_try_change_aio_context() callers
(x-blockdev-set-iothread, blockdev hot-plug/unplug, job completion), not
just the virtio-blk reset path.

Patch 2 is a deterministic regression test in tests/unit/test-blockjob
that fails without patch 1 and passes with it. It is illustrative only:
it pins the exact invariant patch 1 restores, but the fix stands on its
own and the test may be dropped freely.

There is no iotests reproducer. iotests run under qtest, which has no
vCPU, so the only way to change a running job's AioContext from outside
its iothread is x-blockdev-set-iothread, which runs in the main loop
under the BQL - the same lock the pause handshake takes - and the
cross-thread window never opens on an idle qtest VM. Several designs
(back-to-back set-iothread, pause/resume churn, two iothreads, a
latency-injecting null-co feeding a throttled mirror) all pass on the
unfixed build; a timing test that passes on broken code is worse than
none.

To reproduce on master: in the guest, tightly unbind/rebind a virtio-blk
PCI function backed by an iothread; on the host, keep a throttled
blockdev-mirror alive on that disk's node. The assertion fires within
seconds; with patch 1 it runs indefinitely.

checkpatch is clean; test-blockjob, test-bdrv-drain and test-block-iothread
pass.

Denis V. Lunev (2):
  job: keep job paused across overlapping pause requests
  tests/unit/test-blockjob: cover keeping a job paused while a pause is
    pending

 job.c                      |   9 +++-
 tests/unit/test-blockjob.c | 100 +++++++++++++++++++++++++++++++++++++
 2 files changed, 108 insertions(+), 1 deletion(-)

-- 
2.53.0
Re: [PATCH 0/2] job: don't drop job->paused while a pause is pending
Posted by Vladimir Sementsov-Ogievskiy 2 weeks, 4 days ago
On 23.06.26 18:24, Denis V. Lunev wrote:
> A guest can abort QEMU with
> 
>    qemu-system-x86_64: ../job.c:379: job_set_aio_context: Assertion
>    `job->paused || job_is_completed_locked(job)' failed.
> 
> when it touches a virtio-blk device whose BlockBackend lives in an
> iothread while a block job (drive-mirror / blockCopy) runs on the same
> node. It is guest-triggerable and reproduces on current master. It was
> reported against the qemu-kvm 10.0 downstream, hit by the live-migration
> validation suite when a blockdev-mirror (libvirt blockCopy) ran while
> the guest reset its virtio-blk device during boot.
> 
> Under overlapping drains job_pause_point_locked() clears job->paused on
> wake while a pause is still pending (pause_count > 0), and the
> concurrent AioContext change reads paused == false and asserts. Patch 1
> carries the full interleaving and the fix; being in the job layer it
> also covers the other bdrv_try_change_aio_context() callers
> (x-blockdev-set-iothread, blockdev hot-plug/unplug, job completion), not
> just the virtio-blk reset path.
> 
> Patch 2 is a deterministic regression test in tests/unit/test-blockjob
> that fails without patch 1 and passes with it. It is illustrative only:
> it pins the exact invariant patch 1 restores, but the fix stands on its
> own and the test may be dropped freely.
> 
> There is no iotests reproducer. iotests run under qtest, which has no
> vCPU, so the only way to change a running job's AioContext from outside
> its iothread is x-blockdev-set-iothread, which runs in the main loop
> under the BQL - the same lock the pause handshake takes - and the
> cross-thread window never opens on an idle qtest VM. Several designs
> (back-to-back set-iothread, pause/resume churn, two iothreads, a
> latency-injecting null-co feeding a throttled mirror) all pass on the
> unfixed build; a timing test that passes on broken code is worse than
> none.
> 
> To reproduce on master: in the guest, tightly unbind/rebind a virtio-blk
> PCI function backed by an iothread; on the host, keep a throttled
> blockdev-mirror alive on that disk's node. The assertion fires within
> seconds; with patch 1 it runs indefinitely.
> 
> checkpatch is clean; test-blockjob, test-bdrv-drain and test-block-iothread
> pass.
> 
> Denis V. Lunev (2):
>    job: keep job paused across overlapping pause requests
>    tests/unit/test-blockjob: cover keeping a job paused while a pause is
>      pending
> 
>   job.c                      |   9 +++-
>   tests/unit/test-blockjob.c | 100 +++++++++++++++++++++++++++++++++++++
>   2 files changed, 108 insertions(+), 1 deletion(-)
> 

Thanks, applied to my block branch.

-- 
Best regards,
Vladimir