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

[Kevin Wolf]

Am 08.02.2018 um 11:42 hat Paolo Bonzini geschrieben:
> 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.

We shouldn't be adding new instances of BLOCK_OP_* at all. I couldn't
find the time yet to remove the existing ones, but any new protections
should be using the permission system.

I propose a new BLK_PERM_BYPASS that allows its users to bypass the
block layer I/O functions. In other words, bdrv_aio_ioctl() would
require that you got this permission. A dirty bitmap would keep a
BdrvChild with perm=0, shared=BLK_PERM_ALL & ~BLK_PERM_BYPASS, so you
can never have a dirty bitmap and a device using ioctls attached to the
BDS at the same time.

Kevin