On 03/15/2013 04:54 PM, Steffen Maier wrote:
While we're at it: I recently figured that there are targets
responding to inquiry with PQ=1 && PDT=31 for LUN0 if LUN0 has no
backing device (e.g. no disk mapped for the initiator host). While
this is likely to work with in-kernel lun scanning, the kernel does
not even allocate an sg dev in this case.
Yes.
If the target (such do exist) now also does not support report_luns
on WLUN, this effectively prevents any user space lun discovery.
E.g. non-NPIV zfcp-attached LUNs cannot be discovered, because we
cannot do in-kernel scanning due to the lack of initiator lun masking.
Yes, I know; the NetApp case.
But you can always initiate a user-space discovery by
echo '- - -' > /sys/class/scsi_host/hostX/scan
The reason for Linux ignoring LUNs with PDT=31 and PQ=1 is:
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=84961f28e9d13a4b193d0c8545f3c060c1890ff3
[SCSI] Don't add scsi_device for devices that return PQ=1, PDT=0x1f
from kernel 2.6.19.
Since Linux on System z could no longer perform report luns with IBM
DS8000, the following workaround was implemented:
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=01b291bd66564b4bd826326af6bd0b6d17e99439
[SCSI] fix check of PQ and PDT bits for WLUNs
in kernel 2.6.27.
However, this only works for WLUNs reporting PDT=31 and PQ=1 but not
for LUN0.
What made it worse is that, the attached LUN looks perfect from a
zfcp status point of view but is still missing any user visible
handle for the scsi midlayer, so I was puzzled as a user despite the
otherwise clear scsi log message:
"scsi scan: peripheral device type of 31, ***no device added***".
Hmm. You sure this is still valid today?
Assuming the device supports REPORT LUNS we're having this:
res = scsi_probe_and_add_lun(starget, 0, &bflags, NULL, rescan, NULL);
if (res == SCSI_SCAN_LUN_PRESENT || res == SCSI_SCAN_TARGET_PRESENT) {
if (scsi_report_lun_scan(starget, bflags, rescan) != 0)
/*
* The REPORT LUN did not scan the target,
* do a sequential scan.
*/
scsi_sequential_lun_scan(starget, bflags,
starget->scsi_level, rescan);
}
out_reap:
scsi_autopm_put_target(starget);
/* now determine if the target has any children at all
* and if not, nuke it */
scsi_target_reap(starget);
So we don't actually need an sdev to invoke a report lun scan;
all we require is a correct return code from scsi_probe_and_add_lun().
Which we'll get irrespective of the PQ setting.
So from what I'm seeing this case should be covered.
Unless I'm missing something ...
On 03/15/2013 10:46 AM, Hannes Reinecke wrote:
SAM advertises the use of a Well-known LUN (W_LUN) for scanning.
As this avoids exposing LUN 0 (which might be a valid LUN) for
all initiators it is the preferred method for LUN scanning on
some arrays.
So we should be using W_LUN for scanning, too. If the W_LUN is
not supported we'll fall back to use LUN 0.
For broken W_LUN implementations a new blacklist flag
'BLIST_NO_WLUN' is added.
Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 3e58b22..f4ccdea 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1312,6 +1312,7 @@ static int scsi_report_lun_scan(struct
scsi_target *starget, int bflags,
unsigned int num_luns;
unsigned int retries;
int result;
+ int w_lun = SCSI_W_LUN_REPORT_LUNS;
struct scsi_lun *lunp, *lun_data;
u8 *data;
struct scsi_sense_hdr sshdr;
@@ -1337,11 +1338,20 @@ static int scsi_report_lun_scan(struct
scsi_target *starget, int bflags,
return 0;
if (starget->no_report_luns)
return 1;
+ if (bflags & BLIST_NO_WLUN)
+ w_lun = 0;
+retry_report_lun_scan:
if (!(sdev = scsi_device_lookup_by_target(starget, 0))) {
- sdev = scsi_alloc_sdev(starget, 0, NULL);
- if (!sdev)
- return 0;
+ sdev = scsi_alloc_sdev(starget, w_lun, NULL);
+ if (!sdev) {
+ if (w_lun != 0) {
+ w_lun = 0;
+ sdev = scsi_alloc_sdev(starget, w_lun, NULL);
+ }
+ if (!sdev)
+ return 0;
Semantically this belongs to the 2nd scsi_alloc_sdev(starget, w_lun,
NULL) i.e. inside (at the end of) the body of if (w_lun != 0).
Then you can even merge the outer and inner if statement to
if (!sdev && wlun != 0)
Semantics: WLUN did not work and we haven't already tried LUN0.
Maybe it's just me but I found it more difficult to read if the 2nd
check is on its own inside the outer if statement with exactly the
same boolean expression.
+ }
if (scsi_device_get(sdev)) {
__scsi_remove_device(sdev);
return 0;
Now that we not only use LUN0, it would be nice to reflect the
currently used LUN in the corresponding scsi logging statements for
debugging and problem determination purposes.
It seems as if devname only contains HCT but not L, which can now be
either WLUN or LUN0, e.g. here:
for (retries = 0; retries < 3; retries++) {
SCSI_LOG_SCAN_BUS(3, printk (KERN_INFO "scsi scan: Sending"
" REPORT LUNS to %s (try %d)\n", devname,
retries));
result = scsi_execute_req(sdev, scsi_cmd, DMA_FROM_DEVICE,
lun_data, length, &sshdr,
SCSI_TIMEOUT + 4 * HZ, 3, NULL);
SCSI_LOG_SCAN_BUS(3, printk (KERN_INFO "scsi scan: REPORT
LUNS"
" %s (try %d) result 0x%x\n", result
? "failed" : "successful", retries, result));
@@ -1418,6 +1428,18 @@ static int scsi_report_lun_scan(struct
scsi_target *starget, int bflags,
}
if (result) {
+ if (w_lun != 0 && scsi_device_created(sdev)) {
Did you mean _!_scsi_device_created(sdev), i.e. we tried WLUN but
could not attach it?
+ /*
+ * W_LUN probably not supported, try with LUN 0
+ */
Does this only check for an sg dev having been created or does it
check for the response to the report_luns cmnd? It should be the
latter since there are targets allowing attachment of WLUN (with
PQ=1 && PDT=31) but refuse to answer report_luns, e.g. IBM SVC /
Storwize V7000.
Hmm?
Targets supporting WLUN but refuse to answer report_luns on them?
This is the 'REPORT_LUNs WLUN' we're talking to.
The _sole_ purpose of which is to _answer_ to REPORT_LUNS.
Not answering to REPORT LUNS seems to be a violation of
SPC, where support for REPORT LUNs is mandatory.
But in either case, that's why I did the blacklist flag.
We would need to add those devices to it.
Anyway, you are correct in that we need to check the response
from report_luns. I'll cross check here.
Remember your changes to zfcp_san_disc reacting on sg_luns return
code? [Novell bug 742352]
It would be nice if we could avoid start using BLIST_NO_WLUN right
away. ;-)
Oh, that was the idea.
+ SCSI_LOG_SCAN_BUS(3, printk (KERN_INFO "scsi scan:"
+ "W_LUN not supported, try LUN 0\n"));
+ kfree(lun_data);
+ scsi_device_put(sdev);
+ __scsi_remove_device(sdev);
+ w_lun = 0;
+ goto retry_report_lun_scan;
+ }
/*
* The device probably does not support a REPORT LUN
command
*/
diff --git a/include/scsi/scsi_devinfo.h
b/include/scsi/scsi_devinfo.h
index cc1f3e7..ffb42b1 100644
--- a/include/scsi/scsi_devinfo.h
+++ b/include/scsi/scsi_devinfo.h
@@ -31,4 +31,5 @@
#define BLIST_MAX_512 0x800000 /* maximum 512 sector cdb
length */
#define BLIST_ATTACH_PQ3 0x1000000 /* Scan: Attach to PQ3
devices */
#define BLIST_NO_DIF 0x2000000 /* Disable T10 PI (DIF) */
+#define BLIST_NO_WLUN 0x4000000 /* Disable W_LUN scanning */
#endif
The overall idea seems reasonable to me.
See?
I'll be updating the patch.
Cheers,
Hannes]
--
Dr. Hannes Reinecke zSeries & Storage
hare@xxxxxxx +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
