pass(4): always retry CAM_REQUEUE_REQ ccbs

[Andriy Gapon]

At present pass(4) never retries a request (or performs any other kind of error
recovery) unless the request has CAM_PASS_ERR_RECOVER flag set.
This gives applications a control over error checking and handling.
But I think that in the case of CAM_REQUEUE_REQ status error recovery,
specifically a request retry, should always be performed.

Rationale:
CAM_REQUEUE_REQ is really an internal flag/state in CAM subsystem.
It doesn't convey any information about state of medium, device, bus, controller
or the ccb itself.  Application can not make any meaningful differentiation
based on this status.  It either should always retry a ccb with this status or
face truly random failures.  As such, I haven't encountered so far any
application that expects to get CAM_REQUEUE_REQ status.
So I think that CAM_REQUEUE_REQ should be contained within the CAM and never be
leaked to applications unless retry count is exhausted (to prevent endless loops
in case of programming errors, primarily).

What do you think?
Do you see any reason not to do it?

Here's a small patch that implements the behavior:
--- a/sys/cam/scsi/scsi_pass.c
+++ b/sys/cam/scsi/scsi_pass.c
@@ -564,9 +564,8 @@ passsendccb
 	 * error recovery.
 	 */
 	cam_periph_runccb(ccb,
-	    (ccb->ccb_h.flags & CAM_PASS_ERR_RECOVER) ? passerror : NULL,
-	    /* cam_flags */ CAM_RETRY_SELTO, /* sense_flags */SF_RETRY_UA,
-	    softc->device_stats);
+	    passerror, /* cam_flags */ CAM_RETRY_SELTO,
+	    /* sense_flags */SF_RETRY_UA, softc->device_stats);

 	if (need_unmap != 0)
 		cam_periph_unmapmem(ccb, &mapinfo);
@@ -586,7 +585,10 @@ passerror

 	periph = xpt_path_periph(ccb->ccb_h.path);
 	softc = (struct pass_softc *)periph->softc;
-	
-	return(cam_periph_error(ccb, cam_flags, sense_flags,
-				 &softc->saved_ccb));
+
+	if ((ccb->ccb_h.flags & CAM_PASS_ERR_RECOVER) != 0 ||
+	 (ccb->ccb_h.status & CAM_STATUS_MASK) == CAM_REQUEUE_REQ)
+		return(cam_periph_error(ccb, cam_flags, sense_flags,
+					&softc->saved_ccb));
+	return(0);
 }


I am not sure, perhaps I could just explicitly return ERESTART in the case of
CAM_REQUEUE_REQ instead of going through cam_periph_error?

-- 
Andriy Gapon