From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44743) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bFf4e-0001yM-1M for qemu-devel@nongnu.org; Wed, 22 Jun 2016 06:07:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bFf4Z-0001gz-Ri for qemu-devel@nongnu.org; Wed, 22 Jun 2016 06:07:55 -0400 Received: from [59.151.112.132] (port=33968 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bFf4Z-0001g7-6W for qemu-devel@nongnu.org; Wed, 22 Jun 2016 06:07:51 -0400 Message-ID: <576A6486.4070409@cn.fujitsu.com> Date: Wed, 22 Jun 2016 18:12:22 +0800 From: Changlong Xie MIME-Version: 1.0 References: <1466587008-3933-1-git-send-email-xiecl.fnst@cn.fujitsu.com> <1466587008-3933-2-git-send-email-xiecl.fnst@cn.fujitsu.com> <2b17fa8d-d2bb-d20e-2445-e010aa7066b1@redhat.com> In-Reply-To: <2b17fa8d-d2bb-d20e-2445-e010aa7066b1@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] blockjob: assert(cb) in the entry functions of blockjob List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu devel , Jeff Cody Cc: Kevin Wolf , Fam Zheng , Max Reitz , Stefan Hajnoczi On 06/22/2016 05:50 PM, Paolo Bonzini wrote: > > > On 22/06/2016 11:16, Changlong Xie wrote: >> Signed-off-by: Changlong Xie >> --- >> block/commit.c | 1 + >> block/mirror.c | 2 ++ >> block/stream.c | 1 + >> 3 files changed, 4 insertions(+) > > Why is this useful? > commit/mirror/stream/backup use block_job_create(..., cb,..) to create relevant blockjob. When they finished, these jobs will invoke block_job_completed, then invoke job->cb() unconditionally. So i think we need this to avoid segment fault. Actually backup has implemented this. Thanks -Xie > Paolo > >> diff --git a/block/commit.c b/block/commit.c >> index 444333b..13b55c1 100644 >> --- a/block/commit.c >> +++ b/block/commit.c >> @@ -223,6 +223,7 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base, >> BlockDriverState *overlay_bs; >> Error *local_err = NULL; >> >> + assert(cb); >> assert(top != bs); >> if (top == base) { >> error_setg(errp, "Invalid files for merge: top and base are the same"); >> diff --git a/block/mirror.c b/block/mirror.c >> index a04ed9c..fa2bdab 100644 >> --- a/block/mirror.c >> +++ b/block/mirror.c >> @@ -912,6 +912,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target, >> bool is_none_mode; >> BlockDriverState *base; >> >> + assert(cb); >> if (mode == MIRROR_SYNC_MODE_INCREMENTAL) { >> error_setg(errp, "Sync mode 'incremental' not supported"); >> return; >> @@ -935,6 +936,7 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base, >> int ret; >> Error *local_err = NULL; >> >> + assert(cb); >> orig_base_flags = bdrv_get_flags(base); >> >> if (bdrv_reopen(base, bs->open_flags, errp)) { >> diff --git a/block/stream.c b/block/stream.c >> index c0efbda..fc34c63 100644 >> --- a/block/stream.c >> +++ b/block/stream.c >> @@ -226,6 +226,7 @@ void stream_start(BlockDriverState *bs, BlockDriverState *base, >> { >> StreamBlockJob *s; >> >> + assert(cb); >> s = block_job_create(&stream_job_driver, bs, speed, cb, opaque, errp); >> if (!s) { >> return; >> > > >