aboutsummaryrefslogtreecommitdiff
path: root/tests/unit
diff options
context:
space:
mode:
authorStefan Hajnoczi <stefanha@redhat.com>2023-12-05 13:20:03 -0500
committerKevin Wolf <kwolf@redhat.com>2023-12-21 22:49:27 +0100
commitb49f4755c7fa35ea6e17e5b52c1cdaef6b4aa21c (patch)
tree42f28762584421da298ba15cb21d027e002476bd /tests/unit
parent6bc30f19498547fac9cef98316a65cf6c1f14205 (diff)
block: remove AioContext locking
This is the big patch that removes aio_context_acquire()/aio_context_release() from the block layer and affected block layer users. There isn't a clean way to split this patch and the reviewers are likely the same group of people, so I decided to do it in one patch. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Paul Durrant <paul@xen.org> Message-ID: <20231205182011.1976568-7-stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Diffstat (limited to 'tests/unit')
-rw-r--r--tests/unit/test-bdrv-drain.c51
-rw-r--r--tests/unit/test-bdrv-graph-mod.c6
-rw-r--r--tests/unit/test-block-iothread.c31
-rw-r--r--tests/unit/test-blockjob.c137
-rw-r--r--tests/unit/test-replication.c11
5 files changed, 2 insertions, 234 deletions
diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c
index d9754dfebc..17830a69c1 100644
--- a/tests/unit/test-bdrv-drain.c
+++ b/tests/unit/test-bdrv-drain.c
@@ -179,13 +179,7 @@ static void do_drain_end(enum drain_type drain_type, BlockDriverState *bs)
static void do_drain_begin_unlocked(enum drain_type drain_type, BlockDriverState *bs)
{
- if (drain_type != BDRV_DRAIN_ALL) {
- aio_context_acquire(bdrv_get_aio_context(bs));
- }
do_drain_begin(drain_type, bs);
- if (drain_type != BDRV_DRAIN_ALL) {
- aio_context_release(bdrv_get_aio_context(bs));
- }
}
static BlockBackend * no_coroutine_fn test_setup(void)
@@ -209,13 +203,7 @@ static BlockBackend * no_coroutine_fn test_setup(void)
static void do_drain_end_unlocked(enum drain_type drain_type, BlockDriverState *bs)
{
- if (drain_type != BDRV_DRAIN_ALL) {
- aio_context_acquire(bdrv_get_aio_context(bs));
- }
do_drain_end(drain_type, bs);
- if (drain_type != BDRV_DRAIN_ALL) {
- aio_context_release(bdrv_get_aio_context(bs));
- }
}
/*
@@ -520,12 +508,8 @@ static void test_iothread_main_thread_bh(void *opaque)
{
struct test_iothread_data *data = opaque;
- /* Test that the AioContext is not yet locked in a random BH that is
- * executed during drain, otherwise this would deadlock. */
- aio_context_acquire(bdrv_get_aio_context(data->bs));
bdrv_flush(data->bs);
bdrv_dec_in_flight(data->bs); /* incremented by test_iothread_common() */
- aio_context_release(bdrv_get_aio_context(data->bs));
}
/*
@@ -567,7 +551,6 @@ static void test_iothread_common(enum drain_type drain_type, int drain_thread)
blk_set_disable_request_queuing(blk, true);
blk_set_aio_context(blk, ctx_a, &error_abort);
- aio_context_acquire(ctx_a);
s->bh_indirection_ctx = ctx_b;
@@ -582,8 +565,6 @@ static void test_iothread_common(enum drain_type drain_type, int drain_thread)
g_assert(acb != NULL);
g_assert_cmpint(aio_ret, ==, -EINPROGRESS);
- aio_context_release(ctx_a);
-
data = (struct test_iothread_data) {
.bs = bs,
.drain_type = drain_type,
@@ -592,10 +573,6 @@ static void test_iothread_common(enum drain_type drain_type, int drain_thread)
switch (drain_thread) {
case 0:
- if (drain_type != BDRV_DRAIN_ALL) {
- aio_context_acquire(ctx_a);
- }
-
/*
* Increment in_flight so that do_drain_begin() waits for
* test_iothread_main_thread_bh(). This prevents the race between
@@ -613,20 +590,10 @@ static void test_iothread_common(enum drain_type drain_type, int drain_thread)
do_drain_begin(drain_type, bs);
g_assert_cmpint(bs->in_flight, ==, 0);
- if (drain_type != BDRV_DRAIN_ALL) {
- aio_context_release(ctx_a);
- }
qemu_event_wait(&done_event);
- if (drain_type != BDRV_DRAIN_ALL) {
- aio_context_acquire(ctx_a);
- }
g_assert_cmpint(aio_ret, ==, 0);
do_drain_end(drain_type, bs);
-
- if (drain_type != BDRV_DRAIN_ALL) {
- aio_context_release(ctx_a);
- }
break;
case 1:
co = qemu_coroutine_create(test_iothread_drain_co_entry, &data);
@@ -637,9 +604,7 @@ static void test_iothread_common(enum drain_type drain_type, int drain_thread)
g_assert_not_reached();
}
- aio_context_acquire(ctx_a);
blk_set_aio_context(blk, qemu_get_aio_context(), &error_abort);
- aio_context_release(ctx_a);
bdrv_unref(bs);
blk_unref(blk);
@@ -757,7 +722,6 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type,
BlockJob *job;
TestBlockJob *tjob;
IOThread *iothread = NULL;
- AioContext *ctx;
int ret;
src = bdrv_new_open_driver(&bdrv_test, "source", BDRV_O_RDWR,
@@ -787,11 +751,11 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type,
}
if (use_iothread) {
+ AioContext *ctx;
+
iothread = iothread_new();
ctx = iothread_get_aio_context(iothread);
blk_set_aio_context(blk_src, ctx, &error_abort);
- } else {
- ctx = qemu_get_aio_context();
}
target = bdrv_new_open_driver(&bdrv_test, "target", BDRV_O_RDWR,
@@ -800,7 +764,6 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type,
blk_insert_bs(blk_target, target, &error_abort);
blk_set_allow_aio_context_change(blk_target, true);
- aio_context_acquire(ctx);
tjob = block_job_create("job0", &test_job_driver, NULL, src,
0, BLK_PERM_ALL,
0, 0, NULL, NULL, &error_abort);
@@ -821,7 +784,6 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type,
tjob->prepare_ret = -EIO;
break;
}
- aio_context_release(ctx);
job_start(&job->job);
@@ -912,12 +874,10 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type,
}
g_assert_cmpint(ret, ==, (result == TEST_JOB_SUCCESS ? 0 : -EIO));
- aio_context_acquire(ctx);
if (use_iothread) {
blk_set_aio_context(blk_src, qemu_get_aio_context(), &error_abort);
assert(blk_get_aio_context(blk_target) == qemu_get_aio_context());
}
- aio_context_release(ctx);
blk_unref(blk_src);
blk_unref(blk_target);
@@ -1401,9 +1361,7 @@ static void test_append_to_drained(void)
g_assert_cmpint(base_s->drain_count, ==, 1);
g_assert_cmpint(base->in_flight, ==, 0);
- aio_context_acquire(qemu_get_aio_context());
bdrv_append(overlay, base, &error_abort);
- aio_context_release(qemu_get_aio_context());
g_assert_cmpint(base->in_flight, ==, 0);
g_assert_cmpint(overlay->in_flight, ==, 0);
@@ -1438,16 +1396,11 @@ static void test_set_aio_context(void)
bdrv_drained_begin(bs);
bdrv_try_change_aio_context(bs, ctx_a, NULL, &error_abort);
-
- aio_context_acquire(ctx_a);
bdrv_drained_end(bs);
bdrv_drained_begin(bs);
bdrv_try_change_aio_context(bs, ctx_b, NULL, &error_abort);
- aio_context_release(ctx_a);
- aio_context_acquire(ctx_b);
bdrv_try_change_aio_context(bs, qemu_get_aio_context(), NULL, &error_abort);
- aio_context_release(ctx_b);
bdrv_drained_end(bs);
bdrv_unref(bs);
diff --git a/tests/unit/test-bdrv-graph-mod.c b/tests/unit/test-bdrv-graph-mod.c
index 8ee6ef38d8..cafc023db4 100644
--- a/tests/unit/test-bdrv-graph-mod.c
+++ b/tests/unit/test-bdrv-graph-mod.c
@@ -142,10 +142,8 @@ static void test_update_perm_tree(void)
BDRV_CHILD_DATA, &error_abort);
bdrv_graph_wrunlock();
- aio_context_acquire(qemu_get_aio_context());
ret = bdrv_append(filter, bs, NULL);
g_assert_cmpint(ret, <, 0);
- aio_context_release(qemu_get_aio_context());
bdrv_unref(filter);
blk_unref(root);
@@ -211,9 +209,7 @@ static void test_should_update_child(void)
bdrv_attach_child(filter, target, "target", &child_of_bds,
BDRV_CHILD_DATA, &error_abort);
bdrv_graph_wrunlock();
- aio_context_acquire(qemu_get_aio_context());
bdrv_append(filter, bs, &error_abort);
- aio_context_release(qemu_get_aio_context());
bdrv_graph_rdlock_main_loop();
g_assert(target->backing->bs == bs);
@@ -440,9 +436,7 @@ static void test_append_greedy_filter(void)
&error_abort);
bdrv_graph_wrunlock();
- aio_context_acquire(qemu_get_aio_context());
bdrv_append(fl, base, &error_abort);
- aio_context_release(qemu_get_aio_context());
bdrv_unref(fl);
bdrv_unref(top);
}
diff --git a/tests/unit/test-block-iothread.c b/tests/unit/test-block-iothread.c
index 9b15d2768c..3766d5de6b 100644
--- a/tests/unit/test-block-iothread.c
+++ b/tests/unit/test-block-iothread.c
@@ -483,7 +483,6 @@ static void test_sync_op(const void *opaque)
bdrv_graph_rdunlock_main_loop();
blk_set_aio_context(blk, ctx, &error_abort);
- aio_context_acquire(ctx);
if (t->fn) {
t->fn(c);
}
@@ -491,7 +490,6 @@ static void test_sync_op(const void *opaque)
t->blkfn(blk);
}
blk_set_aio_context(blk, qemu_get_aio_context(), &error_abort);
- aio_context_release(ctx);
bdrv_unref(bs);
blk_unref(blk);
@@ -576,9 +574,7 @@ static void test_attach_blockjob(void)
aio_poll(qemu_get_aio_context(), false);
}
- aio_context_acquire(ctx);
blk_set_aio_context(blk, qemu_get_aio_context(), &error_abort);
- aio_context_release(ctx);
tjob->n = 0;
while (tjob->n == 0) {
@@ -595,9 +591,7 @@ static void test_attach_blockjob(void)
WITH_JOB_LOCK_GUARD() {
job_complete_sync_locked(&tjob->common.job, &error_abort);
}
- aio_context_acquire(ctx);
blk_set_aio_context(blk, qemu_get_aio_context(), &error_abort);
- aio_context_release(ctx);
bdrv_unref(bs);
blk_unref(blk);
@@ -654,9 +648,7 @@ static void test_propagate_basic(void)
/* Switch the AioContext back */
main_ctx = qemu_get_aio_context();
- aio_context_acquire(ctx);
blk_set_aio_context(blk, main_ctx, &error_abort);
- aio_context_release(ctx);
g_assert(blk_get_aio_context(blk) == main_ctx);
g_assert(bdrv_get_aio_context(bs_a) == main_ctx);
g_assert(bdrv_get_aio_context(bs_verify) == main_ctx);
@@ -732,9 +724,7 @@ static void test_propagate_diamond(void)
/* Switch the AioContext back */
main_ctx = qemu_get_aio_context();
- aio_context_acquire(ctx);
blk_set_aio_context(blk, main_ctx, &error_abort);
- aio_context_release(ctx);
g_assert(blk_get_aio_context(blk) == main_ctx);
g_assert(bdrv_get_aio_context(bs_verify) == main_ctx);
g_assert(bdrv_get_aio_context(bs_a) == main_ctx);
@@ -764,13 +754,11 @@ static void test_propagate_mirror(void)
&error_abort);
/* Start a mirror job */
- aio_context_acquire(main_ctx);
mirror_start("job0", src, target, NULL, JOB_DEFAULT, 0, 0, 0,
MIRROR_SYNC_MODE_NONE, MIRROR_OPEN_BACKING_CHAIN, false,
BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT,
false, "filter_node", MIRROR_COPY_MODE_BACKGROUND,
&error_abort);
- aio_context_release(main_ctx);
WITH_JOB_LOCK_GUARD() {
job = job_get_locked("job0");
@@ -785,9 +773,7 @@ static void test_propagate_mirror(void)
g_assert(job->aio_context == ctx);
/* Change the AioContext of target */
- aio_context_acquire(ctx);
bdrv_try_change_aio_context(target, main_ctx, NULL, &error_abort);
- aio_context_release(ctx);
g_assert(bdrv_get_aio_context(src) == main_ctx);
g_assert(bdrv_get_aio_context(target) == main_ctx);
g_assert(bdrv_get_aio_context(filter) == main_ctx);
@@ -805,10 +791,8 @@ static void test_propagate_mirror(void)
g_assert(bdrv_get_aio_context(filter) == main_ctx);
/* ...unless we explicitly allow it */
- aio_context_acquire(ctx);
blk_set_allow_aio_context_change(blk, true);
bdrv_try_change_aio_context(target, ctx, NULL, &error_abort);
- aio_context_release(ctx);
g_assert(blk_get_aio_context(blk) == ctx);
g_assert(bdrv_get_aio_context(src) == ctx);
@@ -817,10 +801,8 @@ static void test_propagate_mirror(void)
job_cancel_sync_all();
- aio_context_acquire(ctx);
blk_set_aio_context(blk, main_ctx, &error_abort);
bdrv_try_change_aio_context(target, main_ctx, NULL, &error_abort);
- aio_context_release(ctx);
blk_unref(blk);
bdrv_unref(src);
@@ -836,7 +818,6 @@ static void test_attach_second_node(void)
BlockDriverState *bs, *filter;
QDict *options;
- aio_context_acquire(main_ctx);
blk = blk_new(ctx, BLK_PERM_ALL, BLK_PERM_ALL);
bs = bdrv_new_open_driver(&bdrv_test, "base", BDRV_O_RDWR, &error_abort);
blk_insert_bs(blk, bs, &error_abort);
@@ -846,15 +827,12 @@ static void test_attach_second_node(void)
qdict_put_str(options, "file", "base");
filter = bdrv_open(NULL, NULL, options, BDRV_O_RDWR, &error_abort);
- aio_context_release(main_ctx);
g_assert(blk_get_aio_context(blk) == ctx);
g_assert(bdrv_get_aio_context(bs) == ctx);
g_assert(bdrv_get_aio_context(filter) == ctx);
- aio_context_acquire(ctx);
blk_set_aio_context(blk, main_ctx, &error_abort);
- aio_context_release(ctx);
g_assert(blk_get_aio_context(blk) == main_ctx);
g_assert(bdrv_get_aio_context(bs) == main_ctx);
g_assert(bdrv_get_aio_context(filter) == main_ctx);
@@ -868,11 +846,9 @@ static void test_attach_preserve_blk_ctx(void)
{
IOThread *iothread = iothread_new();
AioContext *ctx = iothread_get_aio_context(iothread);
- AioContext *main_ctx = qemu_get_aio_context();
BlockBackend *blk;
BlockDriverState *bs;
- aio_context_acquire(main_ctx);
blk = blk_new(ctx, BLK_PERM_ALL, BLK_PERM_ALL);
bs = bdrv_new_open_driver(&bdrv_test, "base", BDRV_O_RDWR, &error_abort);
bs->total_sectors = 65536 / BDRV_SECTOR_SIZE;
@@ -881,25 +857,18 @@ static void test_attach_preserve_blk_ctx(void)
blk_insert_bs(blk, bs, &error_abort);
g_assert(blk_get_aio_context(blk) == ctx);
g_assert(bdrv_get_aio_context(bs) == ctx);
- aio_context_release(main_ctx);
/* Remove the node again */
- aio_context_acquire(ctx);
blk_remove_bs(blk);
- aio_context_release(ctx);
g_assert(blk_get_aio_context(blk) == ctx);
g_assert(bdrv_get_aio_context(bs) == qemu_get_aio_context());
/* Re-attach the node */
- aio_context_acquire(main_ctx);
blk_insert_bs(blk, bs, &error_abort);
- aio_context_release(main_ctx);
g_assert(blk_get_aio_context(blk) == ctx);
g_assert(bdrv_get_aio_context(bs) == ctx);
- aio_context_acquire(ctx);
blk_set_aio_context(blk, qemu_get_aio_context(), &error_abort);
- aio_context_release(ctx);
bdrv_unref(bs);
blk_unref(blk);
}
diff --git a/tests/unit/test-blockjob.c b/tests/unit/test-blockjob.c
index a130f6fefb..fe3e0d2d38 100644
--- a/tests/unit/test-blockjob.c
+++ b/tests/unit/test-blockjob.c
@@ -228,7 +228,6 @@ static void cancel_common(CancelJob *s)
BlockJob *job = &s->common;
BlockBackend *blk = s->blk;
JobStatus sts = job->job.status;
- AioContext *ctx = job->job.aio_context;
job_cancel_sync(&job->job, true);
WITH_JOB_LOCK_GUARD() {
@@ -240,9 +239,7 @@ static void cancel_common(CancelJob *s)
job_unref_locked(&job->job);
}
- aio_context_acquire(ctx);
destroy_blk(blk);
- aio_context_release(ctx);
}
@@ -391,132 +388,6 @@ static void test_cancel_concluded(void)
cancel_common(s);
}
-/* (See test_yielding_driver for the job description) */
-typedef struct YieldingJob {
- BlockJob common;
- bool should_complete;
-} YieldingJob;
-
-static void yielding_job_complete(Job *job, Error **errp)
-{
- YieldingJob *s = container_of(job, YieldingJob, common.job);
- s->should_complete = true;
- job_enter(job);
-}
-
-static int coroutine_fn yielding_job_run(Job *job, Error **errp)
-{
- YieldingJob *s = container_of(job, YieldingJob, common.job);
-
- job_transition_to_ready(job);
-
- while (!s->should_complete) {
- job_yield(job);
- }
-
- return 0;
-}
-
-/*
- * This job transitions immediately to the READY state, and then
- * yields until it is to complete.
- */
-static const BlockJobDriver test_yielding_driver = {
- .job_driver = {
- .instance_size = sizeof(YieldingJob),
- .free = block_job_free,
- .user_resume = block_job_user_resume,
- .run = yielding_job_run,
- .complete = yielding_job_complete,
- },
-};
-
-/*
- * Test that job_complete_locked() works even on jobs that are in a paused
- * state (i.e., STANDBY).
- *
- * To do this, run YieldingJob in an IO thread, get it into the READY
- * state, then have a drained section. Before ending the section,
- * acquire the context so the job will not be entered and will thus
- * remain on STANDBY.
- *
- * job_complete_locked() should still work without error.
- *
- * Note that on the QMP interface, it is impossible to lock an IO
- * thread before a drained section ends. In practice, the
- * bdrv_drain_all_end() and the aio_context_acquire() will be
- * reversed. However, that makes for worse reproducibility here:
- * Sometimes, the job would no longer be in STANDBY then but already
- * be started. We cannot prevent that, because the IO thread runs
- * concurrently. We can only prevent it by taking the lock before
- * ending the drained section, so we do that.
- *
- * (You can reverse the order of operations and most of the time the
- * test will pass, but sometimes the assert(status == STANDBY) will
- * fail.)
- */
-static void test_complete_in_standby(void)
-{
- BlockBackend *blk;
- IOThread *iothread;
- AioContext *ctx;
- Job *job;
- BlockJob *bjob;
-
- /* Create a test drive, move it to an IO thread */
- blk = create_blk(NULL);
- iothread = iothread_new();
-
- ctx = iothread_get_aio_context(iothread);
- blk_set_aio_context(blk, ctx, &error_abort);
-
- /* Create our test job */
- bjob = mk_job(blk, "job", &test_yielding_driver, true,
- JOB_MANUAL_FINALIZE | JOB_MANUAL_DISMISS);
- job = &bjob->job;
- assert_job_status_is(job, JOB_STATUS_CREATED);
-
- /* Wait for the job to become READY */
- job_start(job);
- /*
- * Here we are waiting for the status to change, so don't bother
- * protecting the read every time.
- */
- AIO_WAIT_WHILE_UNLOCKED(ctx, job->status != JOB_STATUS_READY);
-
- /* Begin the drained section, pausing the job */
- bdrv_drain_all_begin();
- assert_job_status_is(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();
- aio_context_release(ctx);
-
- WITH_JOB_LOCK_GUARD() {
- /* But the job cannot run, so it will remain on standby */
- assert(job->status == JOB_STATUS_STANDBY);
-
- /* Even though the job is on standby, this should work */
- job_complete_locked(job, &error_abort);
-
- /* The test is done now, clean up. */
- job_finish_sync_locked(job, NULL, &error_abort);
- assert(job->status == JOB_STATUS_PENDING);
-
- job_finalize_locked(job, &error_abort);
- assert(job->status == JOB_STATUS_CONCLUDED);
-
- job_dismiss_locked(&job, &error_abort);
- }
-
- aio_context_acquire(ctx);
- destroy_blk(blk);
- aio_context_release(ctx);
- iothread_join(iothread);
-}
-
int main(int argc, char **argv)
{
qemu_init_main_loop(&error_abort);
@@ -531,13 +402,5 @@ int main(int argc, char **argv)
g_test_add_func("/blockjob/cancel/standby", test_cancel_standby);
g_test_add_func("/blockjob/cancel/pending", test_cancel_pending);
g_test_add_func("/blockjob/cancel/concluded", test_cancel_concluded);
-
- /*
- * This test is flaky and sometimes fails in CI and otherwise:
- * don't run unless user opts in via environment variable.
- */
- if (getenv("QEMU_TEST_FLAKY_TESTS")) {
- g_test_add_func("/blockjob/complete_in_standby", test_complete_in_standby);
- }
return g_test_run();
}
diff --git a/tests/unit/test-replication.c b/tests/unit/test-replication.c
index afff908d77..5d2003b8ce 100644
--- a/tests/unit/test-replication.c
+++ b/tests/unit/test-replication.c
@@ -199,17 +199,13 @@ static BlockBackend *start_primary(void)
static void teardown_primary(void)
{
BlockBackend *blk;
- AioContext *ctx;
/* remove P_ID */
blk = blk_by_name(P_ID);
assert(blk);
- ctx = blk_get_aio_context(blk);
- aio_context_acquire(ctx);
monitor_remove_blk(blk);
blk_unref(blk);
- aio_context_release(ctx);
}
static void test_primary_read(void)
@@ -345,27 +341,20 @@ static void teardown_secondary(void)
{
/* only need to destroy two BBs */
BlockBackend *blk;
- AioContext *ctx;
/* remove S_LOCAL_DISK_ID */
blk = blk_by_name(S_LOCAL_DISK_ID);
assert(blk);
- ctx = blk_get_aio_context(blk);
- aio_context_acquire(ctx);
monitor_remove_blk(blk);
blk_unref(blk);
- aio_context_release(ctx);
/* remove S_ID */
blk = blk_by_name(S_ID);
assert(blk);
- ctx = blk_get_aio_context(blk);
- aio_context_acquire(ctx);
monitor_remove_blk(blk);
blk_unref(blk);
- aio_context_release(ctx);
}
static void test_secondary_read(void)