[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/7] jobs: change start callback to run callback
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PATCH 1/7] jobs: change start callback to run callback |
Date: |
Mon, 20 Aug 2018 15:04:23 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 |
On 08/20/2018 02:28 PM, Eric Blake wrote:
> On 08/17/2018 02:04 PM, 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>
>> ---
>
>> +++ b/block/backup.c
>> @@ -480,9 +480,9 @@ static void
>> backup_incremental_init_copy_bitmap(BackupBlockJob *job)
>> bdrv_dirty_iter_free(dbi);
>> }
>> -static void coroutine_fn backup_run(void *opaque)
>> +static int coroutine_fn backup_run(Job *opaque_job, Error **errp)
>> {
>> - BackupBlockJob *job = opaque;
>> + BackupBlockJob *job = container_of(opaque_job, BackupBlockJob,
>> common.job);
>
> Hmm. Here, you used the naming pair 'opaque_job' vs. 'job',...
>
>> +++ b/block/commit.c
>> @@ -134,9 +134,9 @@ static void commit_complete(Job *job, void *opaque)
>> bdrv_unref(top);
>> }
>> -static void coroutine_fn commit_run(void *opaque)
>> +static int coroutine_fn commit_run(Job *job, Error **errp)
>> {
>> - CommitBlockJob *s = opaque;
>> + CommitBlockJob *s = container_of(job, CommitBlockJob, common.job);
>
> while in the majority of the other clients, it was 'job' vs. 's'. Is it
> worth making these names consistent, or is that too much bikeshed paint?
>
> Reviewed-by: Eric Blake <address@hidden>
>
:)
Was just trying to keep the static down, but it did annoy me that it was
different.
I can either change it for "v2" or send a follow-up, depending.
--js
- Re: [Qemu-devel] [PATCH 3/7] jobs: add exit shim, (continued)
- Re: [Qemu-devel] [PATCH 3/7] jobs: add exit shim, John Snow, 2018/08/22
- Re: [Qemu-devel] [PATCH 3/7] jobs: add exit shim, Max Reitz, 2018/08/25
- Re: [Qemu-devel] [PATCH 3/7] jobs: add exit shim, John Snow, 2018/08/27
- Re: [Qemu-devel] [PATCH 3/7] jobs: add exit shim, Eric Blake, 2018/08/22
- Re: [Qemu-devel] [PATCH 3/7] jobs: add exit shim, John Snow, 2018/08/22
[Qemu-devel] [PATCH 1/7] jobs: change start callback to run callback, John Snow, 2018/08/17
Re: [Qemu-devel] [PATCH 0/7] jobs: remove job_defer_to_main_loop, no-reply, 2018/08/18
Re: [Qemu-devel] [PATCH 0/7] jobs: remove job_defer_to_main_loop, no-reply, 2018/08/20