From nobody Fri Oct 24 09:53:12 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 1519430676275877.7443917224737; Fri, 23 Feb 2018 16:04:36 -0800 (PST) Received: from localhost ([::1]:47937 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1epNKN-0007yP-EW for importer@patchew.org; Fri, 23 Feb 2018 19:04:35 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55474) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1epN8S-0005zP-0k for qemu-devel@nongnu.org; Fri, 23 Feb 2018 18:52:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1epN8O-00063a-9n for qemu-devel@nongnu.org; Fri, 23 Feb 2018 18:52:16 -0500 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:33880 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 1epN8E-0005iJ-NV; Fri, 23 Feb 2018 18:52:02 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 001BE4085010; Fri, 23 Feb 2018 23:51:51 +0000 (UTC) Received: from probe.bos.redhat.com (dhcp-17-231.bos.redhat.com [10.18.17.231]) by smtp.corp.redhat.com (Postfix) with ESMTP id C352810AF9FC; Fri, 23 Feb 2018 23:51:51 +0000 (UTC) From: John Snow To: qemu-block@nongnu.org Date: Fri, 23 Feb 2018 18:51:23 -0500 Message-Id: <20180223235142.21501-3-jsnow@redhat.com> In-Reply-To: <20180223235142.21501-1-jsnow@redhat.com> References: <20180223235142.21501-1-jsnow@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Fri, 23 Feb 2018 23:51:52 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Fri, 23 Feb 2018 23:51:52 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'jsnow@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 v4 02/21] blockjobs: model single jobs as transactions 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, John Snow , pkrempa@redhat.com, jtc@redhat.com, qemu-devel@nongnu.org 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" model all independent jobs as single job transactions. It's one less case we have to worry about when we add more states to the transition machine. This way, we can just treat all job lifetimes exactly the same. This helps tighten assertions of the STM graph and removes some conditionals that would have been needed in the coming commits adding a more explicit job lifetime management API. Signed-off-by: John Snow Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf --- block/backup.c | 3 +-- block/commit.c | 2 +- block/mirror.c | 2 +- block/stream.c | 2 +- blockjob.c | 25 ++++++++++++++++--------- include/block/blockjob_int.h | 3 ++- tests/test-bdrv-drain.c | 4 ++-- tests/test-blockjob-txn.c | 19 +++++++------------ tests/test-blockjob.c | 2 +- 9 files changed, 32 insertions(+), 30 deletions(-) diff --git a/block/backup.c b/block/backup.c index 4a16a37229..7e254dabff 100644 --- a/block/backup.c +++ b/block/backup.c @@ -621,7 +621,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDr= iverState *bs, } =20 /* job->common.len is fixed, so we can't allow resize */ - job =3D block_job_create(job_id, &backup_job_driver, bs, + job =3D block_job_create(job_id, &backup_job_driver, txn, bs, BLK_PERM_CONSISTENT_READ, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE | BLK_PERM_WRITE_UNCHANGED | BLK_PERM_GRAPH_MOD, @@ -677,7 +677,6 @@ BlockJob *backup_job_create(const char *job_id, BlockDr= iverState *bs, block_job_add_bdrv(&job->common, "target", target, 0, BLK_PERM_ALL, &error_abort); job->common.len =3D len; - block_job_txn_add_job(txn, &job->common); =20 return &job->common; =20 diff --git a/block/commit.c b/block/commit.c index bb6c904704..9682158ee7 100644 --- a/block/commit.c +++ b/block/commit.c @@ -289,7 +289,7 @@ void commit_start(const char *job_id, BlockDriverState = *bs, return; } =20 - s =3D block_job_create(job_id, &commit_job_driver, bs, 0, BLK_PERM_ALL, + s =3D block_job_create(job_id, &commit_job_driver, NULL, bs, 0, BLK_PE= RM_ALL, speed, BLOCK_JOB_DEFAULT, NULL, NULL, errp); if (!s) { return; diff --git a/block/mirror.c b/block/mirror.c index c9badc1203..6bab7cfdd8 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -1166,7 +1166,7 @@ static void mirror_start_job(const char *job_id, Bloc= kDriverState *bs, } =20 /* Make sure that the source is not resized while the job is running */ - s =3D block_job_create(job_id, driver, mirror_top_bs, + s =3D block_job_create(job_id, driver, NULL, mirror_top_bs, BLK_PERM_CONSISTENT_READ, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANG= ED | BLK_PERM_WRITE | BLK_PERM_GRAPH_MOD, speed, diff --git a/block/stream.c b/block/stream.c index 499cdacdb0..f3b53f49e2 100644 --- a/block/stream.c +++ b/block/stream.c @@ -244,7 +244,7 @@ void stream_start(const char *job_id, BlockDriverState = *bs, /* Prevent concurrent jobs trying to modify the graph structure here, = we * already have our own plans. Also don't allow resize as the image si= ze is * queried only at the job start and then cached. */ - s =3D block_job_create(job_id, &stream_job_driver, bs, + s =3D block_job_create(job_id, &stream_job_driver, NULL, bs, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANG= ED | BLK_PERM_GRAPH_MOD, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANG= ED | diff --git a/blockjob.c b/blockjob.c index 24833ef30f..7ba3683ee3 100644 --- a/blockjob.c +++ b/blockjob.c @@ -357,10 +357,8 @@ static void block_job_completed_single(BlockJob *job) } } =20 - if (job->txn) { - QLIST_REMOVE(job, txn_list); - block_job_txn_unref(job->txn); - } + QLIST_REMOVE(job, txn_list); + block_job_txn_unref(job->txn); block_job_unref(job); } =20 @@ -647,7 +645,7 @@ static void block_job_event_completed(BlockJob *job, co= nst char *msg) */ =20 void *block_job_create(const char *job_id, const BlockJobDriver *driver, - BlockDriverState *bs, uint64_t perm, + BlockJobTxn *txn, BlockDriverState *bs, uint64_t pe= rm, uint64_t shared_perm, int64_t speed, int flags, BlockCompletionFunc *cb, void *opaque, Error **errp) { @@ -729,6 +727,17 @@ void *block_job_create(const char *job_id, const Block= JobDriver *driver, return NULL; } } + + /* Single jobs are modeled as single-job transactions for sake of + * consolidating the job management logic */ + if (!txn) { + txn =3D block_job_txn_new(); + block_job_txn_add_job(txn, job); + block_job_txn_unref(txn); + } else { + block_job_txn_add_job(txn, job); + } + return job; } =20 @@ -752,13 +761,11 @@ void block_job_early_fail(BlockJob *job) =20 void block_job_completed(BlockJob *job, int ret) { + assert(job && job->txn && !job->completed); assert(blk_bs(job->blk)->job =3D=3D job); - assert(!job->completed); job->completed =3D true; job->ret =3D ret; - if (!job->txn) { - block_job_completed_single(job); - } else if (ret < 0 || block_job_is_cancelled(job)) { + if (ret < 0 || block_job_is_cancelled(job)) { block_job_completed_txn_abort(job); } else { block_job_completed_txn_success(job); diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h index c9b23b0cc9..becaae74c2 100644 --- a/include/block/blockjob_int.h +++ b/include/block/blockjob_int.h @@ -115,6 +115,7 @@ struct BlockJobDriver { * @job_id: The id of the newly-created job, or %NULL to have one * generated automatically. * @job_type: The class object for the newly-created job. + * @txn: The transaction this job belongs to, if any. %NULL otherwise. * @bs: The block * @perm, @shared_perm: Permissions to request for @bs * @speed: The maximum speed, in bytes per second, or 0 for unlimited. @@ -132,7 +133,7 @@ struct BlockJobDriver { * called from a wrapper that is specific to the job type. */ void *block_job_create(const char *job_id, const BlockJobDriver *driver, - BlockDriverState *bs, uint64_t perm, + BlockJobTxn *txn, BlockDriverState *bs, uint64_t pe= rm, uint64_t shared_perm, int64_t speed, int flags, BlockCompletionFunc *cb, void *opaque, Error **errp= ); =20 diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index d760e2b243..a7eecf1c1e 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -541,8 +541,8 @@ static void test_blockjob_common(enum drain_type drain_= type) blk_target =3D blk_new(BLK_PERM_ALL, BLK_PERM_ALL); blk_insert_bs(blk_target, target, &error_abort); =20 - job =3D block_job_create("job0", &test_job_driver, src, 0, BLK_PERM_AL= L, 0, - 0, NULL, NULL, &error_abort); + job =3D block_job_create("job0", &test_job_driver, NULL, src, 0, BLK_P= ERM_ALL, + 0, 0, NULL, NULL, &error_abort); block_job_add_bdrv(job, "target", target, 0, BLK_PERM_ALL, &error_abor= t); block_job_start(job); =20 diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c index 3591c9617f..34f09ef8c1 100644 --- a/tests/test-blockjob-txn.c +++ b/tests/test-blockjob-txn.c @@ -87,7 +87,7 @@ static const BlockJobDriver test_block_job_driver =3D { */ static BlockJob *test_block_job_start(unsigned int iterations, bool use_timer, - int rc, int *result) + int rc, int *result, BlockJobTxn *tx= n) { BlockDriverState *bs; TestBlockJob *s; @@ -101,7 +101,7 @@ static BlockJob *test_block_job_start(unsigned int iter= ations, g_assert_nonnull(bs); =20 snprintf(job_id, sizeof(job_id), "job%u", counter++); - s =3D block_job_create(job_id, &test_block_job_driver, bs, + s =3D block_job_create(job_id, &test_block_job_driver, txn, bs, 0, BLK_PERM_ALL, 0, BLOCK_JOB_DEFAULT, test_block_job_cb, data, &error_abort); s->iterations =3D iterations; @@ -120,8 +120,7 @@ static void test_single_job(int expected) int result =3D -EINPROGRESS; =20 txn =3D block_job_txn_new(); - job =3D test_block_job_start(1, true, expected, &result); - block_job_txn_add_job(txn, job); + job =3D test_block_job_start(1, true, expected, &result, txn); block_job_start(job); =20 if (expected =3D=3D -ECANCELED) { @@ -160,10 +159,8 @@ static void test_pair_jobs(int expected1, int expected= 2) int result2 =3D -EINPROGRESS; =20 txn =3D block_job_txn_new(); - job1 =3D test_block_job_start(1, true, expected1, &result1); - block_job_txn_add_job(txn, job1); - job2 =3D test_block_job_start(2, true, expected2, &result2); - block_job_txn_add_job(txn, job2); + job1 =3D test_block_job_start(1, true, expected1, &result1, txn); + job2 =3D test_block_job_start(2, true, expected2, &result2, txn); block_job_start(job1); block_job_start(job2); =20 @@ -224,10 +221,8 @@ static void test_pair_jobs_fail_cancel_race(void) int result2 =3D -EINPROGRESS; =20 txn =3D block_job_txn_new(); - job1 =3D test_block_job_start(1, true, -ECANCELED, &result1); - block_job_txn_add_job(txn, job1); - job2 =3D test_block_job_start(2, false, 0, &result2); - block_job_txn_add_job(txn, job2); + job1 =3D test_block_job_start(1, true, -ECANCELED, &result1, txn); + job2 =3D test_block_job_start(2, false, 0, &result2, txn); block_job_start(job1); block_job_start(job2); =20 diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c index 23bdf1a932..599e28d732 100644 --- a/tests/test-blockjob.c +++ b/tests/test-blockjob.c @@ -30,7 +30,7 @@ static BlockJob *do_test_id(BlockBackend *blk, const char= *id, BlockJob *job; Error *errp =3D NULL; =20 - job =3D block_job_create(id, &test_block_job_driver, blk_bs(blk), + job =3D block_job_create(id, &test_block_job_driver, NULL, blk_bs(blk), 0, BLK_PERM_ALL, 0, BLOCK_JOB_DEFAULT, block_jo= b_cb, NULL, &errp); if (should_succeed) { --=20 2.14.3