This patch addresses potential data races involving access to Job fields
in the test-bdrv-drain test.

Fixes: 7253220de4 ("test-bdrv-drain: Test drain vs. block jobs")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2900
Signed-off-by: Vitalii Mordan <mor...@ispras.ru>
---
 include/qemu/job.h           |  2 ++
 job.c                        |  6 ++++++
 tests/unit/test-bdrv-drain.c | 20 ++++++++++----------
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/include/qemu/job.h b/include/qemu/job.h
index 2b873f2576..f27551a9ad 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -520,6 +520,8 @@ bool job_is_internal(Job *job);
  */
 bool job_is_cancelled(Job *job);
 
+bool job_is_paused(Job *job);
+
 /* Same as job_is_cancelled(), but called with job lock held. */
 bool job_is_cancelled_locked(Job *job);
 
diff --git a/job.c b/job.c
index 660ce22c56..d9b2dd8532 100644
--- a/job.c
+++ b/job.c
@@ -251,6 +251,12 @@ bool job_is_cancelled_locked(Job *job)
     return job->force_cancel;
 }
 
+bool job_is_paused(Job *job)
+{
+       JOB_LOCK_GUARD();
+       return job->paused;
+}
+
 bool job_is_cancelled(Job *job)
 {
     JOB_LOCK_GUARD();
diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c
index 7410e6f352..65041c9230 100644
--- a/tests/unit/test-bdrv-drain.c
+++ b/tests/unit/test-bdrv-drain.c
@@ -667,10 +667,10 @@ static int coroutine_fn test_job_run(Job *job, Error 
**errp)
 
     /* We are running the actual job code past the pause point in
      * job_co_entry(). */
-    s->running = true;
+    qatomic_set(&s->running, true);
 
     job_transition_to_ready(&s->common.job);
-    while (!s->should_complete) {
+    while (!qatomic_read(&s->should_complete)) {
         /* Avoid job_sleep_ns() because it marks the job as !busy. We want to
          * emulate some actual activity (probably some I/O) here so that drain
          * has to wait for this activity to stop. */
@@ -685,7 +685,7 @@ static int coroutine_fn test_job_run(Job *job, Error **errp)
 static void test_job_complete(Job *job, Error **errp)
 {
     TestBlockJob *s = container_of(job, TestBlockJob, common.job);
-    s->should_complete = true;
+    qatomic_set(&s->should_complete, true);
 }
 
 BlockJobDriver test_job_driver = {
@@ -791,7 +791,7 @@ static void test_blockjob_common_drain_node(enum drain_type 
drain_type,
         /* job_co_entry() is run in the I/O thread, wait for the actual job
          * code to start (we don't want to catch the job in the pause point in
          * job_co_entry(). */
-        while (!tjob->running) {
+        while (!qatomic_read(&tjob->running)) {
             aio_poll(qemu_get_aio_context(), false);
         }
     }
@@ -825,7 +825,7 @@ static void test_blockjob_common_drain_node(enum drain_type 
drain_type,
          *
          * paused is reset in the I/O thread, wait for it
          */
-        while (job->job.paused) {
+        while (job_is_paused(&job->job)) {
             aio_poll(qemu_get_aio_context(), false);
         }
     }
@@ -858,7 +858,7 @@ static void test_blockjob_common_drain_node(enum drain_type 
drain_type,
          *
          * paused is reset in the I/O thread, wait for it
          */
-        while (job->job.paused) {
+        while (job_is_paused(&job->job)) {
             aio_poll(qemu_get_aio_context(), false);
         }
     }
@@ -1422,7 +1422,7 @@ static int coroutine_fn test_drop_backing_job_run(Job 
*job, Error **errp)
     TestDropBackingBlockJob *s =
         container_of(job, TestDropBackingBlockJob, common.job);
 
-    while (!s->should_complete) {
+    while (!qatomic_read(&s->should_complete)) {
         job_sleep_ns(job, 0);
     }
 
@@ -1541,7 +1541,7 @@ static void test_blockjob_commit_by_drained_end(void)
 
     job_start(&job->common.job);
 
-    job->should_complete = true;
+    qatomic_set(&job->should_complete, true);
     bdrv_drained_begin(bs_child);
     g_assert(!job_has_completed);
     bdrv_drained_end(bs_child);
@@ -1565,7 +1565,7 @@ static int coroutine_fn test_simple_job_run(Job *job, 
Error **errp)
 {
     TestSimpleBlockJob *s = container_of(job, TestSimpleBlockJob, common.job);
 
-    while (!s->should_complete) {
+    while (!qatomic_read(&s->should_complete)) {
         job_sleep_ns(job, 0);
     }
 
@@ -1700,7 +1700,7 @@ static void test_drop_intermediate_poll(void)
     job->did_complete = &job_has_completed;
 
     job_start(&job->common.job);
-    job->should_complete = true;
+    qatomic_set(&job->should_complete, true);
 
     g_assert(!job_has_completed);
     ret = bdrv_drop_intermediate(chain[1], chain[0], NULL, false);
-- 
2.34.1


Reply via email to