Re: [Qemu-devel] [PATCH v6 08/14] blockjob: Simplify block_job_finish_sync

[John Snow <jsnow@redhat.com>]

On 09/15/2015 02:11 AM, Fam Zheng wrote:
> With job->completed and job->ret to replace BlockFinishData.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
>  blockjob.c | 27 ++++++---------------------
>  1 file changed, 6 insertions(+), 21 deletions(-)
> 
> diff --git a/blockjob.c b/blockjob.c
> index 293b62a..36c18e0 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -185,43 +185,28 @@ struct BlockFinishData {
>      int ret;
>  };
>  
> -static void block_job_finish_cb(void *opaque, int ret)
> -{
> -    struct BlockFinishData *data = opaque;
> -
> -    data->cancelled = block_job_is_cancelled(data->job);
> -    data->ret = ret;
> -    data->cb(data->opaque, ret);
> -}
> -
>  static int block_job_finish_sync(BlockJob *job,
>                                   void (*finish)(BlockJob *, Error **errp),
>                                   Error **errp)
>  {
> -    struct BlockFinishData data;
>      BlockDriverState *bs = job->bs;
>      Error *local_err = NULL;
> +    int ret;
>  
>      assert(bs->job == job);
>  
> -    /* Set up our own callback to store the result and chain to
> -     * the original callback.
> -     */
> -    data.job = job;
> -    data.cb = job->cb;
> -    data.opaque = job->opaque;
> -    data.ret = -EINPROGRESS;
> -    job->cb = block_job_finish_cb;
> -    job->opaque = &data;
> +    block_job_ref(job);
>      finish(job, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
>          return -EBUSY;
>      }
> -    while (data.ret == -EINPROGRESS) {
> +    while (!job->completed) {
>          aio_poll(bdrv_get_aio_context(bs), true);
>      }
> -    return (data.cancelled && data.ret == 0) ? -ECANCELED : data.ret;
> +    ret = (job->cancelled && job->ret == 0) ? -ECANCELED : job->ret;
> +    block_job_unref(job);
> +    return ret;
>  }
>  
>  /* A wrapper around block_job_cancel() taking an Error ** parameter so it may be
> 

Reviewed-by: John Snow <jsnow@redhat.com>