On 12/02/2018 14:52, Kevin Wolf wrote: > 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 agree. But does this include not fixing bugs wherever clients are using the old op blockers? Paolo > 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.