From patchwork Mon Oct 24 20:56:31 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Snow X-Patchwork-Id: 686199 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org 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 3t2pkq3l40z9t2Y for ; Tue, 25 Oct 2016 08:07:15 +1100 (AEDT) Received: from localhost ([::1]:50123 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bymSd-0002qM-PO for incoming@patchwork.ozlabs.org; Mon, 24 Oct 2016 17:07:11 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57188) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bymIX-000345-VY for qemu-devel@nongnu.org; Mon, 24 Oct 2016 16:56:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bymIW-0003BN-Ib for qemu-devel@nongnu.org; Mon, 24 Oct 2016 16:56:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34298) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bymIT-00037R-0i; Mon, 24 Oct 2016 16:56:41 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3227031B334; Mon, 24 Oct 2016 20:56:40 +0000 (UTC) Received: from scv.usersys.redhat.com (dhcp-17-171.bos.redhat.com [10.18.17.171]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9OKuYDq025782; Mon, 24 Oct 2016 16:56:39 -0400 From: John Snow To: qemu-block@nongnu.org Date: Mon, 24 Oct 2016 16:56:31 -0400 Message-Id: <1477342593-30707-5-git-send-email-jsnow@redhat.com> In-Reply-To: <1477342593-30707-1-git-send-email-jsnow@redhat.com> References: <1477342593-30707-1-git-send-email-jsnow@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Mon, 24 Oct 2016 20:56:40 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v2 4/6] blockjob: add block_job_start 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, vsementsov@virtuozzo.com, jcody@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, pbonzini@redhat.com, John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Instead of automatically starting jobs at creation time via backup_start et al, we'd like to return a job object pointer that can be started manually at later point in time. For now, add the block_job_start mechanism and start the jobs automatically as we have been doing, with conversions job-by-job coming in later patches. Of note: cancellation of unstarted jobs will perform all the normal cleanup as if the job had started, particularly abort and clean. The only difference is that we will not emit any events, because the job never actually started. Signed-off-by: John Snow --- block/backup.c | 3 +-- block/commit.c | 3 +-- block/mirror.c | 3 +-- block/stream.c | 3 +-- blockjob.c | 51 ++++++++++++++++++++++++++++++++++++----------- include/block/blockjob.h | 9 +++++++++ tests/test-blockjob-txn.c | 12 +++++------ 7 files changed, 58 insertions(+), 26 deletions(-) diff --git a/block/backup.c b/block/backup.c index 622f64e..2ce5115 100644 --- a/block/backup.c +++ b/block/backup.c @@ -638,9 +638,8 @@ void backup_start(const char *job_id, BlockDriverState *bs, bdrv_op_block_all(target, job->common.blocker); job->common.len = len; - job->common.co = qemu_coroutine_create(job->common.driver->start, job); block_job_txn_add_job(txn, &job->common); - qemu_coroutine_enter(job->common.co); + block_job_start(&job->common); return; error: diff --git a/block/commit.c b/block/commit.c index cc2030d..89820d7 100644 --- a/block/commit.c +++ b/block/commit.c @@ -275,10 +275,9 @@ void commit_start(const char *job_id, BlockDriverState *bs, s->backing_file_str = g_strdup(backing_file_str); s->on_error = on_error; - s->common.co = qemu_coroutine_create(s->common.driver->start, s); trace_commit_start(bs, base, top, s, s->common.co); - qemu_coroutine_enter(s->common.co); + block_job_start(&s->common); } diff --git a/block/mirror.c b/block/mirror.c index 3a29b94..8130474 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -970,9 +970,8 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs, bdrv_op_block_all(target, s->common.blocker); - s->common.co = qemu_coroutine_create(s->common.driver->start, s); trace_mirror_start(bs, s, s->common.co, opaque); - qemu_coroutine_enter(s->common.co); + block_job_start(&s->common); } void mirror_start(const char *job_id, BlockDriverState *bs, diff --git a/block/stream.c b/block/stream.c index 8ffed9c..3e3a7d3 100644 --- a/block/stream.c +++ b/block/stream.c @@ -231,7 +231,6 @@ void stream_start(const char *job_id, BlockDriverState *bs, s->backing_file_str = g_strdup(backing_file_str); s->on_error = on_error; - s->common.co = qemu_coroutine_create(s->common.driver->start, s); trace_stream_start(bs, base, s, s->common.co); - qemu_coroutine_enter(s->common.co); + block_job_start(&s->common); } diff --git a/blockjob.c b/blockjob.c index 150b87e..f574bc8 100644 --- a/blockjob.c +++ b/blockjob.c @@ -171,7 +171,8 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver, job->blk = blk; job->cb = cb; job->opaque = opaque; - job->busy = true; + job->busy = false; + job->paused = true; job->refcnt = 1; bs->job = job; @@ -199,6 +200,21 @@ bool block_job_is_internal(BlockJob *job) return (job->id == NULL); } +static bool block_job_started(BlockJob *job) +{ + return job->co; +} + +void block_job_start(BlockJob *job) +{ + assert(job && !block_job_started(job) && job->paused && + !job->busy && job->driver->start); + job->paused = false; + job->busy = true; + job->co = qemu_coroutine_create(job->driver->start, job); + qemu_coroutine_enter(job->co); +} + void block_job_ref(BlockJob *job) { ++job->refcnt; @@ -239,14 +255,18 @@ static void block_job_completed_single(BlockJob *job) if (job->cb) { job->cb(job->opaque, job->ret); } - if (block_job_is_cancelled(job)) { - block_job_event_cancelled(job); - } else { - const char *msg = NULL; - if (job->ret < 0) { - msg = strerror(-job->ret); + + /* Emit events only if we actually started */ + if (block_job_started(job)) { + if (block_job_is_cancelled(job)) { + block_job_event_cancelled(job); + } else { + const char *msg = NULL; + if (job->ret < 0) { + msg = strerror(-job->ret); + } + block_job_event_completed(job, msg); } - block_job_event_completed(job, msg); } if (job->txn) { @@ -354,7 +374,8 @@ void block_job_complete(BlockJob *job, Error **errp) { /* Should not be reachable via external interface for internal jobs */ assert(job->id); - if (job->pause_count || job->cancelled || !job->driver->complete) { + if (job->pause_count || job->cancelled || + !block_job_started(job) || !job->driver->complete) { error_setg(errp, "The active block job '%s' cannot be completed", job->id); return; @@ -386,6 +407,8 @@ bool block_job_user_paused(BlockJob *job) void coroutine_fn block_job_pause_point(BlockJob *job) { + assert(job && block_job_started(job)); + if (!block_job_should_pause(job)) { return; } @@ -437,9 +460,13 @@ void block_job_enter(BlockJob *job) void block_job_cancel(BlockJob *job) { - job->cancelled = true; - block_job_iostatus_reset(job); - block_job_enter(job); + if (block_job_started(job)) { + job->cancelled = true; + block_job_iostatus_reset(job); + block_job_enter(job); + } else { + block_job_completed(job, -ECANCELED); + } } bool block_job_is_cancelled(BlockJob *job) diff --git a/include/block/blockjob.h b/include/block/blockjob.h index bfc8233..96a59e1 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -175,6 +175,15 @@ BlockJob *block_job_get(const char *id); void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp); /** + * block_job_start: + * @job: A job that has not yet been started. + * + * Begins execution of a block job. + * Takes ownership of one reference to the job object. + */ +void block_job_start(BlockJob *job); + +/** * block_job_cancel: * @job: The job to be canceled. * diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c index f9afc3b..b132e39 100644 --- a/tests/test-blockjob-txn.c +++ b/tests/test-blockjob-txn.c @@ -24,10 +24,6 @@ typedef struct { int *result; } TestBlockJob; -static const BlockJobDriver test_block_job_driver = { - .instance_size = sizeof(TestBlockJob), -}; - static void test_block_job_complete(BlockJob *job, void *opaque) { BlockDriverState *bs = blk_bs(job->blk); @@ -77,6 +73,11 @@ static void test_block_job_cb(void *opaque, int ret) g_free(data); } +static const BlockJobDriver test_block_job_driver = { + .instance_size = sizeof(TestBlockJob), + .start = test_block_job_run, +}; + /* Create a block job that completes with a given return code after a given * number of event loop iterations. The return code is stored in the given * result pointer. @@ -104,10 +105,9 @@ static BlockJob *test_block_job_start(unsigned int iterations, s->use_timer = use_timer; s->rc = rc; s->result = result; - s->common.co = qemu_coroutine_create(test_block_job_run, s); data->job = s; data->result = result; - qemu_coroutine_enter(s->common.co); + block_job_start(&s->common); return &s->common; }