Re: [Qemu-devel] [PATCH 1/7] jobs: change start callback to run callback

[John Snow]

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