Re: [Qemu-devel] [PATCH 1/2] blockjob: assert(cb) in the entry functions of blockjob

[Changlong Xie <xiecl.fnst@cn.fujitsu.com>]

On 06/22/2016 05:50 PM, Paolo Bonzini wrote:
>
>
> On 22/06/2016 11:16, Changlong Xie wrote:
>> Signed-off-by: Changlong Xie <xiecl.fnst@cn.fujitsu.com>
>> ---
>>   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;
>>
>
>
>