From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45162) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZBwDT-0000Nb-Pk for qemu-devel@nongnu.org; Sun, 05 Jul 2015 22:33:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZBwDP-00032w-Oa for qemu-devel@nongnu.org; Sun, 05 Jul 2015 22:33:07 -0400 Received: from szxga01-in.huawei.com ([58.251.152.64]:62364) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZBwDP-00031P-4Y for qemu-devel@nongnu.org; Sun, 05 Jul 2015 22:33:03 -0400 Message-ID: <5599E8CC.8000001@huawei.com> Date: Mon, 6 Jul 2015 10:32:44 +0800 From: Ting Wang MIME-Version: 1.0 References: <1435311455-56048-1-git-send-email-kathy.wangting@huawei.com> In-Reply-To: <1435311455-56048-1-git-send-email-kathy.wangting@huawei.com> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] blockjob: add block_job_release function List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: jcody@redhat.com, kwolf@redhat.com Cc: famz@redhat.com, qemu-devel@nongnu.org, wu.wubin@huawei.com Ping? Regards, Ting On 2015-6-26 17:37, Ting Wang wrote: > There is job resource leak in function mirror_start_job, > although bdrv_create_dirty_bitmap is unlikely failed. > Add block_job_release for each release when needed. > > Signed-off-by: Ting Wang > --- > block/mirror.c | 2 ++ > blockjob.c | 20 ++++++++++++-------- > include/block/blockjob.h | 8 ++++++++ > 3 files changed, 22 insertions(+), 8 deletions(-) > > diff --git a/block/mirror.c b/block/mirror.c > index 048e452..05034a8 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -689,6 +689,8 @@ 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); > return; > } > bdrv_set_enable_write_cache(s->target, true); > diff --git a/blockjob.c b/blockjob.c > index ec46fad..62bb906 100644 > --- a/blockjob.c > +++ b/blockjob.c > @@ -66,10 +66,7 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs, > > block_job_set_speed(job, speed, &local_err); > if (local_err) { > - bs->job = NULL; > - bdrv_op_unblock_all(bs, job->blocker); > - error_free(job->blocker); > - g_free(job); > + block_job_release(bs); > error_propagate(errp, local_err); > return NULL; > } > @@ -77,18 +74,25 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs, > return job; > } > > -void block_job_completed(BlockJob *job, int ret) > +void block_job_release(BlockDriverState *bs) > { > - BlockDriverState *bs = job->bs; > + BlockJob *job = bs->job; > > - assert(bs->job == job); > - job->cb(job->opaque, ret); > bs->job = NULL; > bdrv_op_unblock_all(bs, job->blocker); > error_free(job->blocker); > g_free(job); > } > > +void block_job_completed(BlockJob *job, int ret) > +{ > + BlockDriverState *bs = job->bs; > + > + assert(bs->job == job); > + job->cb(job->opaque, ret); > + block_job_release(bs); > +} > + > void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp) > { > Error *local_err = NULL; > diff --git a/include/block/blockjob.h b/include/block/blockjob.h > index 57d8ef1..dd9d5e6 100644 > --- a/include/block/blockjob.h > +++ b/include/block/blockjob.h > @@ -166,6 +166,14 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns); > void block_job_yield(BlockJob *job); > > /** > + * block_job_release: > + * @bs: The block device. > + * > + * Release job resources when an error occurred or job completed. > + */ > +void block_job_release(BlockDriverState *bs); > + > +/** > * block_job_completed: > * @job: The job being completed. > * @ret: The status code.