From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37869) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ze9Qw-0000FW-Dh for qemu-devel@nongnu.org; Mon, 21 Sep 2015 18:19:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ze9Qv-0002Qd-CK for qemu-devel@nongnu.org; Mon, 21 Sep 2015 18:19:38 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37929) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ze9Qu-0002QN-W8 for qemu-devel@nongnu.org; Mon, 21 Sep 2015 18:19:37 -0400 References: <1442297513-7001-1-git-send-email-famz@redhat.com> <1442297513-7001-6-git-send-email-famz@redhat.com> From: John Snow Message-ID: <56008276.2010908@redhat.com> Date: Mon, 21 Sep 2015 18:19:34 -0400 MIME-Version: 1.0 In-Reply-To: <1442297513-7001-6-git-send-email-famz@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v6 05/14] blockjob: Introduce reference count List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: Kevin Wolf , Jeff Cody , vsementsov@parallels.com, stefanha@redhat.com, Max Reitz On 09/15/2015 02:11 AM, Fam Zheng wrote: > So that block_job_complete_sync can be simplified. > > Reviewed-by: Max Reitz > Signed-off-by: Fam Zheng > --- > 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