On 16/10/2016 12:02, Stefan Hajnoczi wrote:
> On Thu, Oct 13, 2016 at 07:34:06PM +0200, Paolo Bonzini wrote:
>> +static void backup_drain(BlockJob *job)
>> +{
>> + BackupBlockJob *s = container_of(job, BackupBlockJob, common);
>> +
>> + /* Need to keep a reference in case blk_drain triggers execution
>> + * of backup_complete...
>> + */
>> + if (s->target) {
>> + blk_ref(s->target);
>> + blk_drain(s->target);
>> + blk_unref(s->target);
>> + }
> [...]
>> @@ -331,6 +346,7 @@ static void backup_complete(BlockJob *job, void *opaque)
>> BackupCompleteData *data = opaque;
>>
>> blk_unref(s->target);
>> + s->target = NULL;
>
> Will blk_unref(s->target) segfault since backup_complete() has set it to
> NULL? I expected backup_drain() to stash the pointer in a local
> variable to avoid using s->target.
Yes, indeed.
Paolo
