On Fri, 2019-06-07 at 07:52 +0200, Hannes Reinecke wrote: > On 6/6/19 7:26 PM, Bart Van Assche wrote: > > On 6/5/19 10:46 PM, Hannes Reinecke wrote: > > > Why not simply '-EPERM' ? > > > Translating a state into something else seems counterproductive. > > > > Personally I'm OK with returning -EPERM for attempts from user space to > > change the device state into SDEV_BLOCK. The state translation is > > something that was proposed about two months ago (see also > > https://www.mail-archive.com/linux-scsi@vger.kernel.org/msg82610.html). > > > > > And, while we're at it: > > > The only meaningful states to be set from userspace are 'RUNNING', > > > 'OFFLINE', and 'BLOCK'. > > > Everything else relates to the internal state machine and really > > > shouldn't be touched from userspace at all. > > > So I'd rather filter out everything _but_ the three states, avoid > > > similar issue in the future. > > > > Can you please clarify why you think it is useful to change the SCSI > > device state from user space into SDEV_BLOCK? As one can see in > > scsi_device_set_state() transitions from SDEV_BLOCK into the following > > states are allowed: SDEV_RUNNING, SDEV_OFFLINE, SDEV_TRANSPORT_OFFLINE > > and SDEV_DEL. The mpt3sas driver and also the FC, iSCSI and SRP > > transport drivers all can call scsi_internal_device_unblock_nowait() or > > scsi_target_unblock(). So at least for this LLD and these transport > > drivers if the device state is set to SDEV_BLOCK from user space that > > change can be overridden any time by kernel code. So I'm not sure it is > > useful to change the device state into SDEV_BLOCK from user space. > > > > Yes, I agree. > > So let's restrict userspace to only ever setting 'RUNNING' or 'OFFLINE'. > None of the other states make sense to set from userspace. > > Cheers, > > Hannes
I agree. -Ewan
