On 2018-08-17 21:04, John Snow wrote: > Presently we codify the entry point for a job as the "start" callback, > but a more apt name would be "run" to clarify the idea that when this > function returns we consider the job to have "finished," except for > any cleanup which occurs in separate callbacks later. > > As part of this clarification, change the signature to include an error > object and a return code. The error ptr is not yet used, and the return > code while captured, will be overwritten by actions in the job_completed > function. > > Signed-off-by: John Snow <js...@redhat.com> > --- > block/backup.c | 7 ++++--- > block/commit.c | 7 ++++--- > block/create.c | 8 +++++--- > block/mirror.c | 10 ++++++---- > block/stream.c | 7 ++++--- > include/qemu/job.h | 2 +- > job.c | 6 +++--- > tests/test-bdrv-drain.c | 7 ++++--- > tests/test-blockjob-txn.c | 16 ++++++++-------- > tests/test-blockjob.c | 7 ++++--- > 10 files changed, 43 insertions(+), 34 deletions(-)
[...]
> diff --git a/job.c b/job.c
> index fa671b431a..898260b2b3 100644
> --- a/job.c
> +++ b/job.c
> @@ -544,16 +544,16 @@ static void coroutine_fn job_co_entry(void *opaque)
> {
> Job *job = opaque;
>
> - assert(job && job->driver && job->driver->start);
> + assert(job && job->driver && job->driver->run);
> job_pause_point(job);
> - job->driver->start(job);
> + job->ret = job->driver->run(job, NULL);
> }
Hmmm, this breaks the iff relationship with job->error. We might call
job_update_rc() afterwards, but then job_completed() would need to free
it if it overwrites it with the error description from a potential error
object.
Also, I suspect the following patches might fix the relationship anyway?
(But then an "XXX: This does not hold right now, but will be fixed in a
future patch" in the documentation of Job.error might help.)
Max
signature.asc
Description: OpenPGP digital signature
