jcs.org / qemu
qemu with hax to log dma reads & writes jcs.org/2018/11/12/vfio
fork atom

blockjobs: add block_job_verb permission table

Which commands ("verbs") are appropriate for jobs in which state is
also somewhat burdensome to keep track of.

As of this commit, it looks rather useless, but begins to look more
interesting the more states we add to the STM table.

A recurring theme is that no verb will apply to an 'undefined' job.

Further, it's not presently possible to restrict the "pause" or "resume"
verbs any more than they are in this commit because of the asynchronous
nature of how jobs enter the PAUSED state; justifications for some
seemingly erroneous applications are given below.

=====
Verbs
=====

Cancel: Any state except undefined.
Pause: Any state except undefined;
'created': Requests that the job pauses as it starts.
'running': Normal usage. (PAUSED)
'paused': The job may be paused for internal reasons,
but the user may wish to force an indefinite
user-pause, so this is allowed.
'ready': Normal usage. (STANDBY)
'standby': Same logic as above.
Resume: Any state except undefined;
'created': Will lift a user's pause-on-start request.
'running': Will lift a pause request before it takes effect.
'paused': Normal usage.
'ready': Will lift a pause request before it takes effect.
'standby': Normal usage.
Set-speed: Any state except undefined, though ready may not be meaningful.
Complete: Only a 'ready' job may accept a complete request.

=======
Changes
=======

(1)

To facilitate "nice" error checking, all five major block-job verb
interfaces in blockjob.c now support an errp parameter:

- block_job_user_cancel is added as a new interface.
- block_job_user_pause gains an errp paramter
- block_job_user_resume gains an errp parameter
- block_job_set_speed already had an errp parameter.
- block_job_complete already had an errp parameter.

(2)

block-job-pause and block-job-resume will no longer no-op when trying
to pause an already paused job, or trying to resume a job that isn't
paused. These functions will now report that they did not perform the
action requested because it was not possible.

iotests have been adjusted to address this new behavior.

(3)

block-job-complete doesn't worry about checking !block_job_started,
because the permission table guards against this.

(4)

test-bdrv-drain's job implementation needs to announce that it is
'ready' now, in order to be completed.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>

authored by

John Snow and committed by
Kevin Wolf 0ec4dfb8 f03d9d24

