mbox series

[0/2] scsi:donot skip lun if inquiry returns PQ=1 for all hosts

Message ID 20221214070846.1808300-1-haowenchao@huawei.com (mailing list archive)
Headers show
Series scsi:donot skip lun if inquiry returns PQ=1 for all hosts | expand

Message

Wenchao Hao Dec. 14, 2022, 7:08 a.m. UTC
commit 948e922fc4461 ("scsi: core: map PQ=1, PDT=other values to
SCSI_SCAN_TARGET_PRESENT") returns SCSI_SCAN_TARGET_PRESENT if inquiry
returns PQ=1.

According to the SPC, PQ=1 means the addressed logical unit having the
indicated device type is not accessible, it does not mean the addressed
logical unit is invalid. We still can map this lun to an sg device.

In some conditions, we do not want to skip these devices, for example
with iSCSI:

When iSCSI initiator logged in target, the target attached none valid
lun but lun0. lun0 is not an valid disk, while it would response
inquiry command with PQ=1 and other general scsi commands like probe lun.
The others luns of target is added/removed dynamicly.

We want the lun0 to be mapped to an sg device in initiator, so we can
probe luns of target based on lun0.

In first patch, I add an interface to control if to skip luns return
PQ=1 for inquiry.

In second patch, make iscsi_tcp do not skip luns return PQ=1 as default,
since I do not have iscsi_tcp environment, so here just modified the
iscsi_tcp.

Wenchao Hao (2):
  scsi:core:Add sysfs interface to control if skip lun with PQ=1
  scsi:iscsi_tcp:Do not skip lun inquiry returns PQ=1

 drivers/scsi/iscsi_tcp.c  |  1 +
 drivers/scsi/scsi_scan.c  |  9 ++++++---
 drivers/scsi/scsi_sysfs.c | 29 +++++++++++++++++++++++++++++
 include/scsi/scsi_host.h  |  3 +++
 4 files changed, 39 insertions(+), 3 deletions(-)

Comments

Christoph Hellwig Dec. 15, 2022, 7:06 a.m. UTC | #1
On Wed, Dec 14, 2022 at 03:08:44PM +0800, Wenchao Hao wrote:
> When iSCSI initiator logged in target, the target attached none valid
> lun but lun0. lun0 is not an valid disk, while it would response
> inquiry command with PQ=1 and other general scsi commands like probe lun.
> The others luns of target is added/removed dynamicly.

I can't find any special casing of LUN0 in RFC7144, can you clarify
where you think that treats LUN0 any differently than other transports?
Ulrich Windl Dec. 15, 2022, 8:07 a.m. UTC | #2
>>> Christoph Hellwig <hch@infradead.org> schrieb am 15.12.2022 um 08:06 in
Nachricht <Y5rHX95Vvl1aLhbp@infradead.org>:
> On Wed, Dec 14, 2022 at 03:08:44PM +0800, Wenchao Hao wrote:
>> When iSCSI initiator logged in target, the target attached none valid
>> lun but lun0. lun0 is not an valid disk, while it would response
>> inquiry command with PQ=1 and other general scsi commands like probe lun.
>> The others luns of target is added/removed dynamicly.
> 
> I can't find any special casing of LUN0 in RFC7144, can you clarify
> where you think that treats LUN0 any differently than other transports?

Actusally I have no idea, but as a user of FC SAN systems I can remember a case when a storage system had to present a dummy LUN0 to enable hosts to find other LUNs (while LUN0 was never actually used). Maybe the client code was imperfect, I don't know.

> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "open-iscsi" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to open-iscsi+unsubscribe@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/open-iscsi/Y5rHX95Vvl1aLhbp%40infradead.org 
> .
Wenchao Hao Dec. 15, 2022, 9:09 a.m. UTC | #3
On 2022/12/15 15:06, Christoph Hellwig wrote:
> On Wed, Dec 14, 2022 at 03:08:44PM +0800, Wenchao Hao wrote:
>> When iSCSI initiator logged in target, the target attached none valid
>> lun but lun0. lun0 is not an valid disk, while it would response
>> inquiry command with PQ=1 and other general scsi commands like probe lun.
>> The others luns of target is added/removed dynamicly.
> 
> I can't find any special casing of LUN0 in RFC7144, can you clarify
> where you think that treats LUN0 any differently than other transports?
> .
This is not described in RFC7144. The sense described above
aims to tell that a dummy lun is useful.

In my opinion, if the addressed lun still response the
inquiry and other commands, we should not skip it,
maybe let the scsi drivers like sd/st/sg to determine
how to handle this lun accordint to the PQ value.

As discussed in following mail, another drivers would
be broken too.

https://lore.kernel.org/linux-scsi/CA+PODjqrRzyJnOKoabMOV4EPByNnL1LgTi+QAKENP3NwUq5YCw@mail.gmail.com/

The key point of my patch is to add one way to map
dummy lun to an sg device.
Christoph Hellwig Dec. 16, 2022, 7:11 a.m. UTC | #4
On Thu, Dec 15, 2022 at 09:07:28AM +0100, Ulrich Windl wrote:
> Actusally I have no idea, but as a user of FC SAN systems I can remember a case when a storage system had to present a dummy LUN0 to enable hosts to find other LUNs (while LUN0 was never actually used). Maybe the client code was imperfect, I don't know.

Ignoring some of the well known LU bits that never really became
practically relevant, lun0 is needed to use the REPORT_LUNS command
to scane for the other logical units.  But unless the PQ says it
actually is a valid logic unit, we never add a sdev for it.
Christoph Hellwig Dec. 16, 2022, 7:12 a.m. UTC | #5
On Thu, Dec 15, 2022 at 05:09:31PM +0800, Wenchao Hao wrote:
> In my opinion, if the addressed lun still response the
> inquiry and other commands, we should not skip it,
> maybe let the scsi drivers like sd/st/sg to determine
> how to handle this lun accordint to the PQ value.
> 
> As discussed in following mail, another drivers would
> be broken too.

So why do you force a specific behavior for iSCSI?
Wenchao Hao Dec. 16, 2022, 11:41 a.m. UTC | #6
On 2022/12/16 15:12, Christoph Hellwig wrote:
> On Thu, Dec 15, 2022 at 05:09:31PM +0800, Wenchao Hao wrote:
>> In my opinion, if the addressed lun still response the
>> inquiry and other commands, we should not skip it,
>> maybe let the scsi drivers like sd/st/sg to determine
>> how to handle this lun accordint to the PQ value.
>>
>> As discussed in following mail, another drivers would
>> be broken too.
> 
> So why do you force a specific behavior for iSCSI?
> .

For nothing, I want the iscsi_tcp transport do not skip PQ=1 default
as what it did before commit 948e922fc4461 ("scsi: core: map PQ=1,
PDT=other values to SCSI_SCAN_TARGET_PRESENT").
Christoph Hellwig Dec. 23, 2022, 3:54 p.m. UTC | #7
On Fri, Dec 16, 2022 at 07:41:26PM +0800, Wenchao Hao wrote:
> For nothing, I want the iscsi_tcp transport do not skip PQ=1 default
> as what it did before commit 948e922fc4461 ("scsi: core: map PQ=1,
> PDT=other values to SCSI_SCAN_TARGET_PRESENT").

Well, that commit was very much intentional and is now three an a half
years old, so we've not just going to partially revert it on a
per-transport basis when it is in no way transport related.

If you can come up with a good enough rationale we could do the
sysfs override, but so far the reason mostly seems to be "I want"
and not anctual explanation of why it is useful.
Wenchao Hao Dec. 28, 2022, 9:35 a.m. UTC | #8
On 2022/12/23 23:54, Christoph Hellwig wrote:
> On Fri, Dec 16, 2022 at 07:41:26PM +0800, Wenchao Hao wrote:
>> For nothing, I want the iscsi_tcp transport do not skip PQ=1 default
>> as what it did before commit 948e922fc4461 ("scsi: core: map PQ=1,
>> PDT=other values to SCSI_SCAN_TARGET_PRESENT").
> 
> Well, that commit was very much intentional and is now three an a half
> years old, so we've not just going to partially revert it on a
> per-transport basis when it is in no way transport related.
> 
> If you can come up with a good enough rationale we could do the
> sysfs override, but so far the reason mostly seems to be "I want"
> and not anctual explanation of why it is useful.
> .


Sorry, I did not describe in detail.

Here is the reason of my patches.

1. The SPC did not tell PQ=1 means the addressed lun is invalid explicitly.
2. scsi_bus_match() would prevent luns with PQ=1 be handled by any scsi
   drivers, so the only influence if we do not skip luns with PQ=1 is we
   would add an scsi_device and an sg device.

The reason I force a specific behavior for iSCSI:

1. This issue is occurred with iSCSI, it means there are scenarios where
   targets would return PQ=1 for an valid LUN which should not be skipped.
2. The changes for iSCSI could be tested

I did not change other transports' behavior is because I do not know if PQ=1
would be returned, and I do not have the related environment. If other
transports like adaptec raid also needs these changes, they can override
the default value by other patches.

Thanks.