From: Max Reitz <mreitz@redhat.com>
To: qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
John Snow <jsnow@redhat.com>,
qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>
Subject: [PATCH for-6.0? 2/3] test-blockjob: Test job_wait_unpaused()
Date: Thu, 8 Apr 2021 18:20:38 +0200 [thread overview]
Message-ID: <20210408162039.242670-3-mreitz@redhat.com> (raw)
In-Reply-To: <20210408162039.242670-1-mreitz@redhat.com>
Create a job that remains on STANDBY after a drained section, and see
that invoking job_wait_unpaused() will get it unstuck.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/unit/test-blockjob.c | 140 +++++++++++++++++++++++++++++++++++++
1 file changed, 140 insertions(+)
diff --git a/tests/unit/test-blockjob.c b/tests/unit/test-blockjob.c
index 7519847912..b7736e298d 100644
--- a/tests/unit/test-blockjob.c
+++ b/tests/unit/test-blockjob.c
@@ -16,6 +16,7 @@
#include "block/blockjob_int.h"
#include "sysemu/block-backend.h"
#include "qapi/qmp/qdict.h"
+#include "iothread.h"
static const BlockJobDriver test_block_job_driver = {
.job_driver = {
@@ -375,6 +376,144 @@ 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_wait_unpaused() can get jobs from a paused state to
+ * a running state so that job_complete() can be applied (assuming the
+ * pause occurred due to a drain that has already been lifted).
+ * (This is what QMP's block-job-complete does so it can be executed
+ * even immediately after some other operation instated and lifted a
+ * drain.)
+ *
+ * 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.
+ *
+ * Invoking job_complete() then will fail.
+ *
+ * However, job_wait_unpaused() should see the job is to be resumed,
+ * wait for it to be resumed, and then we can invoke job_complete()
+ * 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;
+ Error *local_err = NULL;
+
+ /* 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 == 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_context_release(ctx);
+
+ /* Begin the drained section, pausing the job */
+ bdrv_drain_all_begin();
+ assert(job->status == 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();
+
+ /* But the job cannot run, so it will remain on standby */
+ assert(job->status == JOB_STATUS_STANDBY);
+
+ /* A job on standby cannot be completed */
+ job_complete(job, &local_err);
+ assert(local_err != NULL);
+ error_free(local_err);
+ local_err = NULL;
+
+ /*
+ * But waiting for it and then completing it should work.
+ * (This is what qmp_block_job_complete() does.)
+ */
+ job_wait_unpaused(job, &error_abort);
+ job_complete(job, &error_abort);
+
+ /* The test is done now, clean up. */
+ job_finish_sync(job, NULL, &error_abort);
+ assert(job->status == JOB_STATUS_PENDING);
+
+ job_finalize(job, &error_abort);
+ assert(job->status == JOB_STATUS_CONCLUDED);
+
+ job_dismiss(&job, &error_abort);
+
+ destroy_blk(blk);
+ aio_context_release(ctx);
+ iothread_join(iothread);
+}
+
int main(int argc, char **argv)
{
qemu_init_main_loop(&error_abort);
@@ -389,5 +528,6 @@ 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);
+ g_test_add_func("/blockjob/complete_in_standby", test_complete_in_standby);
return g_test_run();
}
--
2.29.2
next prev parent reply other threads:[~2021-04-08 16:37 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-08 16:20 [PATCH for-6.0? 0/3] job: Add job_wait_unpaused() for block-job-complete Max Reitz
2021-04-08 16:20 ` [PATCH for-6.0? 1/3] " Max Reitz
2021-04-08 16:55 ` John Snow
2021-04-09 9:31 ` Max Reitz
2021-04-09 10:17 ` Kevin Wolf
2021-04-09 9:44 ` Kevin Wolf
2021-04-09 9:57 ` Max Reitz
2021-04-09 16:54 ` John Snow
2021-04-08 16:58 ` Vladimir Sementsov-Ogievskiy
2021-04-08 17:04 ` John Snow
2021-04-08 17:26 ` Vladimir Sementsov-Ogievskiy
2021-04-09 9:51 ` Max Reitz
2021-04-09 10:07 ` Vladimir Sementsov-Ogievskiy
2021-04-09 10:18 ` Max Reitz
2021-04-09 9:38 ` Max Reitz
2021-04-08 16:20 ` Max Reitz [this message]
2021-04-08 16:20 ` [PATCH for-6.0? 3/3] iotests/041: block-job-complete on user-paused job Max Reitz
2021-04-08 17:09 ` [PATCH for-6.0? 0/3] job: Add job_wait_unpaused() for block-job-complete John Snow
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=20210408162039.242670-3-mreitz@redhat.com \
--to=mreitz@redhat.com \
--cc=jsnow@redhat.com \
--cc=kwolf@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=vsementsov@virtuozzo.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).