On Wed, 2016-12-07 at 15:30 -0500, Ewan D. Milne wrote:
> On Wed, 2016-12-07 at 12:09 -0800, James Bottomley wrote:
> > Hm, it looks like the state set in scsi_sysfs_add_sdev() is bogus.
> > We expect the state to have been properly set before that (in
> > scsi_add_lun), so can we not simply remove it?
> >
> > James
> >
>
> I was considering that, but...
>
> enum scsi_device_state {
> SDEV_CREATED = 1, /* device created but not added to
> sysfs
>
>
> * Only internal commands allowed
> (for inq) */
>
> So it seems the intent was for the state to not change until then.
I think this is historical. There was a change somewhere that moved
the sysfs state handling out of the sdev stat to is_visible, so the
sdev state no-longer reflects it.
> The call to set the SDEV_RUNNING state earlier in scsi_add_lun()
> was added with:
>
> commit 6f4267e3bd1211b3d09130e626b0b3d885077610
> Author: James Bottomley <[email protected]>
> Date: Fri Aug 22 16:53:31 2008 -0500
>
> [SCSI] Update the SCSI state model to allow blocking in the
> created state
>
> Which allows the device to go into ->BLOCK (which is needed, since it
> actually happens).
>
> Should we remove the call from scsi_sysfs_add_sdev() and change the
> comment in scsi_device.h to reflect the intent?
Assuming someone with the problem actually tests it, yes.
> I have not verified the async vs. non-async scan path yet but it
> looks like it would be OK.
I did. The async device addition occurs after scsi_add_lun(), so it
rules the state change in both cases.
James
> -Ewan
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi"
> in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
