On 2018-08-24 00:08, John Snow wrote: > Jobs presently use both an Error object in the case of the create job, > and char strings in the case of generic errors elsewhere. > > Unify the two paths as just j->err, and remove the extra argument from > job_completed. The integer error code for job_completed is kept for now, > to be removed shortly in a separate patch. > > Signed-off-by: John Snow <[email protected]> > --- > block/backup.c | 2 +- > block/commit.c | 2 +- > block/create.c | 5 ++--- > block/mirror.c | 2 +- > block/stream.c | 2 +- > include/qemu/job.h | 10 ++++------ > job-qmp.c | 5 +++-- > job.c | 18 ++++++------------ > tests/test-bdrv-drain.c | 2 +- > tests/test-blockjob-txn.c | 2 +- > tests/test-blockjob.c | 2 +- > 11 files changed, 22 insertions(+), 30 deletions(-)
[...]
> diff --git a/include/qemu/job.h b/include/qemu/job.h
> index 9cf463d228..5c92c53ef0 100644
> --- a/include/qemu/job.h
> +++ b/include/qemu/job.h
> @@ -124,12 +124,12 @@ typedef struct Job {
> /** Estimated progress_current value at the completion of the job */
> int64_t progress_total;
>
> - /** Error string for a failed job (NULL if, and only if, job->ret == 0)
> */
> - char *error;
> -
> /** ret code passed to job_completed. */
> int ret;
>
> + /** Error object for a failed job **/
> + Error *err;
> +
My question remains why you don't keep the iff condition here...
> /** The completion function that will be called when the job completes.
> */
> BlockCompletionFunc *cb;
>
[...]
> diff --git a/job.c b/job.c
> index 76988f6678..bc1d970df4 100644
> --- a/job.c
> +++ b/job.c
[...]
> @@ -546,7 +546,7 @@ static void coroutine_fn job_co_entry(void *opaque)
>
> assert(job && job->driver && job->driver->run);
> job_pause_point(job);
> - job->ret = job->driver->run(job, NULL);
> + job->ret = job->driver->run(job, &job->err);
...by e.g. calling job_update_rc() here.
(Which seems reasonable since this did update the return code.)
Rest looks good, although I'm missing a "jobs: remove @ret from
job_completed" patch in one of the two series...
Max
signature.asc
Description: OpenPGP digital signature
