On Sat, Oct 31 2015 at 11:33am -0400, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote: > > > On 29/10/2015 14:18, Mike Snitzer wrote: > > > 4) dmesg shows that scsi_verify_blk_ioctl() failed for SG_IO (0x2285); > > > it returns -ENOIOCTLCMD, later replaced with -ENOTTY in vfs_ioctl(). > > > > > > $ dmesg > > > <...> > > > [] device-mapper: multipath: Failing path 65:144. > > > [] device-mapper: multipath: Failing path 67:144. > > > [] device-mapper: multipath: Failing path 65:224. > > > [] device-mapper: multipath: Failing path 68:32. > > > [] sgio_inquiry: sending ioctl 2285 to a partition! > > > > So scsi_verify_blk_ioctl() considers the ioctl invalid. > > But that's wrong, I think. It's a false positive in > scsi_verify_blk_ioctl(). > > If the ioctl is valid when bdev becomes non-NULL (and it will be if > ti->len becomes equal to i_size_read(bdev->bd_inode) >> SECTOR_SHIFT), > you should not return -ENOIOCTLCMD aka ENOTTY, because userspace doesn't > think the ioctls can go away and come back. So Hannes's patch broke the > userspace ABI. :( Huh? All that Hannes' patch did was add early verification of the ioctl if there are no paths, since: there is no point queueing an ioctl that is invalid. But looking just now, Christoph's recent ioctl refactoring that I staged for 4.4 does seem to subtley revert Hannes' change: https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.4&id=40cf639be1db8cc2b8183fe2ccd390ca77b90396 With hch's change multipath_prepare_ioctl() will _not_ do early verification of the ioctl if no paths are available (and queue_if_no_path is configured). Because the call to scsi_verify_blk_ioctl() was moved to dm_blk_ioctl() and is only called if the return is > 0 (again -ENOTCONN is being returned). Not to mention hch's lifting of scsi_verify_blk_ioctl() into DM core's dm_blk_ioctl() -- likely motivated by not requiring all targets to do the call like they were doing -- should really be done as part of the new DM target .prepare_ioctl hook. Christoph, I think your DM ioctl changes need more review/work.. which implies they'll very likely miss 4.4.. sorry. Anyway, all DM specifics aside: The point is scsi_verify_blk_ioctl() is saying the ioctl isn't valid. It has nothing to do with the existance of a bdev or not; but everything to do with the unprivledged user's request to issue an ioctl. Paolo, AFAIK unprivledged ioctls is one of your pet-projects so your insight on what, if anything, needs changing to support them is the insight I think we need. -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
