From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
To: qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, Fam Zheng <fam@euphon.net>,
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
Wen Congyang <wencongyang2@huawei.com>,
Xie Changlong <xiechanglong.d@gmail.com>,
Emanuele Giuseppe Esposito <eesposit@redhat.com>,
Markus Armbruster <armbru@redhat.com>,
qemu-devel@nongnu.org, Hanna Reitz <hreitz@redhat.com>,
Stefan Hajnoczi <stefanha@redhat.com>,
Paolo Bonzini <pbonzini@redhat.com>, John Snow <jsnow@redhat.com>
Subject: [PATCH v3 12/16] jobs: use job locks and helpers also in the unit tests
Date: Wed, 5 Jan 2022 09:02:04 -0500 [thread overview]
Message-ID: <20220105140208.365608-13-eesposit@redhat.com> (raw)
In-Reply-To: <20220105140208.365608-1-eesposit@redhat.com>
Add missing job synchronization in the unit tests, with
both explicit locks and helpers.
Note: at this stage, job_{lock/unlock} and job lock guard macros
are *nop*.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
tests/unit/test-bdrv-drain.c | 40 +++++++++++-----------
tests/unit/test-block-iothread.c | 4 +++
tests/unit/test-blockjob-txn.c | 10 ++++++
tests/unit/test-blockjob.c | 57 +++++++++++++++++++++-----------
4 files changed, 72 insertions(+), 39 deletions(-)
diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c
index 3f344a0d0d..c03560e63d 100644
--- a/tests/unit/test-bdrv-drain.c
+++ b/tests/unit/test-bdrv-drain.c
@@ -941,61 +941,63 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type,
}
}
- g_assert_cmpint(job->job.pause_count, ==, 0);
- g_assert_false(job->job.paused);
+ g_assert_cmpint(job_get_pause_count(&job->job), ==, 0);
+ g_assert_false(job_get_paused(&job->job));
g_assert_true(tjob->running);
- g_assert_true(job->job.busy); /* We're in qemu_co_sleep_ns() */
+ g_assert_true(job_get_busy(&job->job)); /* We're in qemu_co_sleep_ns() */
do_drain_begin_unlocked(drain_type, drain_bs);
if (drain_type == BDRV_DRAIN_ALL) {
/* bdrv_drain_all() drains both src and target */
- g_assert_cmpint(job->job.pause_count, ==, 2);
+ g_assert_cmpint(job_get_pause_count(&job->job), ==, 2);
} else {
- g_assert_cmpint(job->job.pause_count, ==, 1);
+ g_assert_cmpint(job_get_pause_count(&job->job), ==, 1);
}
- g_assert_true(job->job.paused);
- g_assert_false(job->job.busy); /* The job is paused */
+ g_assert_true(job_get_paused(&job->job));
+ g_assert_false(job_get_busy(&job->job)); /* The job is paused */
do_drain_end_unlocked(drain_type, drain_bs);
if (use_iothread) {
/* paused is reset in the I/O thread, wait for it */
- while (job->job.paused) {
+ while (job_get_paused(&job->job)) {
aio_poll(qemu_get_aio_context(), false);
}
}
- g_assert_cmpint(job->job.pause_count, ==, 0);
- g_assert_false(job->job.paused);
- g_assert_true(job->job.busy); /* We're in qemu_co_sleep_ns() */
+ g_assert_cmpint(job_get_pause_count(&job->job), ==, 0);
+ g_assert_false(job_get_paused(&job->job));
+ g_assert_true(job_get_busy(&job->job)); /* We're in qemu_co_sleep_ns() */
do_drain_begin_unlocked(drain_type, target);
if (drain_type == BDRV_DRAIN_ALL) {
/* bdrv_drain_all() drains both src and target */
- g_assert_cmpint(job->job.pause_count, ==, 2);
+ g_assert_cmpint(job_get_pause_count(&job->job), ==, 2);
} else {
- g_assert_cmpint(job->job.pause_count, ==, 1);
+ g_assert_cmpint(job_get_pause_count(&job->job), ==, 1);
}
- g_assert_true(job->job.paused);
- g_assert_false(job->job.busy); /* The job is paused */
+ g_assert_true(job_get_paused(&job->job));
+ g_assert_false(job_get_busy(&job->job)); /* The job is paused */
do_drain_end_unlocked(drain_type, target);
if (use_iothread) {
/* paused is reset in the I/O thread, wait for it */
- while (job->job.paused) {
+ while (job_get_paused(&job->job)) {
aio_poll(qemu_get_aio_context(), false);
}
}
- g_assert_cmpint(job->job.pause_count, ==, 0);
- g_assert_false(job->job.paused);
- g_assert_true(job->job.busy); /* We're in qemu_co_sleep_ns() */
+ g_assert_cmpint(job_get_pause_count(&job->job), ==, 0);
+ g_assert_false(job_get_paused(&job->job));
+ g_assert_true(job_get_busy(&job->job)); /* We're in qemu_co_sleep_ns() */
aio_context_acquire(ctx);
+ job_lock();
ret = job_complete_sync_locked(&job->job, &error_abort);
+ job_unlock();
g_assert_cmpint(ret, ==, (result == TEST_JOB_SUCCESS ? 0 : -EIO));
if (use_iothread) {
diff --git a/tests/unit/test-block-iothread.c b/tests/unit/test-block-iothread.c
index 7e1b521d61..b9309beec2 100644
--- a/tests/unit/test-block-iothread.c
+++ b/tests/unit/test-block-iothread.c
@@ -456,7 +456,9 @@ static void test_attach_blockjob(void)
}
aio_context_acquire(ctx);
+ job_lock();
job_complete_sync_locked(&tjob->common.job, &error_abort);
+ job_unlock();
blk_set_aio_context(blk, qemu_get_aio_context(), &error_abort);
aio_context_release(ctx);
@@ -630,7 +632,9 @@ static void test_propagate_mirror(void)
BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT,
false, "filter_node", MIRROR_COPY_MODE_BACKGROUND,
&error_abort);
+ job_lock();
job = job_get_locked("job0");
+ job_unlock();
filter = bdrv_find_node("filter_node");
/* Change the AioContext of src */
diff --git a/tests/unit/test-blockjob-txn.c b/tests/unit/test-blockjob-txn.c
index 5396fcef10..bd69076300 100644
--- a/tests/unit/test-blockjob-txn.c
+++ b/tests/unit/test-blockjob-txn.c
@@ -124,16 +124,20 @@ static void test_single_job(int expected)
job = test_block_job_start(1, true, expected, &result, txn);
job_start(&job->job);
+ job_lock();
if (expected == -ECANCELED) {
job_cancel_locked(&job->job, false);
}
+ job_unlock();
while (result == -EINPROGRESS) {
aio_poll(qemu_get_aio_context(), true);
}
g_assert_cmpint(result, ==, expected);
+ job_lock();
job_txn_unref_locked(txn);
+ job_unlock();
}
static void test_single_job_success(void)
@@ -168,6 +172,7 @@ static void test_pair_jobs(int expected1, int expected2)
/* Release our reference now to trigger as many nice
* use-after-free bugs as possible.
*/
+ job_lock();
job_txn_unref_locked(txn);
if (expected1 == -ECANCELED) {
@@ -176,6 +181,7 @@ static void test_pair_jobs(int expected1, int expected2)
if (expected2 == -ECANCELED) {
job_cancel_locked(&job2->job, false);
}
+ job_unlock();
while (result1 == -EINPROGRESS || result2 == -EINPROGRESS) {
aio_poll(qemu_get_aio_context(), true);
@@ -227,7 +233,9 @@ static void test_pair_jobs_fail_cancel_race(void)
job_start(&job1->job);
job_start(&job2->job);
+ job_lock();
job_cancel_locked(&job1->job, false);
+ job_unlock();
/* Now make job2 finish before the main loop kicks jobs. This simulates
* the race between a pending kick and another job completing.
@@ -242,7 +250,9 @@ static void test_pair_jobs_fail_cancel_race(void)
g_assert_cmpint(result1, ==, -ECANCELED);
g_assert_cmpint(result2, ==, -ECANCELED);
+ job_lock();
job_txn_unref_locked(txn);
+ job_unlock();
}
int main(int argc, char **argv)
diff --git a/tests/unit/test-blockjob.c b/tests/unit/test-blockjob.c
index 2beed3623e..ec9128dbb5 100644
--- a/tests/unit/test-blockjob.c
+++ b/tests/unit/test-blockjob.c
@@ -211,8 +211,11 @@ static CancelJob *create_common(Job **pjob)
bjob = mk_job(blk, "Steve", &test_cancel_driver, true,
JOB_MANUAL_FINALIZE | JOB_MANUAL_DISMISS);
job = &bjob->job;
+ job_lock();
job_ref_locked(job);
assert(job->status == JOB_STATUS_CREATED);
+ job_unlock();
+
s = container_of(bjob, CancelJob, common);
s->blk = blk;
@@ -230,6 +233,7 @@ static void cancel_common(CancelJob *s)
ctx = job->job.aio_context;
aio_context_acquire(ctx);
+ job_lock();
job_cancel_sync_locked(&job->job, true);
if (sts != JOB_STATUS_CREATED && sts != JOB_STATUS_CONCLUDED) {
Job *dummy = &job->job;
@@ -237,6 +241,7 @@ static void cancel_common(CancelJob *s)
}
assert(job->job.status == JOB_STATUS_NULL);
job_unref_locked(&job->job);
+ job_unlock();
destroy_blk(blk);
aio_context_release(ctx);
@@ -259,7 +264,7 @@ static void test_cancel_running(void)
s = create_common(&job);
job_start(job);
- assert(job->status == JOB_STATUS_RUNNING);
+ assert(job_get_status(job) == JOB_STATUS_RUNNING);
cancel_common(s);
}
@@ -272,11 +277,13 @@ static void test_cancel_paused(void)
s = create_common(&job);
job_start(job);
- assert(job->status == JOB_STATUS_RUNNING);
+ assert(job_get_status(job) == JOB_STATUS_RUNNING);
+ job_lock();
job_user_pause_locked(job, &error_abort);
+ job_unlock();
job_enter(job);
- assert(job->status == JOB_STATUS_PAUSED);
+ assert(job_get_status(job) == JOB_STATUS_PAUSED);
cancel_common(s);
}
@@ -289,11 +296,11 @@ static void test_cancel_ready(void)
s = create_common(&job);
job_start(job);
- assert(job->status == JOB_STATUS_RUNNING);
+ assert(job_get_status(job) == JOB_STATUS_RUNNING);
s->should_converge = true;
job_enter(job);
- assert(job->status == JOB_STATUS_READY);
+ assert(job_get_status(job) == JOB_STATUS_READY);
cancel_common(s);
}
@@ -306,15 +313,17 @@ static void test_cancel_standby(void)
s = create_common(&job);
job_start(job);
- assert(job->status == JOB_STATUS_RUNNING);
+ assert(job_get_status(job) == JOB_STATUS_RUNNING);
s->should_converge = true;
job_enter(job);
- assert(job->status == JOB_STATUS_READY);
+ assert(job_get_status(job) == JOB_STATUS_READY);
+ job_lock();
job_user_pause_locked(job, &error_abort);
+ job_unlock();
job_enter(job);
- assert(job->status == JOB_STATUS_STANDBY);
+ assert(job_get_status(job) == JOB_STATUS_STANDBY);
cancel_common(s);
}
@@ -327,20 +336,22 @@ static void test_cancel_pending(void)
s = create_common(&job);
job_start(job);
- assert(job->status == JOB_STATUS_RUNNING);
+ assert(job_get_status(job) == JOB_STATUS_RUNNING);
s->should_converge = true;
job_enter(job);
- assert(job->status == JOB_STATUS_READY);
+ assert(job_get_status(job) == JOB_STATUS_READY);
+ job_lock();
job_complete_locked(job, &error_abort);
+ job_unlock();
job_enter(job);
while (!job->deferred_to_main_loop) {
aio_poll(qemu_get_aio_context(), true);
}
- assert(job->status == JOB_STATUS_READY);
+ assert(job_get_status(job) == JOB_STATUS_READY);
aio_poll(qemu_get_aio_context(), true);
- assert(job->status == JOB_STATUS_PENDING);
+ assert(job_get_status(job) == JOB_STATUS_PENDING);
cancel_common(s);
}
@@ -353,25 +364,29 @@ static void test_cancel_concluded(void)
s = create_common(&job);
job_start(job);
- assert(job->status == JOB_STATUS_RUNNING);
+ assert(job_get_status(job) == JOB_STATUS_RUNNING);
s->should_converge = true;
job_enter(job);
- assert(job->status == JOB_STATUS_READY);
+ assert(job_get_status(job) == JOB_STATUS_READY);
+ job_lock();
job_complete_locked(job, &error_abort);
+ job_unlock();
job_enter(job);
while (!job->deferred_to_main_loop) {
aio_poll(qemu_get_aio_context(), true);
}
- assert(job->status == JOB_STATUS_READY);
+ assert(job_get_status(job) == JOB_STATUS_READY);
aio_poll(qemu_get_aio_context(), true);
- assert(job->status == JOB_STATUS_PENDING);
+ assert(job_get_status(job) == JOB_STATUS_PENDING);
aio_context_acquire(job->aio_context);
+ job_lock();
job_finalize_locked(job, &error_abort);
+ job_unlock();
aio_context_release(job->aio_context);
- assert(job->status == JOB_STATUS_CONCLUDED);
+ assert(job_get_status(job) == JOB_STATUS_CONCLUDED);
cancel_common(s);
}
@@ -459,22 +474,23 @@ static void test_complete_in_standby(void)
bjob = mk_job(blk, "job", &test_yielding_driver, true,
JOB_MANUAL_FINALIZE | JOB_MANUAL_DISMISS);
job = &bjob->job;
- assert(job->status == JOB_STATUS_CREATED);
+ assert(job_get_status(job) == JOB_STATUS_CREATED);
/* Wait for the job to become READY */
job_start(job);
aio_context_acquire(ctx);
- AIO_WAIT_WHILE(ctx, job->status != JOB_STATUS_READY);
+ AIO_WAIT_WHILE(ctx, job_get_status(job) != JOB_STATUS_READY);
aio_context_release(ctx);
/* Begin the drained section, pausing the job */
bdrv_drain_all_begin();
- assert(job->status == JOB_STATUS_STANDBY);
+ assert(job_get_status(job) == JOB_STATUS_STANDBY);
/* Lock the IO thread to prevent the job from being run */
aio_context_acquire(ctx);
/* This will schedule the job to resume it */
bdrv_drain_all_end();
+ job_lock();
/* But the job cannot run, so it will remain on standby */
assert(job->status == JOB_STATUS_STANDBY);
@@ -489,6 +505,7 @@ static void test_complete_in_standby(void)
assert(job->status == JOB_STATUS_CONCLUDED);
job_dismiss_locked(&job, &error_abort);
+ job_unlock();
destroy_blk(blk);
aio_context_release(ctx);
--
2.31.1
next prev parent reply other threads:[~2022-01-05 14:36 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-05 14:01 [PATCH v3 00/16] job: replace AioContext lock with job_mutex Emanuele Giuseppe Esposito
2022-01-05 14:01 ` [PATCH v3 01/16] job.c: make job_mutex and job_lock/unlock() public Emanuele Giuseppe Esposito
2022-01-19 9:56 ` Paolo Bonzini
2022-01-19 11:13 ` Paolo Bonzini
2022-01-05 14:01 ` [PATCH v3 02/16] job.h: categorize fields in struct Job Emanuele Giuseppe Esposito
2022-01-19 9:57 ` Paolo Bonzini
2022-01-05 14:01 ` [PATCH v3 03/16] job.h: define locked functions Emanuele Giuseppe Esposito
2022-01-19 10:44 ` Paolo Bonzini
2022-01-21 15:25 ` Emanuele Giuseppe Esposito
2022-01-21 16:04 ` Vladimir Sementsov-Ogievskiy
2022-01-24 14:26 ` Paolo Bonzini
2022-01-26 15:58 ` Emanuele Giuseppe Esposito
2022-01-05 14:01 ` [PATCH v3 04/16] job.h: define unlocked functions Emanuele Giuseppe Esposito
2022-01-05 14:01 ` [PATCH v3 05/16] block/mirror.c: use of job helpers in drivers to avoid TOC/TOU Emanuele Giuseppe Esposito
2022-01-19 11:06 ` Paolo Bonzini
2022-01-05 14:01 ` [PATCH v3 06/16] job.c: make job_event_* functions static Emanuele Giuseppe Esposito
2022-01-05 14:01 ` [PATCH v3 07/16] job.c: move inner aiocontext lock in callbacks Emanuele Giuseppe Esposito
2022-01-05 14:02 ` [PATCH v3 08/16] aio-wait.h: introduce AIO_WAIT_WHILE_UNLOCKED Emanuele Giuseppe Esposito
2022-01-05 14:02 ` [PATCH v3 09/16] jobs: remove aiocontext locks since the functions are under BQL Emanuele Giuseppe Esposito
2022-01-19 11:09 ` Paolo Bonzini
2022-01-26 16:18 ` Emanuele Giuseppe Esposito
2022-01-05 14:02 ` [PATCH v3 10/16] jobs: protect jobs with job_lock/unlock Emanuele Giuseppe Esposito
2022-01-19 10:50 ` Paolo Bonzini
2022-01-05 14:02 ` [PATCH v3 11/16] jobs: document all static functions and add _locked() suffix Emanuele Giuseppe Esposito
2022-01-05 14:02 ` Emanuele Giuseppe Esposito [this message]
2022-01-05 14:02 ` [PATCH v3 13/16] jobs: add job lock in find_* functions Emanuele Giuseppe Esposito
2022-01-05 14:02 ` [PATCH v3 14/16] job.c: use job_get_aio_context() Emanuele Giuseppe Esposito
2022-01-19 10:31 ` Paolo Bonzini
2022-01-21 12:33 ` Emanuele Giuseppe Esposito
2022-01-21 17:43 ` Emanuele Giuseppe Esposito
2022-01-21 15:18 ` Emanuele Giuseppe Esposito
2022-01-24 14:22 ` Paolo Bonzini
2022-01-26 15:58 ` Emanuele Giuseppe Esposito
2022-01-05 14:02 ` [PATCH v3 15/16] job.c: enable job lock/unlock and remove Aiocontext locks Emanuele Giuseppe Esposito
2022-01-19 10:35 ` Paolo Bonzini
2022-01-05 14:02 ` [PATCH v3 16/16] block_job_query: remove atomic read Emanuele Giuseppe Esposito
2022-01-19 10:34 ` Paolo Bonzini
2022-01-19 11:15 ` [PATCH v3 00/16] job: replace AioContext lock with job_mutex Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220105140208.365608-13-eesposit@redhat.com \
--to=eesposit@redhat.com \
--cc=armbru@redhat.com \
--cc=fam@euphon.net \
--cc=hreitz@redhat.com \
--cc=jsnow@redhat.com \
--cc=kwolf@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
--cc=vsementsov@virtuozzo.com \
--cc=wencongyang2@huawei.com \
--cc=xiechanglong.d@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.