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

[Eric Blake]

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>

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org