Sanitize PQ3 device handling
PATCH 1/1 Sanitize PQ3 device handling
Hi James,
the attached patch sanitzes the handling of devices with a periperal qualifier of 3. As we don't set non-existing devices offline anymore there is no real sense in distinguishing between PQ 1 and PQ3, as the net effect (no available device) is identical. However, we really want to have access to the target via sg.
Please apply.
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke [email protected] SuSE Linux AG S390 & zSeries Maxfeldstraße 5 +49 911 74053 688 90409 Nürnberg http://www.suse.de
drivers/scsi/scsi_scan.c +37 -33
@@ -579,6 +579,8 @@ static void scsi_probe_lun(struct scsi_r* * Return: * SCSI_SCAN_NO_RESPONSE: could not allocate or setup a Scsi_Device +* SCSI_SCAN_TARGET_PRESENT: a Scsi_Device was allocated and initialized +* however there is (currently) no LUN connected. * SCSI_SCAN_LUN_PRESENT: a new Scsi_Device was allocated and initialized **/ static int scsi_add_lun (struct scsi_device *sdev,char *inq_result,int *bflags)
@@ -651,6 +653,11 @@ static int scsi_add_lun(struct scsi_devi* Don't set the device offline here; rather let the upper * level drivers eval the PQ to decide whether they should * attach. So remove ((inq_result[0] >> 5) & 7) == 1 check. +* +* Since we don't set the device offline anymore there is +* no sense at all in treating PQ 1 and PQ 3 differently +* as both do not have a device attached. If a PQ 1 target +* ever get it's device back we need to rescan it anyway. */ sdev->inq_periph_qual = (inq_result[0 ] >>5 ) &7 ;
@@ -739,7 +746,16 @@ static int scsi_add_lun(struct scsi_devi*/ scsi_sysfs_add_sdev (sdev); -return SCSI_SCAN_LUN_PRESENT ; +/* +* Targets with a PQ of 1 or 3 do not have +* a device attached; tell the upper layers +* about it. +*/ +if (((inq_result[0 ] >>5 ) &7 ) ==1 || + ((inq_result[0 ] >>5 ) &7 ) ==3 ) +return SCSI_SCAN_TARGET_PRESENT ; +else +return SCSI_SCAN_LUN_PRESENT ; }/**
@@ -812,26 +828,8 @@ static int scsi_probe_and_add_lun(struct/* * result contains valid SCSI INQUIRY data. */ -if ((result[0 ] >>5 ) ==3 ) { -/* -* For a Peripheral qualifier 3 (011b), the SCSI -* spec says: The device server is not capable of -* supporting a physical device on this logical -* unit. -* -* For disks, this implies that there is no -* logical disk configured at sdev->lun, but there -* is a target id responding. -*/ -SCSI_LOG_SCAN_BUS (3 ,printk (KERN_INFO -"scsi scan: peripheral qualifier of 3," -" no device added\n" )); - res =SCSI_SCAN_TARGET_PRESENT ; - goto out_free_result; - } - res =scsi_add_lun (sdev, result, &bflags); -if (res ==SCSI_SCAN_LUN_PRESENT ) { +if (res !=SCSI_SCAN_NO_RESPONSE ) {if (bflags &BLIST_KEY ) { sdev->lockable =0 ;scsi_unlock_floptical (sreq, result);
@@ -845,7 +843,7 @@ static int scsi_probe_and_add_lun(struct out_free_sreq:scsi_release_request (sreq); out_free_sdev: -if (res ==SCSI_SCAN_LUN_PRESENT ) { +if (res !=SCSI_SCAN_NO_RESPONSE ) {if (sdevp) {scsi_device_get (sdev); *sdevp = sdev;
@@ -1199,7 +1197,7 @@ struct scsi_device *__scsi_add_device(stdown (&shost->scan_mutex); res =scsi_probe_and_add_lun (starget, lun,NULL , &sdev,1 , hostdata); -if (res !=SCSI_SCAN_LUN_PRESENT ) +if (res ==SCSI_SCAN_NO_RESPONSE ) sdev =ERR_PTR (-ENODEV );up (&shost->scan_mutex);scsi_target_reap (starget);
@@ -1279,23 +1277,29 @@ void scsi_scan_target(struct device *par* would not configure LUN 0 until all LUNs are scanned. */ res =scsi_probe_and_add_lun (starget,0 , &bflags, &sdev, rescan,NULL ); -if (res ==SCSI_SCAN_LUN_PRESENT ) { -if (scsi_report_lun_scan (sdev, bflags, rescan) !=0 ) +if (res !=SCSI_SCAN_NO_RESPONSE ) { +if (scsi_report_lun_scan (sdev, bflags, rescan) !=0 ) {/* * The REPORT LUN did not scan the target, * do a sequential scan. */ +if (res ==SCSI_SCAN_TARGET_PRESENT ) +/* +* There's a target here, but lun 0 is +* offline so we can't use the report_lun +* scan. Fall back to a sequential lun scan +* with a bflags of SPARSELUN. +* +* The old code also used a default scsi level +* of SCSI_2 which seems a bit spurious. Any +* misbehaving device should rather be added +* to the blacklist. +*/ + bflags |=BLIST_SPARSELUN ; +scsi_sequential_lun_scan (starget, bflags, res, sdev->scsi_level, rescan); - }else if (res ==SCSI_SCAN_TARGET_PRESENT ) { -/* -* There's a target here, but lun 0 is offline so we -* can't use the report_lun scan. Fall back to a -* sequential lun scan with a bflags of SPARSELUN and -* a default scsi level of SCSI_2 -*/ -scsi_sequential_lun_scan (starget,BLIST_SPARSELUN , -SCSI_SCAN_TARGET_PRESENT ,SCSI_2 , rescan); + } }if (sdev)scsi_device_put (sdev);
Only in linux-2.6.12-rc4/drivers/scsi: scsi_scan.c~ Only in linux-2.6.12-rc4/drivers/scsi: scsi_scan.c.orig
Quoted 15 lines
drivers/scsi/scsi_scan.c +8
-2res =scsi_probe_and_add_lun (starget,0 , &bflags, &sdev, rescan,NULL ); -if (res ==SCSI_SCAN_LUN_PRESENT ) { -if (scsi_report_lun_scan (sdev, bflags, rescan) !=0 ) +if (res !=SCSI_SCAN_NO_RESPONSE ) { +if (scsi_report_lun_scan (sdev, bflags, rescan) !=0 ) {/* * The REPORT LUN did not scan the target, * do a sequential scan. */ +if (res ==SCSI_SCAN_TARGET_PRESENT ) +/* +* There's a target here, but lun 0 is +* offline so we can't use the report_lun +* scan. Fall back to a sequential lun scan +* with a bflags of SPARSELUN.
drivers/scsi/scsi_scan.c void scsi_scan_target(struct device *par · line 1301
Quoted 10 lines
drivers/scsi/scsi_scan.c +8
-0+* +* The old code also used a default scsi level +* of SCSI_2 which seems a bit spurious. Any +* misbehaving device should rather be added +* to the blacklist. +*/ + bflags |=BLIST_SPARSELUN ; +scsi_sequential_lun_scan (starget, bflags, res, sdev->scsi_level, rescan);
If scsi_probe_and_add_lun returns SCSI_SCAN_TARGET_PRESENT sdev isn't valid at this point. You should probably default to SCSI_2 for that case.
Quoted 20 lines · Christoph Hellwig
Quoted 20 lines
res = scsi_probe_and_add_lun(starget, 0, &bflags, &sdev, rescan, NULL);drivers/scsi/scsi_scan.c +2
-2-if (res ==SCSI_SCAN_LUN_PRESENT ) { -if (scsi_report_lun_scan (sdev, bflags, rescan) !=0 ) +if (res !=SCSI_SCAN_NO_RESPONSE ) { +if (scsi_report_lun_scan (sdev, bflags, rescan) !=0 ) {/* * The REPORT LUN did not scan the target, * do a sequential scan. */drivers/scsi/scsi_scan.c +11
-0+if (res ==SCSI_SCAN_TARGET_PRESENT ) +/* +* There's a target here, but lun 0 is +* offline so we can't use the report_lun +* scan. Fall back to a sequential lun scan +* with a bflags of SPARSELUN. +* +* The old code also used a default scsi level +* of SCSI_2 which seems a bit spurious. Any +* misbehaving device should rather be added +* to the blacklist.
Quoted 9 lines
Quoted 5 lines
drivers/scsi/scsi_scan.c +3
-0+*/ + bflags |=BLIST_SPARSELUN ; +scsi_sequential_lun_scan(starget, bflags, res, sdev->scsi_level, rescan);If
scsi_probe_and_add_lunreturnsSCSI_SCAN_TARGET_PRESENTsdev isn't valid at this point. You should probably default toSCSI_2for that case.
How so? This is exactly the behaviour I've removed. sdev is valid even for SCSI_SCAN_TARGET_PRESENT.
If not please show me.
Cheers,
Hannes
Signature · 8 lines
Dr. Hannes Reinecke [email protected] SuSE Linux AG S390 & zSeries Maxfeldstraße 5 +49 911 74053 688 90409 Nürnberg http://www.suse.de - 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
Quoted 6 lines · Hannes Reinecke
Quoted 3 lines
If
scsi_probe_and_add_lunreturnsSCSI_SCAN_TARGET_PRESENTsdev isn't valid at this point. You should probably default toSCSI_2for that case.How so? This is exactly the behaviour I've removed. sdev is valid even for
SCSI_SCAN_TARGET_PRESENT.
If not please show me.
Indeed, you replaced the check at the end of scsi_probe_and_add_lun. I don't think it makes a lot of sense to leave around the stale scsi_dev for PQ3, though.
Doing a REPORT_LUNS for SCSI_SCAN_TARGET_PRESENT does OTOH make a lot of sense, there's nothing in the spec against it. So what about dropping that device after we're done with REPORT_LUNS?
Quoted 10 lines · Christoph Hellwig
Quoted 6 lines · Hannes Reinecke
Quoted 3 lines
If
scsi_probe_and_add_lunreturnsSCSI_SCAN_TARGET_PRESENTsdev isn't valid at this point. You should probably default toSCSI_2for that case.How so? This is exactly the behaviour I've removed. sdev is valid even for
SCSI_SCAN_TARGET_PRESENT.
If not please show me.Indeed, you replaced the check at the end of
scsi_probe_and_add_lun. I don't think it makes a lot of sense to leave around the stalescsi_devfor PQ3, though.
Why not? sg connects to it and you can ask it to provide lots of useless information. Eg doing an report_luns by hand if the configuration of the SAN has changed; PQ3 only states that _this_ device will never have a LUN attached.
Quoted 5 lines
Doing a
REPORT_LUNSforSCSI_SCAN_TARGET_PRESENTdoes OTOH make a lot of sense, there's nothing in the spec against it. So what about dropping that device after we're done withREPORT_LUNS?
See above. I'd like to have them around to be able to send commands to it.
Cheers,
Hannes
Signature · 8 lines
Dr. Hannes Reinecke [email protected] SuSE Linux AG S390 & zSeries Maxfeldstraße 5 +49 911 74053 688 90409 Nürnberg http://www.suse.de - 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
Quoted 20 lines · Hannes Reinecke
Quoted 6 lines · Christoph Hellwig
Quoted 4 lines · Hannes Reinecke
Quoted 3 lines
If
scsi_probe_and_add_lunreturnsSCSI_SCAN_TARGET_PRESENTsdev isn't valid at this point. You should probably default toSCSI_2for that case.Quoted 6 lines
Quoted 3 lines
How so? This is exactly the behaviour I've removed. sdev is valid even for
SCSI_SCAN_TARGET_PRESENT.
If not please show me.Indeed, you replaced the check at the end of
scsi_probe_and_add_lun. I don't think it makes a lot of sense to leave around the stalescsi_devfor PQ3, though.Why not? sg connects to it and you can ask it to provide lots of useless information. Eg doing an
report_lunsby hand if the configuration of the SAN has changed; PQ3 only states that _this_ device will never have a LUN attached.
Quoted 6 lines
Quoted 4 lines
Doing a
REPORT_LUNSforSCSI_SCAN_TARGET_PRESENTdoes OTOH make a lot of sense, there's nothing in the spec against it. So what about dropping that device after we're done withREPORT_LUNS?See above. I'd like to have them around to be able to send commands to it.
I like this (Hannes') patch. It makes those targets "addressable".
We do not necessarily want to care only about "live sdevs" -- a quiet SCSI target sitting in the domain should be addressable as Hannes points out. We should be able to send commands to it and get back responses as it may cater other services, not as apparent as a block device.
Luben