diff --git a/block/trace-events b/block/trace-events index 112a8972bb..38e8b6db4a 100644 --- a/block/trace-events +++ b/block/trace-events @@ -6,6 +6,7 @@ bdrv_lock_medium(void *bs, bool locked) "bs %p locked %d" # blockjob.c block_job_state_transition(void *job, int ret, const char *legal, const char *s0, const char *s1) "job %p (ret: %d) attempting %s transition (%s-->%s)" +block_job_apply_verb(void *job, const char *state, const char *verb, const char *legal) "job %p in state %s; applying verb %s (%s)" # block/block-backend.c blk_co_preadv(void *blk, void *bs, int64_t offset, unsigned int bytes, int flags) "blk %p bs %p offset %"PRId64" bytes %u flags 0x%x" diff --git a/blockdev.c b/blockdev.c index b9de18f3b2..93ed3308e3 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3825,7 +3825,7 @@ void qmp_block_job_cancel(const char *device, } trace_qmp_block_job_cancel(job); - block_job_cancel(job); + block_job_user_cancel(job, errp); out: aio_context_release(aio_context); } @@ -3835,12 +3835,12 @@ void qmp_block_job_pause(const char *device, Error **errp) AioContext *aio_context; BlockJob *job = find_block_job(device, &aio_context, errp); - if (!job || block_job_user_paused(job)) { + if (!job) { return; } trace_qmp_block_job_pause(job); - block_job_user_pause(job); + block_job_user_pause(job, errp); aio_context_release(aio_context); } @@ -3849,12 +3849,12 @@ void qmp_block_job_resume(const char *device, Error **errp) AioContext *aio_context; BlockJob *job = find_block_job(device, &aio_context, errp); - if (!job || !block_job_user_paused(job)) { + if (!job) { return; } trace_qmp_block_job_resume(job); - block_job_user_resume(job); + block_job_user_resume(job, errp); aio_context_release(aio_context); } diff --git a/blockjob.c b/blockjob.c index 442426e27b..d369c0cb4d 100644 --- a/blockjob.c +++ b/blockjob.c @@ -53,6 +53,15 @@ bool BlockJobSTT[BLOCK_JOB_STATUS__MAX][BLOCK_JOB_STATUS__MAX] = { /* S: */ [BLOCK_JOB_STATUS_STANDBY] = {0, 0, 0, 0, 1, 0}, }; +bool BlockJobVerbTable[BLOCK_JOB_VERB__MAX][BLOCK_JOB_STATUS__MAX] = { + /* U, C, R, P, Y, S */ + [BLOCK_JOB_VERB_CANCEL] = {0, 1, 1, 1, 1, 1}, + [BLOCK_JOB_VERB_PAUSE] = {0, 1, 1, 1, 1, 1}, + [BLOCK_JOB_VERB_RESUME] = {0, 1, 1, 1, 1, 1}, + [BLOCK_JOB_VERB_SET_SPEED] = {0, 1, 1, 1, 1, 1}, + [BLOCK_JOB_VERB_COMPLETE] = {0, 0, 0, 0, 1, 0}, +}; + static void block_job_state_transition(BlockJob *job, BlockJobStatus s1) { BlockJobStatus s0 = job->status; @@ -67,6 +76,23 @@ static void block_job_state_transition(BlockJob *job, BlockJobStatus s1) job->status = s1; } +static int block_job_apply_verb(BlockJob *job, BlockJobVerb bv, Error **errp) +{ + assert(bv >= 0 && bv <= BLOCK_JOB_VERB__MAX); + trace_block_job_apply_verb(job, qapi_enum_lookup(&BlockJobStatus_lookup, + job->status), + qapi_enum_lookup(&BlockJobVerb_lookup, bv), + BlockJobVerbTable[bv][job->status] ? + "allowed" : "prohibited"); + if (BlockJobVerbTable[bv][job->status]) { + return 0; + } + error_setg(errp, "Job '%s' in state '%s' cannot accept command verb '%s'", + job->id, qapi_enum_lookup(&BlockJobStatus_lookup, job->status), + qapi_enum_lookup(&BlockJobVerb_lookup, bv)); + return -EPERM; +} + static void block_job_lock(void) { qemu_mutex_lock(&block_job_mutex); @@ -517,6 +543,9 @@ void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp) error_setg(errp, QERR_UNSUPPORTED); return; } + if (block_job_apply_verb(job, BLOCK_JOB_VERB_SET_SPEED, errp)) { + return; + } job->driver->set_speed(job, speed, &local_err); if (local_err) { error_propagate(errp, local_err); @@ -536,8 +565,10 @@ 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 || - !block_job_started(job) || !job->driver->complete) { + if (block_job_apply_verb(job, BLOCK_JOB_VERB_COMPLETE, errp)) { + return; + } + if (job->pause_count || job->cancelled || !job->driver->complete) { error_setg(errp, "The active block job '%s' cannot be completed", job->id); return; @@ -546,8 +577,15 @@ void block_job_complete(BlockJob *job, Error **errp) job->driver->complete(job, errp); } -void block_job_user_pause(BlockJob *job) +void block_job_user_pause(BlockJob *job, Error **errp) { + if (block_job_apply_verb(job, BLOCK_JOB_VERB_PAUSE, errp)) { + return; + } + if (job->user_paused) { + error_setg(errp, "Job is already paused"); + return; + } job->user_paused = true; block_job_pause(job); } @@ -557,13 +595,19 @@ bool block_job_user_paused(BlockJob *job) return job->user_paused; } -void block_job_user_resume(BlockJob *job) +void block_job_user_resume(BlockJob *job, Error **errp) { - if (job && job->user_paused && job->pause_count > 0) { - block_job_iostatus_reset(job); - job->user_paused = false; - block_job_resume(job); + assert(job); + if (!job->user_paused || job->pause_count <= 0) { + error_setg(errp, "Can't resume a job that was not paused"); + return; } + if (block_job_apply_verb(job, BLOCK_JOB_VERB_RESUME, errp)) { + return; + } + block_job_iostatus_reset(job); + job->user_paused = false; + block_job_resume(job); } void block_job_cancel(BlockJob *job) @@ -576,6 +620,14 @@ void block_job_cancel(BlockJob *job) } } +void block_job_user_cancel(BlockJob *job, Error **errp) +{ + if (block_job_apply_verb(job, BLOCK_JOB_VERB_CANCEL, errp)) { + return; + } + block_job_cancel(job); +} + /* A wrapper around block_job_cancel() taking an Error ** parameter so it may be * used with block_job_finish_sync() without the need for (rather nasty) * function pointer casts there. */ @@ -999,8 +1051,9 @@ BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError on_err, action, &error_abort); } if (action == BLOCK_ERROR_ACTION_STOP) { + block_job_pause(job); /* make the pause user visible, which will be resumed from QMP. */ - block_job_user_pause(job); + job->user_paused = true; block_job_iostatus_set_err(job, error); } return action; diff --git a/include/block/blockjob.h b/include/block/blockjob.h index b39a2f9521..df0a9773d1 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -249,7 +249,7 @@ BlockJobInfo *block_job_query(BlockJob *job, Error **errp); * Asynchronously pause the specified job. * Do not allow a resume until a matching call to block_job_user_resume. */ -void block_job_user_pause(BlockJob *job); +void block_job_user_pause(BlockJob *job, Error **errp); /** * block_job_paused: @@ -266,7 +266,16 @@ bool block_job_user_paused(BlockJob *job); * Resume the specified job. * Must be paired with a preceding block_job_user_pause. */ -void block_job_user_resume(BlockJob *job); +void block_job_user_resume(BlockJob *job, Error **errp); + +/** + * block_job_user_cancel: + * @job: The job to be cancelled. + * + * Cancels the specified job, but may refuse to do so if the + * operation isn't currently meaningful. + */ +void block_job_user_cancel(BlockJob *job, Error **errp); /** * block_job_cancel_sync: diff --git a/qapi/block-core.json b/qapi/block-core.json index 08f785e753..ecd24ce5b7 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -958,6 +958,26 @@ { 'enum': 'BlockJobType', 'data': ['commit', 'stream', 'mirror', 'backup'] } +## +# @BlockJobVerb: +# +# Represents command verbs that can be applied to a blockjob. +# +# @cancel: see @block-job-cancel +# +# @pause: see @block-job-pause +# +# @resume: see @block-job-resume +# +# @set-speed: see @block-job-set-speed +# +# @complete: see @block-job-complete +# +# Since: 2.12 +## +{ 'enum': 'BlockJobVerb', + 'data': ['cancel', 'pause', 'resume', 'set-speed', 'complete' ] } + ## # @BlockJobStatus: # diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index a7eecf1c1e..7673de1062 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -505,6 +505,7 @@ static void coroutine_fn test_job_start(void *opaque) { TestBlockJob *s = opaque; + block_job_event_ready(&s->common); while (!s->should_complete) { block_job_sleep_ns(&s->common, 100000); }