Re: [PATCH 2/4] scsi: use 64-bit LUNs
On 13-02-25 10:33 AM, Steffen Maier wrote: Hi Hannes, I like the idea and most of the patch set, so I only have a few questions left and some review comments below. Just curious: Do you also plan to adapt systemd/udev, especially path_id for fc transport with its open coded copy of int_to_scsilun()? Since I don't see zfcp touched in this patch set, assuming this set will get merged, I plan to adapt a few spots in zfcp that might not work with 64 bit luns out of the box although most of it already works with 64 bit luns inside. Speaking of opaque handling of scsi luns: Lately I noticed that some sg3_utils tools decode the lun format and only report parts of the entire 64 bit fcp lun, e.g. sg_scan or "sg_luns -d". I don't have enough historical scsi experience to know why that is, maybe because of some SPI background. By itself this is not a problem, however, rescan-scsi-bus.sh makes use of those scsi lun parts and then fails when matching those with the full scsi lun exposed by the midlayer to user space. E.g. with flat space addresses of IBM DS8000 this does not seem to work: # sg_luns -v -s2 -d /dev/sg2 | head report luns cdb: a0 00 02 00 00 00 00 00 20 00 00 00 report luns: requested 8192 bytes but got 4112 bytes Lun list length = 4104 which imples 513 lun entries Report luns [select_report=2]: c101 REPORT LUNS well known logical unit 40204000 Flat space addressing: lun=32 40204001 Flat space addressing: lun=32 40204002 Flat space addressing: lun=32 ^^<=== 0x20 of the lun's 1st short According to sam5r13.pdf ** section 4.7.7.3 you owe me a beer :-) "Flat space" addressing is only 16 bits long. "Extended Flat space" and "Long Extended Flat space" addressing would have the top bit set in byte 0 (and no part of the actual lun is in byte 0). # sg_luns --test=40204002 -H Decoded LUN: Flat space addressing: lun=0x0020 A vendor specific LUN or am I misreading SAM-5? sg_luns --test=d2204002 -H Decoded LUN: Extended flat space addressing: lun=0x204002 I guess we cannot adapt sg_ioctl SG_GET_SCSI_ID that easily without breaking the user space interface. But how does a user of this interface know that there are 64 bit luns in the kernel but the ioctl cannot handle the new kernel functionality (and may be affected by aliasing)? I think that would involve adding a new ioctl (e.g. SG_GET_SCSI_ID64) and last time I proposed that L. Torvalds said something like: over his dead body. Doug Gilbert ** now we have conglomerate LUNs! -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] scsi: use 64-bit LUNs
On 02/25/2013 04:33 PM, Steffen Maier wrote:
Hi Hannes,
I like the idea and most of the patch set, so I only have a few questions left
> and some review comments below.
Just curious: Do you also plan to adapt systemd/udev, especially path_id for
> fc transport with its open coded copy of int_to_scsilun()?
Yes.
Since I don't see zfcp touched in this patch set, assuming this set will get
> merged, I plan to adapt a few spots in zfcp that might not work
with 64 bit
> luns out of the box although most of it already works with 64 bit
luns inside.
Ah. Yes, this might be a good idea. I've already got a patch from
QLogic regarding qla4xxx, so maybe we should be adding them as
separate patches on top of the original patchset.
Speaking of opaque handling of scsi luns: Lately I noticed that some sg3_utils
> tools decode the lun format and only report parts of the entire
64 bit fcp lun,
> e.g. sg_scan or "sg_luns -d". I don't have enough historical scsi
experience to
> know why that is, maybe because of some SPI background. By itself
this is not a
> problem, however, rescan-scsi-bus.sh makes use of those scsi lun
parts and then
> fails when matching those with the full scsi lun exposed by the
midlayer to user
> space. E.g. with flat space addresses of IBM DS8000 this does not
seem to work:
# sg_luns -v -s2 -d /dev/sg2 | head
report luns cdb: a0 00 02 00 00 00 00 00 20 00 00 00
report luns: requested 8192 bytes but got 4112 bytes
Lun list length = 4104 which imples 513 lun entries
Report luns [select_report=2]:
c101
REPORT LUNS well known logical unit
40204000
Flat space addressing: lun=32
40204001
Flat space addressing: lun=32
40204002
Flat space addressing: lun=32
^^<=== 0x20 of the lun's 1st short
Did I overlook something or are rescan-scsi-bus.sh and maybe other tools really
> broken with some "modern" scsi targets?
Should've been fixed by now.
There was a bug in rescan-scsi-bus.sh which indeed tried to decode
LUNs, but that has been fixed.
On 02/19/2013 09:18 AM, Hannes Reinecke wrote:
The SCSI standard uses a 64-bit value for LUNs, and large arrays
employing large LUN numbers become more and more common.
So move the linux SCSI stack to use 64-bit LUN numbers.
Signed-off-by: Hannes Reinecke
---
drivers/scsi/scsi_proc.c|2 +-
drivers/scsi/scsi_sysfs.c | 14
drivers/scsi/scsi_transport_fc.c|4 +-
drivers/scsi/scsi_transport_iscsi.c |4 +-
drivers/scsi/scsi_transport_sas.c |2 +-
drivers/scsi/sg.c |4 +-
include/scsi/scsi.h |2 +-
include/scsi/scsi_device.h | 22 ++--
include/scsi/scsi_transport.h |2 +-
50 files changed, 239 insertions(+), 247 deletions(-)
--- a/drivers/scsi/scsi_proc.c
+++ b/drivers/scsi/scsi_proc.c
@@ -196,7 +196,7 @@ static int proc_print_scsidevice(struct device *dev, void
*data)
sdev = to_scsi_device(dev);
seq_printf(s,
- "Host: scsi%d Channel: %02d Id: %02d Lun: %02d\n Vendor: ",
+ "Host: scsi%d Channel: %02d Id: %02d Lun: %02llu\n Vendor: ",
sdev->host->host_no, sdev->channel, sdev->id, sdev->lun);
for (i = 0; i < 8; i++) {
if (sdev->vendor[i] >= 0x20)
Is it intentional that you did not adapt scsi_add_single_device(),
> scsi_remove_single_device(), proc_scsi_write() to cope with 64
bit luns?
(in the admittedly old and probably somewhat deprecated procfs interface;
> but then again, proc_print_scsidevice can output 64 bit luns now)
I deliberately did _not_ touch procfs (apart from the necessary
bits). Precisely because it's being marked as deprecated.
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 931a7d9..6e98f05 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -80,7 +80,7 @@ const char *scsi_host_state_name(enum scsi_host_state state)
return name;
}
-static int check_set(unsigned int *val, char *src)
+static int check_set(unsigned long long *val, char *src)
{
char *last;
@@ -90,7 +90,7 @@ static int check_set(unsigned int *val, char *src)
/*
* Doesn't check for int overflow
*/
- *val = simple_strtoul(src, &last, 0);
+ *val = simple_strtoull(src, &last, 0);
if (*last != '\0')
return 1;
}
@@ -99,11 +99,11 @@ static int check_set(unsigned int *val, char *src)
static int scsi_scan(struct Scsi_Host *shost, const char *str)
{
- char s1[15], s2[15], s3[15], junk;
- unsigned int channel, id, lun;
+ char s1[15], s2[15], s3[17], junk;
Since we use automatic base detection with the 3rd argument of simple_strtoull()
> being 0
Re: [PATCH 2/4] scsi: use 64-bit LUNs
Hi Hannes,
I like the idea and most of the patch set, so I only have a few questions left
and some review comments below.
Just curious: Do you also plan to adapt systemd/udev, especially path_id for fc
transport with its open coded copy of int_to_scsilun()?
Since I don't see zfcp touched in this patch set, assuming this set will get
merged, I plan to adapt a few spots in zfcp that might not work with 64 bit
luns out of the box although most of it already works with 64 bit luns inside.
Speaking of opaque handling of scsi luns: Lately I noticed that some sg3_utils
tools decode the lun format and only report parts of the entire 64 bit fcp lun,
e.g. sg_scan or "sg_luns -d". I don't have enough historical scsi experience to
know why that is, maybe because of some SPI background. By itself this is not a
problem, however, rescan-scsi-bus.sh makes use of those scsi lun parts and then
fails when matching those with the full scsi lun exposed by the midlayer to
user space. E.g. with flat space addresses of IBM DS8000 this does not seem to
work:
# sg_luns -v -s2 -d /dev/sg2 | head
report luns cdb: a0 00 02 00 00 00 00 00 20 00 00 00
report luns: requested 8192 bytes but got 4112 bytes
Lun list length = 4104 which imples 513 lun entries
Report luns [select_report=2]:
c101
REPORT LUNS well known logical unit
40204000
Flat space addressing: lun=32
40204001
Flat space addressing: lun=32
40204002
Flat space addressing: lun=32
^^<=== 0x20 of the lun's 1st short
Did I overlook something or are rescan-scsi-bus.sh and maybe other tools really
broken with some "modern" scsi targets?
On 02/19/2013 09:18 AM, Hannes Reinecke wrote:
> The SCSI standard uses a 64-bit value for LUNs, and large arrays
> employing large LUN numbers become more and more common.
> So move the linux SCSI stack to use 64-bit LUN numbers.
>
> Signed-off-by: Hannes Reinecke
> ---
> drivers/scsi/scsi_proc.c|2 +-
> drivers/scsi/scsi_sysfs.c | 14
> drivers/scsi/scsi_transport_fc.c|4 +-
> drivers/scsi/scsi_transport_iscsi.c |4 +-
> drivers/scsi/scsi_transport_sas.c |2 +-
> drivers/scsi/sg.c |4 +-
> include/scsi/scsi.h |2 +-
> include/scsi/scsi_device.h | 22 ++--
> include/scsi/scsi_transport.h |2 +-
> 50 files changed, 239 insertions(+), 247 deletions(-)
> --- a/drivers/scsi/scsi_proc.c
> +++ b/drivers/scsi/scsi_proc.c
> @@ -196,7 +196,7 @@ static int proc_print_scsidevice(struct device *dev, void
> *data)
>
> sdev = to_scsi_device(dev);
> seq_printf(s,
> - "Host: scsi%d Channel: %02d Id: %02d Lun: %02d\n Vendor: ",
> + "Host: scsi%d Channel: %02d Id: %02d Lun: %02llu\n Vendor: ",
> sdev->host->host_no, sdev->channel, sdev->id, sdev->lun);
> for (i = 0; i < 8; i++) {
> if (sdev->vendor[i] >= 0x20)
Is it intentional that you did not adapt scsi_add_single_device(),
scsi_remove_single_device(), proc_scsi_write() to cope with 64 bit luns?
(in the admittedly old and probably somewhat deprecated procfs interface; but
then again, proc_print_scsidevice can output 64 bit luns now)
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index 931a7d9..6e98f05 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -80,7 +80,7 @@ const char *scsi_host_state_name(enum scsi_host_state state)
> return name;
> }
>
> -static int check_set(unsigned int *val, char *src)
> +static int check_set(unsigned long long *val, char *src)
> {
> char *last;
>
> @@ -90,7 +90,7 @@ static int check_set(unsigned int *val, char *src)
> /*
>* Doesn't check for int overflow
>*/
> - *val = simple_strtoul(src, &last, 0);
> + *val = simple_strtoull(src, &last, 0);
> if (*last != '\0')
> return 1;
> }
> @@ -99,11 +99,11 @@ static int check_set(unsigned int *val, char *src)
>
> static int scsi_scan(struct Scsi_Host *shost, const char *str)
> {
> - char s1[15], s2[15], s3[15], junk;
> - unsigned int channel, id, lun;
> + char s1[15], s2[15], s3[17], junk;
Since we use automatic base detection with the 3rd argument of
simple_strtoull() being 0 in check_set() above, I think the user is free to
specify the lun to scan for in decimal/octal/hex base for s3.
With 64 bits, couldn't this need a maximum of 20 decimal digits (plus '\0'
termination) which is more than 16? I think this is a legitimate use case as
long as the scsi lun is represented in decimal in sysfs so users might just
have it from the h:c:t:l device name there.
While I don't think anyone would specify the lun in octal, it could even need
22 digits.
[Maximum numb
