[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 1/7] jobs: change start callback to run callback
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH 1/7] jobs: change start callback to run callback |
Date: |
Wed, 22 Aug 2018 12:51:09 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
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 <address@hidden>
> ---
> 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
- Re: [Qemu-block] [Qemu-devel] [PATCH 3/7] jobs: add exit shim, (continued)
- Re: [Qemu-block] [Qemu-devel] [PATCH 3/7] jobs: add exit shim, Max Reitz, 2018/08/25
- Re: [Qemu-block] [PATCH 3/7] jobs: add exit shim, John Snow, 2018/08/22
- Re: [Qemu-block] [PATCH 3/7] jobs: add exit shim, Max Reitz, 2018/08/25
- Re: [Qemu-block] [PATCH 3/7] jobs: add exit shim, John Snow, 2018/08/27
- Re: [Qemu-block] [PATCH 3/7] jobs: add exit shim, Max Reitz, 2018/08/29
- Re: [Qemu-block] [Qemu-devel] [PATCH 3/7] jobs: add exit shim, Eric Blake, 2018/08/22
- Re: [Qemu-block] [Qemu-devel] [PATCH 3/7] jobs: add exit shim, John Snow, 2018/08/22
[Qemu-block] [PATCH 1/7] jobs: change start callback to run callback, John Snow, 2018/08/17
Re: [Qemu-block] [Qemu-devel] [PATCH 0/7] jobs: remove job_defer_to_main_loop, no-reply, 2018/08/18
Re: [Qemu-block] [Qemu-devel] [PATCH 0/7] jobs: remove job_defer_to_main_loop, no-reply, 2018/08/20