TUR path is down after adapter gets reset in multipath configuration
cover TUR path is down after adapter gets reset in multipath configuration
Based on Christoph's and Brian's patches, these patch fix the TUR path is down after adapter gets reset issue.
Thanks,
Wendy
PATCH 1/2 scsi: TUR path is down after adapter gets reset in multipath configuration(scsi_error.c)
After an ipr adapter gets reset, all disk array devices require a start unit command to be issued to them before they will accept commands. So, with the SCSI EH change, we now end up in a scenario with dual ipr adapters where the TUR getting issued from the health checker returns with a Not Ready response and since SCSI EH no longer triggers the Start Unit in this scenario, the path never recovers.
--- drivers/scsi/scsi_error.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/scsi/scsi_error.c +8 -8
@@ -459,14 +459,6 @@ static int scsi_check_sense(struct scsi_if (!scsi_command_normalize_sense (scmd, &sshdr))return FAILED ;/* no valid sense data */ -if (scmd->cmnd[0 ] ==TEST_UNIT_READY && scmd->scsi_done != scsi_eh_done) -/* -* nasty: for mid-layer issued TURs, we need to return the -* actual sense data without any recovery attempt. For eh -* issued ones, we need to try to recover and interpret -*/ -return SUCCESS ; -scsi_report_sense (sdev, &sshdr);if (scsi_sense_is_deferred (&sshdr))
@@ -482,6 +474,14 @@ static int scsi_check_sense(struct scsi_/* handler does not care. Drop down to default handling */ } +if (scmd->cmnd[0 ] ==TEST_UNIT_READY && scmd->scsi_done != scsi_eh_done) +/* +* nasty: for mid-layer issued TURs, we need to return the +* actual sense data without any recovery attempt. For eh +* issued ones, we need to try to recover and interpret +*/ +return SUCCESS ; +/* * Previous logic looked for FILEMARK, EOM or ILI which are * mainly associated with tapes and returned SUCCESS.
RE: [PATCH 1/2] scsi: TUR path is down after adapter gets reset in multipath configuration(scsi_error.c)
Quoted 10 lines
-----Original Message-----
From: [email protected] [mailto:linux-scsi- [email protected]] On Behalf Of [email protected]
Sent: Monday, 27 October, 2014 1:02 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]
Subject: [PATCH 1/2] scsi: TUR path is down after adapter gets reset in multipathconfiguration(scsi_error.c)
Quoted 8 lines
After an ipr adapter gets reset, all disk array devices require a start unit command to be issued to them before they will accept commands. So, with the SCSI EH change, we now end up in a scenario with dual ipr adapters where the TUR getting issued from the health checker returns with a Not Ready response and since SCSI EH no longer triggers the Start Unit in this scenario, the path never recovers.
This description doesn't make sense without reading the thread:
[dm-devel] [PATCH 1/1] multipath-tools: Change path checker for IBM IPR devices
Example: what is "the SCSI EH change"?
Re: [PATCH 1/2] scsi: TUR path is down after adapter gets reset in multipath configuration(scsi_error.c)
In reply to [email protected]Quoted 9 lines · On Mon, Oct 27, 2014 at 01:01:48PM -0500, [email protected] wrote:
After an ipr adapter gets reset, all disk array devices require a start unit command to be issued to them before they will accept commands. So, with the SCSI EH change, we now end up in a scenario with dual ipr adapters where the TUR getting issued from the health checker returns with a Not Ready response and since SCSI EH no longer triggers the Start Unit in this scenario, the path never recovers.
Signed-off-by Christoph Hellwig <[email protected]>Tested-by Wen Xiong <[email protected]>
Th patch description is entirely wrong, but I guess I'll have to take the blame for that as it's my patch. How about something like:
From: Christoph Hellwig <[email protected]>
Subject: scsi: call device handler for failed TUR command
Multipath devices using the TUR path checker need to see the sense code for a failed TUR command in their device handler. Since commit <insert commit id here> we always return success for mid layer issued TUR commands before calling the device handler, which stopped the TUR path checker from working.
Move the call to the device handler check sense method before the early return for TUR commands to give the device handler a chance to intercept them.
PATCH 2/2 scsi: TUR path is down after adapter gets reset in multipath configuration(scsi_dh_alus.c)
In reply to [email protected]This patch also fixes the 02/04/02 K/C/Q check in alua_check_sense handler.
--- drivers/scsi/device_handler/scsi_dh_alua.c | 7 +++++++ 1 file changed, 7 insertions(+)
drivers/scsi/device_handler/scsi_dh_alua.c +7 -0
@@ -474,6 +474,13 @@ static int alua_check_sense(struct scsi_* LUN Not Ready -- Offline */ return SUCCESS ; +if (sdev->allow_restart && + (sense_hdr->asc ==0x04 ) && (sense_hdr->ascq ==0x02 )) +/* +* if the device is not started, we need to wake +* the error handler to start the motor +*/ +return FAILED ;break ;case UNIT_ATTENTION :if (sense_hdr->asc ==0x29 && sense_hdr->ascq ==0x00 )
RE: [PATCH 2/2] scsi: TUR path is down after adapter gets reset in multipath configuration(scsi_dh_alus.c)
Quoted 10 lines
-----Original Message-----
From: [email protected] [mailto:linux-scsi- [email protected]] On Behalf Of [email protected]
Sent: Monday, 27 October, 2014 1:02 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]
Subject: [PATCH 2/2] scsi: TUR path is down after adapter gets reset in multipathconfiguration(scsi_dh_alus.c)
Quoted 5 lines
This patch also fixes the 02/04/02 K/C/Q check in
alua_check_sensehandler.Signed-off-by Brian King <[email protected]>Teste-by Wen Xiong <[email protected]>
Missing a d
Quoted 11 lines
--- drivers/scsi/device_handler/scsi_dh_alua.c | 7 +++++++ 1 file changed, 7 insertions(+)Index:
b/drivers/scsi/device_handler/scsi_dh_alua.c
===================================================================
---a/drivers/scsi/device_handler/scsi_dh_alua.c2014-10-23
13:00:45.000000000 -0500
+++b/drivers/scsi/device_handler/scsi_dh_alua.c2014-10-23
13:04:16.152079004 -0500
drivers/scsi/device_handler/scsi_dh_alua.c static int alua_check_sense(struct scsi_ · line 479
Quoted 6 lines
drivers/scsi/device_handler/scsi_dh_alua.c +2
-0@@ -474,6 +474,13 @@ static int alua_check_sense(struct scsi_* LUN Not Ready -- Offline */ return SUCCESS ; +if (sdev->allow_restart && + (sense_hdr->asc ==0x04 ) && (sense_hdr->ascq ==0x02 ))
The coding style in that function does not include the extra parenthesis, as shown by the next excerpt:
Quoted 8 lines
drivers/scsi/device_handler/scsi_dh_alua.c +5
-0+/* +* if the device is not started, we need to wake +* the error handler to start the motor +*/ +return FAILED ;break ;case UNIT_ATTENTION :if (
sense_hdr->asc==0x29&&sense_hdr->ascq==0x00)
Thus function is used several places:
* installed as the scsi_device_handler .check_sense function
* called to parse the response to REPORT TARGET PORT GROUPS in alua_rtpg
* called to parse the response to SET TARGET PORT GROUPS in stpg_endio
I'm not sure that adding NOT READY/LOGICAL UNIT NOT READY, INITIALIZING COMMAND REQUIRED (2h/04h/02h) is a good idea for the second case. The expected way to handle that response is to send START STOP UNIT with START=1.
There are conditions in which REPORT TARGET PORT GROUPS is allowed and START STOP UNIT with START=1 is not allowed:
* CbCS (capabilities-based command security) only allows START STOP UNIT if physical access (PHY ACC) is enabled, while REPORT TARGET PORT GROUPS is always allowed.
* the standby or unavailable asymmetric access states only guarantee that REPORT TARGET PORT GROUPS is allowed, not START STOP UNIT. The device is permitted to support START STOP UNIT, but it's not required.
So, it's really not a response that should be returned for that command.
Any device that does return that response must also support START STOP UNIT or it's misleading the application client. In that case, falling through to the EH to send START STOP UNIT is the right thing to do.
SET TARGET PORT GROUPS is questionable too; it also has different CbCS permissions and asymmetric access state requirements than START STOP UNIT with START=1.
Perhaps that new "return FAILED" should be skipped if the opcode is REPORT TARGET PORT GROUPS or SET TARGET PORT GROUPS?
---
Rob Elliott HP Server Storage
Re: [PATCH 2/2] scsi: TUR path is down after adapter gets reset in multipath configuration(scsi_dh_alus.c)
Quoted 22 lines · Robert (Server Storage)
Quoted 10 lines
-----Original Message-----
From: [email protected] [mailto:linux-scsi- [email protected]] On Behalf Of [email protected]
Sent: Monday, 27 October, 2014 1:02 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]
Subject: [PATCH 2/2] scsi: TUR path is down after adapter gets reset in multipathconfiguration(scsi_dh_alus.c)Quoted 5 lines
This patch also fixes the 02/04/02 K/C/Q check in
alua_check_sensehandler.Signed-off-by Brian King <[email protected]>Teste-by Wen Xiong <[email protected]>Missing a d
Quoted 4 lines
--- drivers/scsi/device_handler/scsi_dh_alua.c | 7 +++++++ 1 file changed, 7 insertions(+)
Quoted 16 lines
drivers/scsi/device_handler/scsi_dh_alua.c static int alua_check_sense(struct scsi_ · line 479
Quoted 13 lines
Index:
b/drivers/scsi/device_handler/scsi_dh_alua.c
===================================================================
---a/drivers/scsi/device_handler/scsi_dh_alua.c2014-10-23
13:00:45.000000000 -0500
+++b/drivers/scsi/device_handler/scsi_dh_alua.c2014-10-23
13:04:16.152079004 -0500drivers/scsi/device_handler/scsi_dh_alua.c +2
-0@@ -474,6 +474,13 @@ static int alua_check_sense(struct scsi_* LUN Not Ready -- Offline */ return SUCCESS ; +if (sdev->allow_restart && + (sense_hdr->asc ==0x04 ) && (sense_hdr->ascq ==0x02 ))The coding style in that function does not include the extra parenthesis, as shown by the next excerpt:
I just lifted this bit from scsi_error.c without noticing the coding style difference. We can certainly change this.
Quoted 40 lines
Quoted 9 lines
drivers/scsi/device_handler/scsi_dh_alua.c +5
-0+/* +* if the device is not started, we need to wake +* the error handler to start the motor +*/ +return FAILED ;break ;case UNIT_ATTENTION :if (
sense_hdr->asc==0x29&&sense_hdr->ascq==0x00)Thus function is used several places:
* installed as thescsi_device_handler.check_sense function
* called to parse the response to REPORT TARGET PORT GROUPS inalua_rtpg
* called to parse the response to SET TARGET PORT GROUPS instpg_endioI'm not sure that adding NOT READY/LOGICAL UNIT NOT READY, INITIALIZING COMMAND REQUIRED (2h/04h/02h) is a good idea for the second case. The expected way to handle that response is to send START STOP UNIT with START=1.
There are conditions in which REPORT TARGET PORT GROUPS is allowed and START STOP UNIT with START=1 is not allowed:
* CbCS (capabilities-based command security) only allows START STOP UNIT if physical access (PHY ACC) is enabled, while REPORT TARGET PORT GROUPS is always allowed.
* the standby or unavailable asymmetric access states only guarantee that REPORT TARGET PORT GROUPS is allowed, not START STOP UNIT. The device is permitted to support START STOP UNIT, but it's not required.So, it's really not a response that should be returned for that command.
Any device that does return that response must also support START STOP UNIT or it's misleading the application client. In that case, falling through to the EH to send START STOP UNIT is the right thing to do.
Quoted 7 lines
SET TARGET PORT GROUPS is questionable too; it also has different CbCS permissions and asymmetric access state requirements than START STOP UNIT with START=1.
Perhaps that new "return FAILED" should be skipped if the opcode is REPORT TARGET PORT GROUPS or SET TARGET PORT GROUPS?
I'm fine with that.
Wendy - do you want to make these changes and resend?
Thanks,
Brian
Signature · 3 lines
Brian King Power Linux I/O IBM Linux Technology Center