On 10/27/2014 02:56 PM, Elliott, Robert (Server Storage) wrote: >> -----Original Message----- >> From: linux-scsi-owner@xxxxxxxxxxxxxxx [mailto:linux-scsi- >> owner@xxxxxxxxxxxxxxx] On Behalf Of wenxiong@xxxxxxxxxxxxxxxxxx >> Sent: Monday, 27 October, 2014 1:02 PM >> To: James.Bottomley@xxxxxxxxxxxxxxxxxxxxx >> Cc: hch@xxxxxxxxxxxxx; linux-scsi@xxxxxxxxxxxxxxx; >> brking@xxxxxxxxxxxxxxxxxx >> Subject: [PATCH 2/2] scsi: TUR path is down after adapter gets reset >> in multipath configuration(scsi_dh_alus.c) >> >> This patch also fixes the 02/04/02 K/C/Q check in alua_check_sense >> handler. >> >> Signed-off-by: Brian King <brking@xxxxxxxxxxxxxxxxxx> >> Teste-by: Wen Xiong <wenxiong@xxxxxxxxxxxxxxxxxx> > > Missing a d > >> --- >> 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.c 2014-10-23 >> 13:00:45.000000000 -0500 >> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c 2014-10-23 >> 13:04:16.152079004 -0500 >> @@ -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. > >> + /* >> + * 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? I'm fine with that. Wendy - do you want to make these changes and resend? Thanks, Brian -- Brian King Power Linux I/O IBM Linux Technology Center -- 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
