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 <js...@redhat.com>
>> ---
>
>> +++ 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 <ebl...@redhat.com>
>
:)
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
