Re: [Qemu-block] [Qemu-devel] [PATCH 2/2] scsi: add block job opblockers for scsi-block

[Paolo Bonzini]

On 08/02/2018 02:35, Fam Zheng wrote:
> On Wed, 02/07 17:36, Paolo Bonzini wrote:
>> @@ -2626,6 +2656,36 @@ static void scsi_block_realize(SCSIDevice *dev, Error 
>> **errp)
>>  
>>      scsi_realize(&s->qdev, errp);
>>      scsi_generic_read_device_identification(&s->qdev);
>> +
>> +    /* For op blockers, due to lack of support for dirty bitmaps.  */
>> +    error_setg(&sb->mirror_source,
>> +               "scsi-block does not support acting as a mirroring source");
>> +    error_setg(&sb->commit_source,
>> +               "scsi-block does not support acting as an active commit 
>> source");
> 
> An alternative way would be adding BLOCK_OP_TYPE_DIRTY_BITMAP. The error 
> message
> will not be as nice but it can be useful for another (blockjob) operation that
> requires dirty bitmap support, or another device that doesn't support dirty
> bitmaps. Though there isn't one for now.

Yeah, I thought about it.  Another possibility is make BLOCK_OP_TYPE_* a
bitmask.  Then you can easily add a single Error * for multiple
blockers, and BLOCK_OP_TYPE_DIRTY_BITMAP can be defined as
BLOCK_OP_TYPE_MIRROR_SOURCE|BLOCK_OP_TYPE_COMMIT_SOURCE; likewise for
notifiers below.

Paolo

>> +
>> +    /* For op blockers, due to lack of support for write notifiers.  */
>> +    error_setg(&sb->backup_source,
>> +               "scsi-block does not support acting as a backup source");
>> +
>> +    sb->insert_bs.notify = scsi_block_insert_bs;
>> +    blk_add_insert_bs_notifier(s->qdev.conf.blk, &sb->insert_bs);
>> +    sb->remove_bs.notify = scsi_block_remove_bs;
>> +    blk_add_remove_bs_notifier(s->qdev.conf.blk, &sb->remove_bs);
>> +
>> +    scsi_block_insert_bs(&sb->insert_bs, s->qdev.conf.blk);
>> +}
>> +
>> +static void scsi_block_unrealize(SCSIDevice *dev, Error **errp)
>> +{
>> +    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
>> +    SCSIBlockState *sb = DO_UPCAST(SCSIBlockState, sd, s);
>> +
>> +    notifier_remove(&sb->insert_bs);
>> +    notifier_remove(&sb->remove_bs);
>> +    scsi_block_remove_bs(&sb->insert_bs, s->qdev.conf.blk);
>> +    error_free(sb->mirror_source);
>> +    error_free(sb->commit_source);
>> +    error_free(sb->backup_source);
>>  }
>>  
>>  typedef struct SCSIBlockReq {
>> @@ -3017,6 +3077,7 @@ static void scsi_block_class_initfn(ObjectClass 
>> *klass, void *data)
>>      SCSIDiskClass *sdc = SCSI_DISK_BASE_CLASS(klass);
>>  
>>      sc->realize      = scsi_block_realize;
>> +    sc->unrealize    = scsi_block_unrealize;
>>      sc->alloc_req    = scsi_block_new_request;
>>      sc->parse_cdb    = scsi_block_parse_cdb;
>>      sdc->dma_readv   = scsi_block_dma_readv;
>> @@ -3031,6 +3092,7 @@ static const TypeInfo scsi_block_info = {
>>      .name          = "scsi-block",
>>      .parent        = TYPE_SCSI_DISK_BASE,
>>      .class_init    = scsi_block_class_initfn,
>> +    .instance_size = sizeof(SCSIBlockState),
>>  };
>>  #endif
>>  
>> diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
>> index c4e52a5fa3..a48a49ca79 100644
>> --- a/include/sysemu/block-backend.h
>> +++ b/include/sysemu/block-backend.h
>> @@ -182,6 +182,7 @@ void blk_set_guest_block_size(BlockBackend *blk, int 
>> align);
>>  void *blk_try_blockalign(BlockBackend *blk, size_t size);
>>  void *blk_blockalign(BlockBackend *blk, size_t size);
>>  bool blk_op_is_blocked(BlockBackend *blk, BlockOpType op, Error **errp);
>> +void blk_op_block(BlockBackend *blk, BlockOpType op, Error *reason);
>>  void blk_op_unblock(BlockBackend *blk, BlockOpType op, Error *reason);
>>  void blk_op_block_all(BlockBackend *blk, Error *reason);
>>  void blk_op_unblock_all(BlockBackend *blk, Error *reason);
>> -- 
>> 2.14.3
>>
>>
> 
> Fam
>