From nobody Wed Oct 29 09:08:31 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1524585296255188.58930779459445; Tue, 24 Apr 2018 08:54:56 -0700 (PDT) Received: from localhost ([::1]:59259 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fB0HP-0005l8-9q for importer@patchew.org; Tue, 24 Apr 2018 11:54:55 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47936) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fAzpJ-00087z-KB for qemu-devel@nongnu.org; Tue, 24 Apr 2018 11:25:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fAzpG-0008EX-4y for qemu-devel@nongnu.org; Tue, 24 Apr 2018 11:25:53 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:58900 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fAzpA-00087G-6X; Tue, 24 Apr 2018 11:25:44 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D250440201DF; Tue, 24 Apr 2018 15:25:43 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-117-100.ams2.redhat.com [10.36.117.100]) by smtp.corp.redhat.com (Postfix) with ESMTP id C1FC02026990; Tue, 24 Apr 2018 15:25:42 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 24 Apr 2018 17:24:58 +0200 Message-Id: <20180424152515.25664-17-kwolf@redhat.com> In-Reply-To: <20180424152515.25664-1-kwolf@redhat.com> References: <20180424152515.25664-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Tue, 24 Apr 2018 15:25:43 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Tue, 24 Apr 2018 15:25:43 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [RFC PATCH 16/33] job: Move defer_to_main_loop to Job X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, jcody@redhat.com, jsnow@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Signed-off-by: Kevin Wolf --- include/block/blockjob.h | 5 ---- include/block/blockjob_int.h | 19 --------------- include/qemu/job.h | 20 ++++++++++++++++ block/backup.c | 7 +++--- block/commit.c | 11 +++++---- block/mirror.c | 15 ++++++------ block/stream.c | 14 +++++------ blockjob.c | 57 ++++------------------------------------= ---- job.c | 33 +++++++++++++++++++++++++ tests/test-bdrv-drain.c | 7 +++--- tests/test-blockjob-txn.c | 13 +++++----- tests/test-blockjob.c | 7 +++--- 12 files changed, 98 insertions(+), 110 deletions(-) diff --git a/include/block/blockjob.h b/include/block/blockjob.h index 04efc94ffc..90942826f5 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -92,11 +92,6 @@ typedef struct BlockJob { */ bool ready; =20 - /** - * Set to true when the job has deferred work to the main loop. - */ - bool deferred_to_main_loop; - /** Status that is published by the query-block-jobs QMP API */ BlockDeviceIoStatus iostatus; =20 diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h index e9790e9a0d..b0338282c8 100644 --- a/include/block/blockjob_int.h +++ b/include/block/blockjob_int.h @@ -233,23 +233,4 @@ void block_job_event_ready(BlockJob *job); BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError on_= err, int is_read, int error); =20 -typedef void BlockJobDeferToMainLoopFn(BlockJob *job, void *opaque); - -/** - * block_job_defer_to_main_loop: - * @job: The job - * @fn: The function to run in the main loop - * @opaque: The opaque value that is passed to @fn - * - * This function must be called by the main job coroutine just before it - * returns. @fn is executed in the main loop with the BlockDriverState - * AioContext acquired. Block jobs must call bdrv_unref(), bdrv_close(), = and - * anything that uses bdrv_drain_all() in the main loop. - * - * The @job AioContext is held while @fn executes. - */ -void block_job_defer_to_main_loop(BlockJob *job, - BlockJobDeferToMainLoopFn *fn, - void *opaque); - #endif diff --git a/include/qemu/job.h b/include/qemu/job.h index c1882bb0dd..63a40b2714 100644 --- a/include/qemu/job.h +++ b/include/qemu/job.h @@ -58,6 +58,9 @@ typedef struct Job { */ bool cancelled; =20 + /** Set to true when the job has deferred work to the main loop. */ + bool deferred_to_main_loop; + /** Element of the list of jobs */ QLIST_ENTRY(Job) job_list; } Job; @@ -124,6 +127,23 @@ Job *job_next(Job *job); */ Job *job_get(const char *id); =20 +typedef void JobDeferToMainLoopFn(Job *job, void *opaque); + +/** + * @job: The job + * @fn: The function to run in the main loop + * @opaque: The opaque value that is passed to @fn + * + * This function must be called by the main job coroutine just before it + * returns. @fn is executed in the main loop with the job AioContext acqu= ired. + * + * Block jobs must call bdrv_unref(), bdrv_close(), and anything that uses + * bdrv_drain_all() in the main loop. + * + * The @job AioContext is held while @fn executes. + */ +void job_defer_to_main_loop(Job *job, JobDeferToMainLoopFn *fn, void *opaq= ue); + /* TODO To be removed from the public interface */ void job_state_transition(Job *job, JobStatus s1); int job_apply_verb(Job *job, JobVerb bv, Error **errp); diff --git a/block/backup.c b/block/backup.c index 1d330b7266..ce9cc01617 100644 --- a/block/backup.c +++ b/block/backup.c @@ -317,11 +317,12 @@ typedef struct { int ret; } BackupCompleteData; =20 -static void backup_complete(BlockJob *job, void *opaque) +static void backup_complete(Job *job, void *opaque) { + BlockJob *bjob =3D container_of(job, BlockJob, job); BackupCompleteData *data =3D opaque; =20 - block_job_completed(job, data->ret); + block_job_completed(bjob, data->ret); g_free(data); } =20 @@ -519,7 +520,7 @@ static void coroutine_fn backup_run(void *opaque) =20 data =3D g_malloc(sizeof(*data)); data->ret =3D ret; - block_job_defer_to_main_loop(&job->common, backup_complete, data); + job_defer_to_main_loop(&job->common.job, backup_complete, data); } =20 static const BlockJobDriver backup_job_driver =3D { diff --git a/block/commit.c b/block/commit.c index 85baea8f92..d326766e4d 100644 --- a/block/commit.c +++ b/block/commit.c @@ -72,9 +72,10 @@ typedef struct { int ret; } CommitCompleteData; =20 -static void commit_complete(BlockJob *job, void *opaque) +static void commit_complete(Job *job, void *opaque) { - CommitBlockJob *s =3D container_of(job, CommitBlockJob, common); + CommitBlockJob *s =3D container_of(job, CommitBlockJob, common.job); + BlockJob *bjob =3D &s->common; CommitCompleteData *data =3D opaque; BlockDriverState *top =3D blk_bs(s->top); BlockDriverState *base =3D blk_bs(s->base); @@ -90,7 +91,7 @@ static void commit_complete(BlockJob *job, void *opaque) * the normal backing chain can be restored. */ blk_unref(s->base); =20 - if (!job_is_cancelled(&s->common.job) && ret =3D=3D 0) { + if (!job_is_cancelled(job) && ret =3D=3D 0) { /* success */ ret =3D bdrv_drop_intermediate(s->commit_top_bs, base, s->backing_file_str); @@ -114,7 +115,7 @@ static void commit_complete(BlockJob *job, void *opaque) * block_job_finish_sync()), block_job_completed() won't free it and * therefore the blockers on the intermediate nodes remain. This would * cause bdrv_set_backing_hd() to fail. */ - block_job_remove_all_bdrv(job); + block_job_remove_all_bdrv(bjob); =20 block_job_completed(&s->common, ret); g_free(data); @@ -211,7 +212,7 @@ out: =20 data =3D g_malloc(sizeof(*data)); data->ret =3D ret; - block_job_defer_to_main_loop(&s->common, commit_complete, data); + job_defer_to_main_loop(&s->common.job, commit_complete, data); } =20 static const BlockJobDriver commit_job_driver =3D { diff --git a/block/mirror.c b/block/mirror.c index 937b915268..cb6a4f6fe1 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -484,9 +484,10 @@ typedef struct { int ret; } MirrorExitData; =20 -static void mirror_exit(BlockJob *job, void *opaque) +static void mirror_exit(Job *job, void *opaque) { - MirrorBlockJob *s =3D container_of(job, MirrorBlockJob, common); + MirrorBlockJob *s =3D container_of(job, MirrorBlockJob, common.job); + BlockJob *bjob =3D &s->common; MirrorExitData *data =3D opaque; AioContext *replace_aio_context =3D NULL; BlockDriverState *src =3D s->source; @@ -568,7 +569,7 @@ static void mirror_exit(BlockJob *job, void *opaque) * the blockers on the intermediate nodes so that the resulting state = is * valid. Also give up permissions on mirror_top_bs->backing, which mi= ght * block the removal. */ - block_job_remove_all_bdrv(job); + block_job_remove_all_bdrv(bjob); bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL, &error_abort); bdrv_replace_node(mirror_top_bs, backing_bs(mirror_top_bs), &error_abo= rt); @@ -576,9 +577,9 @@ static void mirror_exit(BlockJob *job, void *opaque) /* We just changed the BDS the job BB refers to (with either or both o= f the * bdrv_replace_node() calls), so switch the BB back so the cleanup do= es * the right thing. We don't need any permissions any more now. */ - blk_remove_bs(job->blk); - blk_set_perm(job->blk, 0, BLK_PERM_ALL, &error_abort); - blk_insert_bs(job->blk, mirror_top_bs, &error_abort); + blk_remove_bs(bjob->blk); + blk_set_perm(bjob->blk, 0, BLK_PERM_ALL, &error_abort); + blk_insert_bs(bjob->blk, mirror_top_bs, &error_abort); =20 block_job_completed(&s->common, data->ret); =20 @@ -897,7 +898,7 @@ immediate_exit: if (need_drain) { bdrv_drained_begin(bs); } - block_job_defer_to_main_loop(&s->common, mirror_exit, data); + job_defer_to_main_loop(&s->common.job, mirror_exit, data); } =20 static void mirror_complete(BlockJob *job, Error **errp) diff --git a/block/stream.c b/block/stream.c index 22c71ae100..0bba81678c 100644 --- a/block/stream.c +++ b/block/stream.c @@ -58,16 +58,16 @@ typedef struct { int ret; } StreamCompleteData; =20 -static void stream_complete(BlockJob *job, void *opaque) +static void stream_complete(Job *job, void *opaque) { - StreamBlockJob *s =3D container_of(job, StreamBlockJob, common); + StreamBlockJob *s =3D container_of(job, StreamBlockJob, common.job); + BlockJob *bjob =3D &s->common; StreamCompleteData *data =3D opaque; - BlockDriverState *bs =3D blk_bs(job->blk); + BlockDriverState *bs =3D blk_bs(bjob->blk); BlockDriverState *base =3D s->base; Error *local_err =3D NULL; =20 - if (!job_is_cancelled(&s->common.job) && bs->backing && - data->ret =3D=3D 0) { + if (!job_is_cancelled(job) && bs->backing && data->ret =3D=3D 0) { const char *base_id =3D NULL, *base_fmt =3D NULL; if (base) { base_id =3D s->backing_file_str; @@ -88,7 +88,7 @@ out: /* Reopen the image back in read-only mode if necessary */ if (s->bs_flags !=3D bdrv_get_flags(bs)) { /* Give up write permissions before making it read-only */ - blk_set_perm(job->blk, 0, BLK_PERM_ALL, &error_abort); + blk_set_perm(bjob->blk, 0, BLK_PERM_ALL, &error_abort); bdrv_reopen(bs, s->bs_flags, NULL); } =20 @@ -205,7 +205,7 @@ out: /* Modify backing chain and close BDSes in main loop */ data =3D g_malloc(sizeof(*data)); data->ret =3D ret; - block_job_defer_to_main_loop(&s->common, stream_complete, data); + job_defer_to_main_loop(&s->common.job, stream_complete, data); } =20 static const BlockJobDriver stream_job_driver =3D { diff --git a/blockjob.c b/blockjob.c index 734dd168dd..7524058e45 100644 --- a/blockjob.c +++ b/blockjob.c @@ -347,7 +347,7 @@ static void block_job_decommission(BlockJob *job) job->completed =3D true; job->busy =3D false; job->paused =3D false; - job->deferred_to_main_loop =3D true; + job->job.deferred_to_main_loop =3D true; block_job_txn_del_job(job); job_state_transition(&job->job, JOB_STATUS_NULL); job_unref(&job->job); @@ -502,7 +502,7 @@ static int block_job_finish_sync(BlockJob *job, /* block_job_drain calls block_job_enter, and it should be enough to * induce progress until the job completes or moves to the main thread. */ - while (!job->deferred_to_main_loop && !job->completed) { + while (!job->job.deferred_to_main_loop && !job->completed) { block_job_drain(job); } while (!job->completed) { @@ -722,7 +722,7 @@ void block_job_cancel(BlockJob *job, bool force) block_job_cancel_async(job, force); if (!block_job_started(job)) { block_job_completed(job, -ECANCELED); - } else if (job->deferred_to_main_loop) { + } else if (job->job.deferred_to_main_loop) { block_job_completed_txn_abort(job); } else { block_job_enter(job); @@ -1036,7 +1036,7 @@ static void block_job_enter_cond(BlockJob *job, bool(= *fn)(BlockJob *job)) if (!block_job_started(job)) { return; } - if (job->deferred_to_main_loop) { + if (job->job.deferred_to_main_loop) { return; } =20 @@ -1051,7 +1051,7 @@ static void block_job_enter_cond(BlockJob *job, bool(= *fn)(BlockJob *job)) return; } =20 - assert(!job->deferred_to_main_loop); + assert(!job->job.deferred_to_main_loop); timer_del(&job->sleep_timer); job->busy =3D true; block_job_unlock(); @@ -1157,50 +1157,3 @@ BlockErrorAction block_job_error_action(BlockJob *jo= b, BlockdevOnError on_err, } return action; } - -typedef struct { - BlockJob *job; - AioContext *aio_context; - BlockJobDeferToMainLoopFn *fn; - void *opaque; -} BlockJobDeferToMainLoopData; - -static void block_job_defer_to_main_loop_bh(void *opaque) -{ - BlockJobDeferToMainLoopData *data =3D opaque; - AioContext *aio_context; - - /* Prevent race with block_job_defer_to_main_loop() */ - aio_context_acquire(data->aio_context); - - /* Fetch BDS AioContext again, in case it has changed */ - aio_context =3D blk_get_aio_context(data->job->blk); - if (aio_context !=3D data->aio_context) { - aio_context_acquire(aio_context); - } - - data->fn(data->job, data->opaque); - - if (aio_context !=3D data->aio_context) { - aio_context_release(aio_context); - } - - aio_context_release(data->aio_context); - - g_free(data); -} - -void block_job_defer_to_main_loop(BlockJob *job, - BlockJobDeferToMainLoopFn *fn, - void *opaque) -{ - BlockJobDeferToMainLoopData *data =3D g_malloc(sizeof(*data)); - data->job =3D job; - data->aio_context =3D blk_get_aio_context(job->blk); - data->fn =3D fn; - data->opaque =3D opaque; - job->deferred_to_main_loop =3D true; - - aio_bh_schedule_oneshot(qemu_get_aio_context(), - block_job_defer_to_main_loop_bh, data); -} diff --git a/job.c b/job.c index a28a7765e5..8e79bcfb74 100644 --- a/job.c +++ b/job.c @@ -28,6 +28,7 @@ #include "qapi/error.h" #include "qemu/job.h" #include "qemu/id.h" +#include "qemu/main-loop.h" #include "trace-root.h" =20 static QLIST_HEAD(, Job) jobs =3D QLIST_HEAD_INITIALIZER(jobs); @@ -171,3 +172,35 @@ void job_unref(Job *job) g_free(job); } } + +typedef struct { + Job *job; + JobDeferToMainLoopFn *fn; + void *opaque; +} JobDeferToMainLoopData; + +static void job_defer_to_main_loop_bh(void *opaque) +{ + JobDeferToMainLoopData *data =3D opaque; + Job *job =3D data->job; + AioContext *aio_context =3D job->aio_context; + + /* Prevent race with job_defer_to_main_loop() */ + aio_context_acquire(aio_context); + data->fn(data->job, data->opaque); + aio_context_release(aio_context); + + g_free(data); +} + +void job_defer_to_main_loop(Job *job, JobDeferToMainLoopFn *fn, void *opaq= ue) +{ + JobDeferToMainLoopData *data =3D g_malloc(sizeof(*data)); + data->job =3D job; + data->fn =3D fn; + data->opaque =3D opaque; + job->deferred_to_main_loop =3D true; + + aio_bh_schedule_oneshot(qemu_get_aio_context(), + job_defer_to_main_loop_bh, data); +} diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index f9e37d479c..4f8cba8377 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -496,9 +496,10 @@ typedef struct TestBlockJob { bool should_complete; } TestBlockJob; =20 -static void test_job_completed(BlockJob *job, void *opaque) +static void test_job_completed(Job *job, void *opaque) { - block_job_completed(job, 0); + BlockJob *bjob =3D container_of(job, BlockJob, job); + block_job_completed(bjob, 0); } =20 static void coroutine_fn test_job_start(void *opaque) @@ -510,7 +511,7 @@ static void coroutine_fn test_job_start(void *opaque) block_job_sleep_ns(&s->common, 100000); } =20 - block_job_defer_to_main_loop(&s->common, test_job_completed, NULL); + job_defer_to_main_loop(&s->common.job, test_job_completed, NULL); } =20 static void test_job_complete(BlockJob *job, Error **errp) diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c index 26b4bbb230..c03f9662d8 100644 --- a/tests/test-blockjob-txn.c +++ b/tests/test-blockjob-txn.c @@ -24,16 +24,17 @@ typedef struct { int *result; } TestBlockJob; =20 -static void test_block_job_complete(BlockJob *job, void *opaque) +static void test_block_job_complete(Job *job, void *opaque) { - BlockDriverState *bs =3D blk_bs(job->blk); + BlockJob *bjob =3D container_of(job, BlockJob, job); + BlockDriverState *bs =3D blk_bs(bjob->blk); int rc =3D (intptr_t)opaque; =20 - if (job_is_cancelled(&job->job)) { + if (job_is_cancelled(job)) { rc =3D -ECANCELED; } =20 - block_job_completed(job, rc); + block_job_completed(bjob, rc); bdrv_unref(bs); } =20 @@ -54,8 +55,8 @@ static void coroutine_fn test_block_job_run(void *opaque) } } =20 - block_job_defer_to_main_loop(job, test_block_job_complete, - (void *)(intptr_t)s->rc); + job_defer_to_main_loop(&job->job, test_block_job_complete, + (void *)(intptr_t)s->rc); } =20 typedef struct { diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c index fa31481537..5f43bd72a4 100644 --- a/tests/test-blockjob.c +++ b/tests/test-blockjob.c @@ -161,11 +161,12 @@ typedef struct CancelJob { bool completed; } CancelJob; =20 -static void cancel_job_completed(BlockJob *job, void *opaque) +static void cancel_job_completed(Job *job, void *opaque) { + BlockJob *bjob =3D container_of(job, BlockJob, job); CancelJob *s =3D opaque; s->completed =3D true; - block_job_completed(job, 0); + block_job_completed(bjob, 0); } =20 static void cancel_job_complete(BlockJob *job, Error **errp) @@ -191,7 +192,7 @@ static void coroutine_fn cancel_job_start(void *opaque) } =20 defer: - block_job_defer_to_main_loop(&s->common, cancel_job_completed, s); + job_defer_to_main_loop(&s->common.job, cancel_job_completed, s); } =20 static const BlockJobDriver test_cancel_driver =3D { --=20 2.13.6