Re: [Qemu-devel] [PATCH v6 05/14] blockjob: Introduce reference count

[John Snow]

On 09/15/2015 02:11 AM, Fam Zheng wrote:
> So that block_job_complete_sync can be simplified.
> 
> Reviewed-by: Max Reitz <address@hidden>
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>  block/mirror.c           |  2 +-
>  blockjob.c               | 22 ++++++++++++++--------
>  include/block/blockjob.h | 18 +++++++++++++++---
>  3 files changed, 30 insertions(+), 12 deletions(-)
> 
> diff --git a/block/mirror.c b/block/mirror.c
> index a258926..3472ad4 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -736,7 +736,7 @@ static void mirror_start_job(BlockDriverState *bs, 
> BlockDriverState *target,
>      s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
>      if (!s->dirty_bitmap) {
>          g_free(s->replaces);
> -        block_job_release(bs);
> +        block_job_unref(&s->common);
>          return;
>      }
>      bdrv_set_enable_write_cache(s->target, true);
> diff --git a/blockjob.c b/blockjob.c
> index 62bb906..ec12887 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -58,6 +58,7 @@ void *block_job_create(const BlockJobDriver *driver, 
> BlockDriverState *bs,
>      job->cb            = cb;
>      job->opaque        = opaque;
>      job->busy          = true;
> +    job->refcnt        = 1;
>      bs->job = job;
>  
>      /* Only set speed when necessary to avoid NotSupported error */
> @@ -66,7 +67,7 @@ void *block_job_create(const BlockJobDriver *driver, 
> BlockDriverState *bs,
>  
>          block_job_set_speed(job, speed, &local_err);
>          if (local_err) {
> -            block_job_release(bs);
> +            block_job_unref(job);
>              error_propagate(errp, local_err);
>              return NULL;
>          }
> @@ -74,14 +75,19 @@ void *block_job_create(const BlockJobDriver *driver, 
> BlockDriverState *bs,
>      return job;
>  }
>  
> -void block_job_release(BlockDriverState *bs)
> +void block_job_ref(BlockJob *job)
>  {
> -    BlockJob *job = bs->job;
> +    ++job->refcnt;
> +}
>  
> -    bs->job = NULL;
> -    bdrv_op_unblock_all(bs, job->blocker);
> -    error_free(job->blocker);
> -    g_free(job);
> +void block_job_unref(BlockJob *job)
> +{
> +    if (--job->refcnt == 0) {
> +        job->bs->job = NULL;
> +        bdrv_op_unblock_all(job->bs, job->blocker);
> +        error_free(job->blocker);
> +        g_free(job);
> +    }
>  }
>  
>  void block_job_completed(BlockJob *job, int ret)
> @@ -90,7 +96,7 @@ void block_job_completed(BlockJob *job, int ret)
>  
>      assert(bs->job == job);
>      job->cb(job->opaque, ret);
> -    block_job_release(bs);
> +    block_job_unref(job);
>  }
>  
>  void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
> diff --git a/include/block/blockjob.h b/include/block/blockjob.h
> index dd9d5e6..3e7ad21 100644
> --- a/include/block/blockjob.h
> +++ b/include/block/blockjob.h
> @@ -122,6 +122,9 @@ struct BlockJob {
>  
>      /** The opaque value that is passed to the completion function.  */
>      void *opaque;
> +
> +    /** Reference count of the block job */
> +    int refcnt;
>  };
>  
>  /**
> @@ -166,12 +169,21 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType 
> type, int64_t ns);
>  void block_job_yield(BlockJob *job);
>  
>  /**
> - * block_job_release:
> + * block_job_ref:
>   * @bs: The block device.
>   *
> - * Release job resources when an error occurred or job completed.
> + * Grab a reference to the block job. Should be paired with block_job_unref.
>   */
> -void block_job_release(BlockDriverState *bs);
> +void block_job_ref(BlockJob *job);
> +
> +/**
> + * block_job_unref:
> + * @bs: The block device.
> + *
> + * Release reference to the block job and release resources if it is the last
> + * reference.
> + */
> +void block_job_unref(BlockJob *job);
>  
>  /**
>   * block_job_completed:
> 

Reviewed-by: John Snow <address@hidden>