[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/9] jobs: canonize Error object
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/9] jobs: canonize Error object |
Date: |
Mon, 27 Aug 2018 12:43:09 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 2018-08-27 11:41, Max Reitz wrote:
> 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 <address@hidden>
>> ---
>> 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 can't read" confirmed
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v2 0/9] jobs: Job Exit Refactoring Pt 1, John Snow, 2018/08/23
- [Qemu-devel] [PATCH v2 8/9] jobs: remove ret argument to job_completed; privatize it, John Snow, 2018/08/23
- [Qemu-devel] [PATCH v2 2/9] jobs: canonize Error object, John Snow, 2018/08/23
- [Qemu-devel] [PATCH v2 7/9] block/backup: make function variables consistently named, John Snow, 2018/08/23
- [Qemu-devel] [PATCH v2 5/9] block/mirror: utilize job_exit shim, John Snow, 2018/08/23
- [Qemu-devel] [PATCH v2 6/9] jobs: utilize job_exit shim, John Snow, 2018/08/23
- [Qemu-devel] [PATCH v2 9/9] jobs: remove job_defer_to_main_loop, John Snow, 2018/08/23
- [Qemu-devel] [PATCH v2 4/9] block/commit: utilize job_exit shim, John Snow, 2018/08/23