+100 -16
+1
block/trace-events
··· 6 6 7 7 # blockjob.c 8 8 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)" 9 + block_job_apply_verb(void *job, const char *state, const char *verb, const char *legal) "job %p in state %s; applying verb %s (%s)" 9 10 10 11 # block/block-backend.c 11 12 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"
+5 -5
blockdev.c
··· 3825 3825 } 3826 3826 3827 3827 trace_qmp_block_job_cancel(job); 3828 - block_job_cancel(job); 3828 + block_job_user_cancel(job, errp); 3829 3829 out: 3830 3830 aio_context_release(aio_context); 3831 3831 } ··· 3835 3835 AioContext *aio_context; 3836 3836 BlockJob *job = find_block_job(device, &aio_context, errp); 3837 3837 3838 - if (!job || block_job_user_paused(job)) { 3838 + if (!job) { 3839 3839 return; 3840 3840 } 3841 3841 3842 3842 trace_qmp_block_job_pause(job); 3843 - block_job_user_pause(job); 3843 + block_job_user_pause(job, errp); 3844 3844 aio_context_release(aio_context); 3845 3845 } 3846 3846 ··· 3849 3849 AioContext *aio_context; 3850 3850 BlockJob *job = find_block_job(device, &aio_context, errp); 3851 3851 3852 - if (!job || !block_job_user_paused(job)) { 3852 + if (!job) { 3853 3853 return; 3854 3854 } 3855 3855 3856 3856 trace_qmp_block_job_resume(job); 3857 - block_job_user_resume(job); 3857 + block_job_user_resume(job, errp); 3858 3858 aio_context_release(aio_context); 3859 3859 } 3860 3860
+62 -9
blockjob.c
··· 53 53 /* S: */ [BLOCK_JOB_STATUS_STANDBY] = {0, 0, 0, 0, 1, 0}, 54 54 }; 55 55 56 + bool BlockJobVerbTable[BLOCK_JOB_VERB__MAX][BLOCK_JOB_STATUS__MAX] = { 57 + /* U, C, R, P, Y, S */ 58 + [BLOCK_JOB_VERB_CANCEL] = {0, 1, 1, 1, 1, 1}, 59 + [BLOCK_JOB_VERB_PAUSE] = {0, 1, 1, 1, 1, 1}, 60 + [BLOCK_JOB_VERB_RESUME] = {0, 1, 1, 1, 1, 1}, 61 + [BLOCK_JOB_VERB_SET_SPEED] = {0, 1, 1, 1, 1, 1}, 62 + [BLOCK_JOB_VERB_COMPLETE] = {0, 0, 0, 0, 1, 0}, 63 + }; 64 + 56 65 static void block_job_state_transition(BlockJob *job, BlockJobStatus s1) 57 66 { 58 67 BlockJobStatus s0 = job->status; ··· 65 74 s1)); 66 75 assert(BlockJobSTT[s0][s1]); 67 76 job->status = s1; 77 + } 78 + 79 + static int block_job_apply_verb(BlockJob *job, BlockJobVerb bv, Error **errp) 80 + { 81 + assert(bv >= 0 && bv <= BLOCK_JOB_VERB__MAX); 82 + trace_block_job_apply_verb(job, qapi_enum_lookup(&BlockJobStatus_lookup, 83 + job->status), 84 + qapi_enum_lookup(&BlockJobVerb_lookup, bv), 85 + BlockJobVerbTable[bv][job->status] ? 86 + "allowed" : "prohibited"); 87 + if (BlockJobVerbTable[bv][job->status]) { 88 + return 0; 89 + } 90 + error_setg(errp, "Job '%s' in state '%s' cannot accept command verb '%s'", 91 + job->id, qapi_enum_lookup(&BlockJobStatus_lookup, job->status), 92 + qapi_enum_lookup(&BlockJobVerb_lookup, bv)); 93 + return -EPERM; 68 94 } 69 95 70 96 static void block_job_lock(void) ··· 517 543 error_setg(errp, QERR_UNSUPPORTED); 518 544 return; 519 545 } 546 + if (block_job_apply_verb(job, BLOCK_JOB_VERB_SET_SPEED, errp)) { 547 + return; 548 + } 520 549 job->driver->set_speed(job, speed, &local_err); 521 550 if (local_err) { 522 551 error_propagate(errp, local_err); ··· 536 565 { 537 566 /* Should not be reachable via external interface for internal jobs */ 538 567 assert(job->id); 539 - if (job->pause_count || job->cancelled || 540 - !block_job_started(job) || !job->driver->complete) { 568 + if (block_job_apply_verb(job, BLOCK_JOB_VERB_COMPLETE, errp)) { 569 + return; 570 + } 571 + if (job->pause_count || job->cancelled || !job->driver->complete) { 541 572 error_setg(errp, "The active block job '%s' cannot be completed", 542 573 job->id); 543 574 return; ··· 546 577 job->driver->complete(job, errp); 547 578 } 548 579 549 - void block_job_user_pause(BlockJob *job) 580 + void block_job_user_pause(BlockJob *job, Error **errp) 550 581 { 582 + if (block_job_apply_verb(job, BLOCK_JOB_VERB_PAUSE, errp)) { 583 + return; 584 + } 585 + if (job->user_paused) { 586 + error_setg(errp, "Job is already paused"); 587 + return; 588 + } 551 589 job->user_paused = true; 552 590 block_job_pause(job); 553 591 } ··· 557 595 return job->user_paused; 558 596 } 559 597 560 - void block_job_user_resume(BlockJob *job) 598 + void block_job_user_resume(BlockJob *job, Error **errp) 561 599 { 562 - if (job && job->user_paused && job->pause_count > 0) { 563 - block_job_iostatus_reset(job); 564 - job->user_paused = false; 565 - block_job_resume(job); 600 + assert(job); 601 + if (!job->user_paused || job->pause_count <= 0) { 602 + error_setg(errp, "Can't resume a job that was not paused"); 603 + return; 566 604 } 605 + if (block_job_apply_verb(job, BLOCK_JOB_VERB_RESUME, errp)) { 606 + return; 607 + } 608 + block_job_iostatus_reset(job); 609 + job->user_paused = false; 610 + block_job_resume(job); 567 611 } 568 612 569 613 void block_job_cancel(BlockJob *job) ··· 574 618 } else { 575 619 block_job_completed(job, -ECANCELED); 576 620 } 621 + } 622 + 623 + void block_job_user_cancel(BlockJob *job, Error **errp) 624 + { 625 + if (block_job_apply_verb(job, BLOCK_JOB_VERB_CANCEL, errp)) { 626 + return; 627 + } 628 + block_job_cancel(job); 577 629 } 578 630 579 631 /* A wrapper around block_job_cancel() taking an Error ** parameter so it may be ··· 999 1051 action, &error_abort); 1000 1052 } 1001 1053 if (action == BLOCK_ERROR_ACTION_STOP) { 1054 + block_job_pause(job); 1002 1055 /* make the pause user visible, which will be resumed from QMP. */ 1003 - block_job_user_pause(job); 1056 + job->user_paused = true; 1004 1057 block_job_iostatus_set_err(job, error); 1005 1058 } 1006 1059 return action;
+11 -2
include/block/blockjob.h
··· 249 249 * Asynchronously pause the specified job. 250 250 * Do not allow a resume until a matching call to block_job_user_resume. 251 251 */ 252 - void block_job_user_pause(BlockJob *job); 252 + void block_job_user_pause(BlockJob *job, Error **errp); 253 253 254 254 /** 255 255 * block_job_paused: ··· 266 266 * Resume the specified job. 267 267 * Must be paired with a preceding block_job_user_pause. 268 268 */ 269 - void block_job_user_resume(BlockJob *job); 269 + void block_job_user_resume(BlockJob *job, Error **errp); 270 + 271 + /** 272 + * block_job_user_cancel: 273 + * @job: The job to be cancelled. 274 + * 275 + * Cancels the specified job, but may refuse to do so if the 276 + * operation isn't currently meaningful. 277 + */ 278 + void block_job_user_cancel(BlockJob *job, Error **errp); 270 279 271 280 /** 272 281 * block_job_cancel_sync:
+20
qapi/block-core.json
··· 959 959 'data': ['commit', 'stream', 'mirror', 'backup'] } 960 960 961 961 ## 962 + # @BlockJobVerb: 963 + # 964 + # Represents command verbs that can be applied to a blockjob. 965 + # 966 + # @cancel: see @block-job-cancel 967 + # 968 + # @pause: see @block-job-pause 969 + # 970 + # @resume: see @block-job-resume 971 + # 972 + # @set-speed: see @block-job-set-speed 973 + # 974 + # @complete: see @block-job-complete 975 + # 976 + # Since: 2.12 977 + ## 978 + { 'enum': 'BlockJobVerb', 979 + 'data': ['cancel', 'pause', 'resume', 'set-speed', 'complete' ] } 980 + 981 + ## 962 982 # @BlockJobStatus: 963 983 # 964 984 # Indicates the present state of a given blockjob in its lifetime.
+1
tests/test-bdrv-drain.c
··· 505 505 { 506 506 TestBlockJob *s = opaque; 507 507 508 + block_job_event_ready(&s->common); 508 509 while (!s->should_complete) { 509 510 block_job_sleep_ns(&s->common, 100000); 510 511 }