On 08/22/2018 06:51 AM, Max Reitz wrote:
> 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
>
Hmm... does it? ... I guess you mean that we are setting job->ret
earlier than we used to, which gives us a window where you can have ret
set, but error unset.
This will get settled out by the end of the series anyway:
- char *error gets replaced with Error *err,
- I remove the error object from job_completed
- v2 will remove the ret argument, too.
--js
