Re: [Qemu-block] [PATCH v10 08/10] Implement new driver for block replication

[Wen Congyang]

On 10/13/2015 12:25 AM, Stefan Hajnoczi wrote:
> On Fri, Sep 25, 2015 at 02:17:36PM +0800, Wen Congyang wrote:
>> +static void backup_job_completed(void *opaque, int ret)
>> +{
>> +    BDRVReplicationState *s = opaque;
>> +
>> +    if (s->replication_state != BLOCK_REPLICATION_DONE) {
>> +        /* The backup job is cancelled unexpectedly */
>> +        s->error = -EIO;
>> +    }
>> +
>> +    bdrv_op_block(s->hidden_disk, BLOCK_OP_TYPE_BACKUP_TARGET,
>> +                  s->active_disk->backing_blocker);
>> +    bdrv_op_block(s->secondary_disk, BLOCK_OP_TYPE_BACKUP_SOURCE,
>> +                  s->hidden_disk->backing_blocker);
>> +
>> +    bdrv_put_ref_bh_schedule(s->secondary_disk);
> 
> Why is bdrv_put_ref_bh_schedule() necessary?

It is copied from block_job_cb(). According to the comments in 
bdrv_put_ref_bh_schedule():
/*
 * Release a BDS reference in a BH
 *
 * It is not safe to use bdrv_unref() from a callback function when the callers
 * still need the BlockDriverState.  In such cases we schedule a BH to release
 * the reference.
 */

If the comment is right, I think it is necessary to call 
bdrv_put_ref_bh_schedule() here.
Because the job is created on the BDS s->secondary disk, backup_job_completed() 
is
called in block_job_completed(), and we will still use s->secondary_disk in 
block_job_release().

Thanks
Wen Congyang

> .
>