pass(4): always retry CAM_REQUEUE_REQ ccbs

[Matthew Jacob]

I think it's a good idea, but returning ERESTART directly would be a 
more seamless change.

> 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?
>
>