On 9/30/22 04:57, Mike Christie wrote:
On 9/23/22 7:02 PM, Uday Shankar wrote:
---
Looking for feedback on the "allowed source states" list. The bug I hit
is solved by prohibiting transitions out of SDEV_BLOCKED, but I think
most others shouldn't be allowed either.
I think it's ok to be restrictive:
1. BLOCKED is just broken. When the transport classes and scsi_lib transition
out of that state they do a lot more than just set the set. We are leaving
the kernel in mismatched state where the device state is running but the
block layerand transport classes are not ready for IO.
2. CREATED does not happen. We go into RUNNING then do scsi_sysfs_add_sdev so
userspace should not see the CREATED state.
3. I'm not 100% sure about SDEV_QUIESCE though. It looks like it has similar issues
as BLOCKED where scsi_device_resume will undo things it did in scsi_device_quiesce,
so we can't just set the state to RUNNING and expect things to work. I'm not
sure about the scsi_transport_spi cases.
It would be best for James or Hannes to comment because they know that code well.
Indeed, you are correct.
The only sensible state transitions to be modified from userland is
indeed between SDEV_RUNNING and SDEV_OFFLINE.
All other states are in fact part of the SCSI midlayer operations, and
there a quite strict rules on which state transitions are allowed and
who should be initiating them.
So yes, I'm fine with this patch.
Reviewed-by: Hannes Reinecke <hare@xxxxxxx>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman
