From patchwork Fri May 18 13:20:59 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 916365 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=2001:4830:134:3::11; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 40nV2c1wDWz9s1w for ; Fri, 18 May 2018 23:51:35 +1000 (AEST) Received: from localhost ([::1]:39065 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fJfnB-0000CT-BX for incoming@patchwork.ozlabs.org; Fri, 18 May 2018 09:51:33 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59916) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fJfKo-0000HK-Ld for qemu-devel@nongnu.org; Fri, 18 May 2018 09:22:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fJfKi-0006FI-3r for qemu-devel@nongnu.org; Fri, 18 May 2018 09:22:14 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:40994 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 1fJfKb-00069w-O0; Fri, 18 May 2018 09:22:01 -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 5837C40711B2; Fri, 18 May 2018 13:22:01 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-117-149.ams2.redhat.com [10.36.117.149]) by smtp.corp.redhat.com (Postfix) with ESMTP id 085D42024CBF; Fri, 18 May 2018 13:21:59 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Fri, 18 May 2018 15:20:59 +0200 Message-Id: <20180518132114.4070-26-kwolf@redhat.com> In-Reply-To: <20180518132114.4070-1-kwolf@redhat.com> References: <20180518132114.4070-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.7]); Fri, 18 May 2018 13:22:01 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Fri, 18 May 2018 13:22:01 +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] [PATCH v2 25/40] job: Switch transactions to JobTxn 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, qemu-devel@nongnu.org, jsnow@redhat.com, jcody@redhat.com, armbru@redhat.com, mreitz@redhat.com Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" This doesn't actually move any transaction code to Job yet, but it renames the type for transactions from BlockJobTxn to JobTxn and makes them contain Jobs rather than BlockJobs Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- include/block/block_int.h | 2 +- include/block/blockjob.h | 11 ++++---- include/block/blockjob_int.h | 2 +- include/qemu/job.h | 3 +++ block/backup.c | 2 +- blockdev.c | 14 +++++------ blockjob.c | 60 +++++++++++++++++++++++--------------------- tests/test-blockjob-txn.c | 8 +++--- 8 files changed, 54 insertions(+), 48 deletions(-) diff --git a/include/block/block_int.h b/include/block/block_int.h index 76b589da57..6c0927bce3 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -1029,7 +1029,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, BlockdevOnError on_target_error, int creation_flags, BlockCompletionFunc *cb, void *opaque, - BlockJobTxn *txn, Error **errp); + JobTxn *txn, Error **errp); void hmp_drive_add_node(Monitor *mon, const char *optstr); diff --git a/include/block/blockjob.h b/include/block/blockjob.h index 85ce18a381..44df025bd0 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -33,7 +33,7 @@ #define BLOCK_JOB_SLICE_TIME 100000000ULL /* ns */ typedef struct BlockJobDriver BlockJobDriver; -typedef struct BlockJobTxn BlockJobTxn; +typedef struct JobTxn JobTxn; /** * BlockJob: @@ -85,8 +85,7 @@ typedef struct BlockJob { /** BlockDriverStates that are involved in this block job */ GSList *nodes; - BlockJobTxn *txn; - QLIST_ENTRY(BlockJob) txn_list; + JobTxn *txn; } BlockJob; /** @@ -273,7 +272,7 @@ void block_job_iostatus_reset(BlockJob *job); * group. Jobs wait for each other before completing. Cancelling one job * cancels all jobs in the transaction. */ -BlockJobTxn *block_job_txn_new(void); +JobTxn *block_job_txn_new(void); /** * block_job_txn_unref: @@ -282,7 +281,7 @@ BlockJobTxn *block_job_txn_new(void); * or block_job_txn_new. If it's the last reference to the object, it will be * freed. */ -void block_job_txn_unref(BlockJobTxn *txn); +void block_job_txn_unref(JobTxn *txn); /** * block_job_txn_add_job: @@ -293,7 +292,7 @@ void block_job_txn_unref(BlockJobTxn *txn); * The caller must call either block_job_txn_unref() or block_job_completed() * to release the reference that is automatically grabbed here. */ -void block_job_txn_add_job(BlockJobTxn *txn, BlockJob *job); +void block_job_txn_add_job(JobTxn *txn, BlockJob *job); /** * block_job_is_internal: diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h index b8ca7bb0c9..ce66a9b51c 100644 --- a/include/block/blockjob_int.h +++ b/include/block/blockjob_int.h @@ -91,7 +91,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, - BlockJobTxn *txn, BlockDriverState *bs, uint64_t perm, + JobTxn *txn, BlockDriverState *bs, uint64_t perm, uint64_t shared_perm, int64_t speed, int flags, BlockCompletionFunc *cb, void *opaque, Error **errp); diff --git a/include/qemu/job.h b/include/qemu/job.h index 17e2ceca87..d4aa7fa981 100644 --- a/include/qemu/job.h +++ b/include/qemu/job.h @@ -132,6 +132,9 @@ typedef struct Job { /** Element of the list of jobs */ QLIST_ENTRY(Job) job_list; + + /** Element of the list of jobs in a job transaction */ + QLIST_ENTRY(Job) txn_list; } Job; /** diff --git a/block/backup.c b/block/backup.c index ca7d990b21..6172f90c90 100644 --- a/block/backup.c +++ b/block/backup.c @@ -547,7 +547,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, BlockdevOnError on_target_error, int creation_flags, BlockCompletionFunc *cb, void *opaque, - BlockJobTxn *txn, Error **errp) + JobTxn *txn, Error **errp) { int64_t len; BlockDriverInfo bdi; diff --git a/blockdev.c b/blockdev.c index 0967f6ab66..817c3848c0 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1446,7 +1446,7 @@ typedef struct BlkActionOps { struct BlkActionState { TransactionAction *action; const BlkActionOps *ops; - BlockJobTxn *block_job_txn; + JobTxn *block_job_txn; TransactionProperties *txn_props; QSIMPLEQ_ENTRY(BlkActionState) entry; }; @@ -1864,7 +1864,7 @@ typedef struct DriveBackupState { BlockJob *job; } DriveBackupState; -static BlockJob *do_drive_backup(DriveBackup *backup, BlockJobTxn *txn, +static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn, Error **errp); static void drive_backup_prepare(BlkActionState *common, Error **errp) @@ -1954,7 +1954,7 @@ typedef struct BlockdevBackupState { BlockJob *job; } BlockdevBackupState; -static BlockJob *do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn, +static BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn, Error **errp); static void blockdev_backup_prepare(BlkActionState *common, Error **errp) @@ -2243,7 +2243,7 @@ void qmp_transaction(TransactionActionList *dev_list, Error **errp) { TransactionActionList *dev_entry = dev_list; - BlockJobTxn *block_job_txn = NULL; + JobTxn *block_job_txn = NULL; BlkActionState *state, *next; Error *local_err = NULL; @@ -2251,7 +2251,7 @@ void qmp_transaction(TransactionActionList *dev_list, QSIMPLEQ_INIT(&snap_bdrv_states); /* Does this transaction get canceled as a group on failure? - * If not, we don't really need to make a BlockJobTxn. + * If not, we don't really need to make a JobTxn. */ props = get_transaction_properties(props); if (props->completion_mode != ACTION_COMPLETION_MODE_INDIVIDUAL) { @@ -3264,7 +3264,7 @@ out: aio_context_release(aio_context); } -static BlockJob *do_drive_backup(DriveBackup *backup, BlockJobTxn *txn, +static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn, Error **errp) { BlockDriverState *bs; @@ -3434,7 +3434,7 @@ BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp) return bdrv_named_nodes_list(errp); } -BlockJob *do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn, +BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn, Error **errp) { BlockDriverState *bs; diff --git a/blockjob.c b/blockjob.c index 1ed3e9c88d..bd35c4fa7a 100644 --- a/blockjob.c +++ b/blockjob.c @@ -37,13 +37,13 @@ #include "qemu/timer.h" /* Transactional group of block jobs */ -struct BlockJobTxn { +struct JobTxn { /* Is this txn being cancelled? */ bool aborting; /* List of jobs */ - QLIST_HEAD(, BlockJob) jobs; + QLIST_HEAD(, Job) jobs; /* Reference count */ int refcnt; @@ -94,27 +94,27 @@ BlockJob *block_job_get(const char *id) } } -BlockJobTxn *block_job_txn_new(void) +JobTxn *block_job_txn_new(void) { - BlockJobTxn *txn = g_new0(BlockJobTxn, 1); + JobTxn *txn = g_new0(JobTxn, 1); QLIST_INIT(&txn->jobs); txn->refcnt = 1; return txn; } -static void block_job_txn_ref(BlockJobTxn *txn) +static void block_job_txn_ref(JobTxn *txn) { txn->refcnt++; } -void block_job_txn_unref(BlockJobTxn *txn) +void block_job_txn_unref(JobTxn *txn) { if (txn && --txn->refcnt == 0) { g_free(txn); } } -void block_job_txn_add_job(BlockJobTxn *txn, BlockJob *job) +void block_job_txn_add_job(JobTxn *txn, BlockJob *job) { if (!txn) { return; @@ -123,14 +123,14 @@ void block_job_txn_add_job(BlockJobTxn *txn, BlockJob *job) assert(!job->txn); job->txn = txn; - QLIST_INSERT_HEAD(&txn->jobs, job, txn_list); + QLIST_INSERT_HEAD(&txn->jobs, &job->job, txn_list); block_job_txn_ref(txn); } void block_job_txn_del_job(BlockJob *job) { if (job->txn) { - QLIST_REMOVE(job, txn_list); + QLIST_REMOVE(&job->job, txn_list); block_job_txn_unref(job->txn); job->txn = NULL; } @@ -285,18 +285,22 @@ static void job_cancel_async(Job *job, bool force) job->force_cancel |= force; } -static int block_job_txn_apply(BlockJobTxn *txn, int fn(BlockJob *), bool lock) +static int block_job_txn_apply(JobTxn *txn, int fn(BlockJob *), bool lock) { AioContext *ctx; - BlockJob *job, *next; + Job *job, *next; + BlockJob *bjob; int rc = 0; QLIST_FOREACH_SAFE(job, &txn->jobs, txn_list, next) { + assert(is_block_job(job)); + bjob = container_of(job, BlockJob, job); + if (lock) { - ctx = blk_get_aio_context(job->blk); + ctx = job->aio_context; aio_context_acquire(ctx); } - rc = fn(job); + rc = fn(bjob); if (lock) { aio_context_release(ctx); } @@ -310,8 +314,8 @@ static int block_job_txn_apply(BlockJobTxn *txn, int fn(BlockJob *), bool lock) static void block_job_completed_txn_abort(BlockJob *job) { AioContext *ctx; - BlockJobTxn *txn = job->txn; - BlockJob *other_job; + JobTxn *txn = job->txn; + Job *other_job; if (txn->aborting) { /* @@ -324,7 +328,7 @@ static void block_job_completed_txn_abort(BlockJob *job) /* We are the first failed job. Cancel other jobs. */ QLIST_FOREACH(other_job, &txn->jobs, txn_list) { - ctx = blk_get_aio_context(other_job->blk); + ctx = other_job->aio_context; aio_context_acquire(ctx); } @@ -332,18 +336,18 @@ static void block_job_completed_txn_abort(BlockJob *job) * them; this job, however, may or may not be cancelled, depending * on the caller, so leave it. */ QLIST_FOREACH(other_job, &txn->jobs, txn_list) { - if (other_job != job) { - job_cancel_async(&other_job->job, false); + if (other_job != &job->job) { + job_cancel_async(other_job, false); } } while (!QLIST_EMPTY(&txn->jobs)) { other_job = QLIST_FIRST(&txn->jobs); - ctx = blk_get_aio_context(other_job->blk); - if (!job_is_completed(&other_job->job)) { - assert(job_is_cancelled(&other_job->job)); - job_finish_sync(&other_job->job, NULL, NULL); + ctx = other_job->aio_context; + if (!job_is_completed(other_job)) { + assert(job_is_cancelled(other_job)); + job_finish_sync(other_job, NULL, NULL); } - job_finalize_single(&other_job->job); + job_finalize_single(other_job); aio_context_release(ctx); } @@ -385,8 +389,8 @@ static int block_job_transition_to_pending(BlockJob *job) static void block_job_completed_txn_success(BlockJob *job) { - BlockJobTxn *txn = job->txn; - BlockJob *other_job; + JobTxn *txn = job->txn; + Job *other_job; job_state_transition(&job->job, JOB_STATUS_WAITING); @@ -395,10 +399,10 @@ static void block_job_completed_txn_success(BlockJob *job) * txn. */ QLIST_FOREACH(other_job, &txn->jobs, txn_list) { - if (!job_is_completed(&other_job->job)) { + if (!job_is_completed(other_job)) { return; } - assert(other_job->job.ret == 0); + assert(other_job->ret == 0); } block_job_txn_apply(txn, block_job_transition_to_pending, false); @@ -628,7 +632,7 @@ static void block_job_event_pending(Notifier *n, void *opaque) */ void *block_job_create(const char *job_id, const BlockJobDriver *driver, - BlockJobTxn *txn, BlockDriverState *bs, uint64_t perm, + JobTxn *txn, BlockDriverState *bs, uint64_t perm, uint64_t shared_perm, int64_t speed, int flags, BlockCompletionFunc *cb, void *opaque, Error **errp) { diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c index 1572f8d96f..ec5d592b68 100644 --- a/tests/test-blockjob-txn.c +++ b/tests/test-blockjob-txn.c @@ -93,7 +93,7 @@ static const BlockJobDriver test_block_job_driver = { */ static BlockJob *test_block_job_start(unsigned int iterations, bool use_timer, - int rc, int *result, BlockJobTxn *txn) + int rc, int *result, JobTxn *txn) { BlockDriverState *bs; TestBlockJob *s; @@ -122,7 +122,7 @@ static BlockJob *test_block_job_start(unsigned int iterations, static void test_single_job(int expected) { BlockJob *job; - BlockJobTxn *txn; + JobTxn *txn; int result = -EINPROGRESS; txn = block_job_txn_new(); @@ -160,7 +160,7 @@ static void test_pair_jobs(int expected1, int expected2) { BlockJob *job1; BlockJob *job2; - BlockJobTxn *txn; + JobTxn *txn; int result1 = -EINPROGRESS; int result2 = -EINPROGRESS; @@ -222,7 +222,7 @@ static void test_pair_jobs_fail_cancel_race(void) { BlockJob *job1; BlockJob *job2; - BlockJobTxn *txn; + JobTxn *txn; int result1 = -EINPROGRESS; int result2 = -EINPROGRESS;