Am 27.01.2018 um 03:05 hat John Snow geschrieben: > For jobs that have reached their terminal state, prior to having their > last reference put down (meaning jobs that have completed successfully, > unsuccessfully, or have been canceled), allow the user to dismiss the > job's lingering status report via block-job-dismiss. > > This gives management APIs the chance to conclusively determine if a job > failed or succeeded, even if the event broadcast was missed. > > Note that jobs do not yet linger in any such state, they are freed > immediately upon reaching this previously-unnamed state. such a state is > added immediately in the next commit. > > Valid objects: > Concluded: (added in a future commit); dismisses the concluded job. > > Signed-off-by: John Snow <js...@redhat.com> > --- > block/trace-events | 1 + > blockdev.c | 14 ++++++++++++++ > blockjob.c | 30 ++++++++++++++++++++++++++++++ > include/block/blockjob.h | 9 +++++++++ > qapi/block-core.json | 19 +++++++++++++++++++ > 5 files changed, 73 insertions(+) > > diff --git a/block/trace-events b/block/trace-events > index 11c8d5f590..8f61566770 100644 > --- a/block/trace-events > +++ b/block/trace-events > @@ -46,6 +46,7 @@ qmp_block_job_cancel(void *job) "job %p" > qmp_block_job_pause(void *job) "job %p" > qmp_block_job_resume(void *job) "job %p" > qmp_block_job_complete(void *job) "job %p" > +qmp_block_job_dismiss(void *job) "job %p" > qmp_block_stream(void *bs, void *job) "bs %p job %p" > > # block/file-win32.c > diff --git a/blockdev.c b/blockdev.c > index 2c0773bba7..5e8edff322 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -3849,6 +3849,20 @@ void qmp_block_job_complete(const char *device, Error > **errp) > aio_context_release(aio_context); > } > > +void qmp_block_job_dismiss(const char *id, Error **errp) > +{ > + AioContext *aio_context; > + BlockJob *job = find_block_job(id, &aio_context, errp); > + > + if (!job) { > + return; > + } > + > + trace_qmp_block_job_dismiss(job); > + block_job_dismiss(&job, errp); > + aio_context_release(aio_context); > +} > + > void qmp_change_backing_file(const char *device, > const char *image_node_name, > const char *backing_file, > diff --git a/blockjob.c b/blockjob.c > index ea216aca5e..5531f5c2ab 100644 > --- a/blockjob.c > +++ b/blockjob.c > @@ -58,6 +58,7 @@ enum BlockJobVerb { > BLOCK_JOB_VERB_RESUME, > BLOCK_JOB_VERB_SET_SPEED, > BLOCK_JOB_VERB_COMPLETE, > + BLOCK_JOB_VERB_DISMISS, > BLOCK_JOB_VERB__MAX > }; > > @@ -68,6 +69,7 @@ bool > BlockJobVerb[BLOCK_JOB_VERB__MAX][BLOCK_JOB_STATUS__MAX] = { > [BLOCK_JOB_VERB_RESUME] = {0, 0, 0, 1, 0}, > [BLOCK_JOB_VERB_SET_SPEED] = {0, 1, 1, 1, 1}, > [BLOCK_JOB_VERB_COMPLETE] = {0, 0, 0, 0, 1}, > + [BLOCK_JOB_VERB_DISMISS] = {0, 0, 0, 0, 0},
This makes it very obvious that the patches should probably be reordered. > }; > > static void block_job_state_transition(BlockJob *job, BlockJobStatus s1) > @@ -426,6 +428,13 @@ static void block_job_cancel_async(BlockJob *job) > job->cancelled = true; > } > > +static void block_job_do_dismiss(BlockJob *job) > +{ > + assert(job && job->manual == true); > + block_job_state_transition(job, BLOCK_JOB_STATUS_UNDEFINED); I don't think you made that an allowed transition from anywhere? > + block_job_unref(job); > +} > + > static int block_job_finish_sync(BlockJob *job, > void (*finish)(BlockJob *, Error **errp), > Error **errp) > @@ -455,6 +464,9 @@ static int block_job_finish_sync(BlockJob *job, > aio_poll(qemu_get_aio_context(), true); > } > ret = (job->cancelled && job->ret == 0) ? -ECANCELED : job->ret; > + if (job->manual) { > + block_job_do_dismiss(job); > + } > block_job_unref(job); > return ret; > } > @@ -570,6 +582,24 @@ void block_job_complete(BlockJob *job, Error **errp) > job->driver->complete(job, errp); > } > > +void block_job_dismiss(BlockJob **jobptr, Error **errp) > +{ > + BlockJob *job = *jobptr; > + /* similarly to _complete, this is QMP-interface only. */ > + assert(job->id); > + if (!job->manual) { > + error_setg(errp, "The active block job '%s' was not started with " > + "\'manual\': true, and so cannot be dismissed as it will " > + "clean up after itself automatically", job->id); > + return; > + } If you check instead that the job is in the right state to be dismissed (CONCLUDED), the job->manual check wouldn't be needed any more because the user can never catch an automatically completed job in CONCLUDED. > + error_setg(errp, "unimplemented"); This should hopefully go away when you reorder the patches. > + block_job_do_dismiss(job); > + *jobptr = NULL; > +} Kevin