Clean up sense error logic and printing a bit, partly inspired by

NetBSD. Try to limit special handling of sense errors in sd and st to
a minimum.

ok marco@

3 files changed, 147 insertions, 144 deletions

diff --git a/sys/scsi/scsi_base.c b/sys/scsi/scsi_base.c
index 2929500b2e4..9c9cd9165e6 100644
--- a/sys/scsi/scsi_base.c
+++ b/sys/scsi/scsi_base.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: scsi_base.c,v 1.60 2004/05/17 23:57:51 krw Exp $	*/
+/*	$OpenBSD: scsi_base.c,v 1.61 2004/05/28 23:50:14 krw Exp $	*/
 /*	$NetBSD: scsi_base.c,v 1.43 1997/04/02 02:29:36 mycroft Exp $	*/
 
 /*
@@ -703,96 +703,77 @@ scsi_interpret_sense(xs)
 		if (error != EJUSTRETURN)
 			return error;		/* error >= 0  better ? */
 	}
-	/* otherwise use the default */
-	serr = sense->error_code & SSD_ERRCODE;
-	skey = sense->flags & SSD_KEY;
-	switch (serr) {
-		/*
-		 * If it's code 70, use the extended stuff and interpret the key
-		 */
-	case 0x71:		/* delayed error */
-		sc_print_addr(sc_link);
-		printf(" DEFERRED ERROR, key = 0x%x\n", skey);
-		/* FALLTHROUGH */
-	case 0x70:
-		switch (skey) {
-		case SKEY_NO_SENSE:
-		case SKEY_RECOVERED_ERROR:
-			if (xs->resid == xs->datalen)
-				xs->resid = 0;	/* not short read */
-			/* FALLTHROUGH */
-		case SKEY_EQUAL:
-			error = 0;
-			break;
-		case SKEY_NOT_READY:
-			if ((sc_link->flags & SDEV_REMOVABLE) != 0)
-				sc_link->flags &= ~SDEV_MEDIA_LOADED;
-			if ((xs->flags & SCSI_IGNORE_NOT_READY) != 0)
-				return 0;
-			if (xs->retries && sense->add_sense_code == 0x04 &&
-			    sense->add_sense_code_qual == 0x01) {
-				xs->error = XS_BUSY;	/* ie. sense_retry */
-				return ERESTART;
-			}
-			if (xs->retries && !(sc_link->flags & SDEV_REMOVABLE)) {
-				delay(1000000);
-				return ERESTART;
-			}
-			error = EIO;
-			break;
-		case SKEY_ILLEGAL_REQUEST:
-			if ((xs->flags & SCSI_IGNORE_ILLEGAL_REQUEST) != 0)
-				return 0;
-			error = EINVAL;
-			break;
-		case SKEY_UNIT_ATTENTION:
-			if (sense->add_sense_code == 0x29)
-				return (ERESTART); /* device or bus reset */
-			if ((sc_link->flags & SDEV_REMOVABLE) != 0)
-				sc_link->flags &= ~SDEV_MEDIA_LOADED;
-			if ((xs->flags & SCSI_IGNORE_MEDIA_CHANGE) != 0 ||
-			    /* XXX Should reupload any transient state. */
-			    (sc_link->flags & SDEV_REMOVABLE) == 0)
-				return ERESTART;
-			error = EIO;
-			break;
-		case SKEY_WRITE_PROTECT:
-			error = EROFS;
-			break;
-		case SKEY_BLANK_CHECK:
-			error = 0;
-			break;
-		case SKEY_ABORTED_COMMAND:
-			error = ERESTART;
-			break;
-		case SKEY_VOLUME_OVERFLOW:
-			error = ENOSPC;
-			break;
-		default:
-			error = EIO;
-			break;
-		}
 
-		if (skey && (xs->flags & SCSI_SILENT) == 0)
-			scsi_print_sense(xs);
-
-		return error;
+	/* Default sense interpretation. */
+	serr = sense->error_code & SSD_ERRCODE;
+	if (serr != 0x70 && serr != 0x71)
+		skey = 0xff;	/* Invalid value, since key is 4 bit value. */
+	else
+		skey = sense->flags & SSD_KEY;
 
 	/*
-	 * Not code 70, just report it
+	 * Interpret the key/asc/ascq information where appropriate.
 	 */
-	default:
-		sc_print_addr(sc_link);
-		printf("Sense Error Code %d", serr);
-		if ((sense->error_code & SSD_ERRCODE_VALID) != 0) {
-			struct scsi_sense_data_unextended *usense =
-			    (struct scsi_sense_data_unextended *)sense;
-			printf(" at block no. %d (decimal)",
-			    _3btol(usense->block));
+	error = 0;
+	switch (skey) {
+	case SKEY_NO_SENSE:
+	case SKEY_RECOVERED_ERROR:
+		if (xs->resid == xs->datalen)
+			xs->resid = 0;	/* not short read */
+		break;
+	case SKEY_BLANK_CHECK:
+	case SKEY_EQUAL:
+		break;
+	case SKEY_NOT_READY:
+		if ((sc_link->flags & SDEV_REMOVABLE) != 0)
+			sc_link->flags &= ~SDEV_MEDIA_LOADED;
+		if ((xs->flags & SCSI_IGNORE_NOT_READY) != 0)
+			return 0;
+		if (xs->retries && sense->add_sense_code == 0x04 &&
+		    sense->add_sense_code_qual == 0x01) {
+			xs->error = XS_BUSY;	/* ie. sense_retry */
+			return ERESTART;
 		}
-		printf("\n");
-		return EIO;
+		if (xs->retries && !(sc_link->flags & SDEV_REMOVABLE)) {
+			delay(1000000);
+			return ERESTART;
+		}
+		error = EIO;
+		break;
+	case SKEY_ILLEGAL_REQUEST:
+		if ((xs->flags & SCSI_IGNORE_ILLEGAL_REQUEST) != 0)
+			return 0;
+		error = EINVAL;
+		break;
+	case SKEY_UNIT_ATTENTION:
+		if (sense->add_sense_code == 0x29)
+			return (ERESTART); /* device or bus reset */
+		if ((sc_link->flags & SDEV_REMOVABLE) != 0)
+			sc_link->flags &= ~SDEV_MEDIA_LOADED;
+		if ((xs->flags & SCSI_IGNORE_MEDIA_CHANGE) != 0 ||
+		    /* XXX Should reupload any transient state. */
+		    (sc_link->flags & SDEV_REMOVABLE) == 0)
+			return ERESTART;
+		error = EIO;
+		break;
+	case SKEY_WRITE_PROTECT:
+		error = EROFS;
+		break;
+	case SKEY_ABORTED_COMMAND:
+		error = ERESTART;
+		break;
+	case SKEY_VOLUME_OVERFLOW:
+		error = ENOSPC;
+		break;
+	default:
+		error = EIO;
+		break;
 	}
+
+	if (skey && (xs->flags & SCSI_SILENT) == 0)
+		scsi_print_sense(xs);
+
+	return error;
 }
 
 /*
@@ -1426,13 +1407,26 @@ scsi_print_sense(xs)
 	struct scsi_xfer *xs;
 {
 	struct scsi_sense_data *sense = &xs->sense;
+	u_int8_t serr = sense->error_code & SSD_ERRCODE;
 	int32_t info;
 	char *sbs;
 
 	sc_print_addr(xs->sc_link);
 
 	/* XXX For error 0x71, current opcode is not the relevant one. */
-	printf("Check Condition on opcode 0x%x\n", xs->cmd->opcode);
+	printf("%sCheck Condition (error %#x) on opcode 0x%x\n",
+	    (serr == 0x71) ? "DEFERRED " : "", serr, xs->cmd->opcode);
+
+	if (serr != 0x70 && serr != 0x71) {	
+		if ((sense->error_code & SSD_ERRCODE_VALID) != 0) {
+			struct scsi_sense_data_unextended *usense =
+			    (struct scsi_sense_data_unextended *)sense;
+			printf("   AT BLOCK #: %d (decimal)",
+			    _3btol(usense->block));
+		}
+		return;
+	}
+
 	printf("    SENSE KEY: %s\n", scsi_decode_sense(sense,
 	    DECODE_SENSE_KEY));
 
diff --git a/sys/scsi/sd.c b/sys/scsi/sd.c
index f55fa734777..29c04e65399 100644
--- a/sys/scsi/sd.c
+++ b/sys/scsi/sd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: sd.c,v 1.69 2004/05/17 23:57:51 krw Exp $	*/
+/*	$OpenBSD: sd.c,v 1.70 2004/05/28 23:50:15 krw Exp $	*/
 /*	$NetBSD: sd.c,v 1.111 1997/04/02 02:29:41 mycroft Exp $	*/
 
 /*-
@@ -1096,59 +1096,55 @@ int
 sd_interpret_sense(xs)
 	struct scsi_xfer *xs;
 {
-	struct scsi_link *sc_link = xs->sc_link;
 	struct scsi_sense_data *sense = &xs->sense;
+	struct scsi_link *sc_link = xs->sc_link;
 	struct sd_softc *sd = sc_link->device_softc;
-	int retval = EJUSTRETURN;
+	u_int8_t serr = sense->error_code & SSD_ERRCODE;
+	int retval;
 
 	/*
-	 * If the device is not open yet, let the generic code handle it.
+	 * Let the generic code handle everything except a few categories of
+	 * LUN not ready errors on open devices.
 	 */
-	if ((sc_link->flags & SDEV_MEDIA_LOADED) == 0) {
-		return (retval);
-	}
-
-	/*
-	 * If it isn't a extended or extended/deferred error, let
-	 * the generic code handle it.
-	 */
-	if ((sense->error_code & SSD_ERRCODE) != 0x70 &&
-	    (sense->error_code & SSD_ERRCODE) != 0x71) {	/* DEFFERRED */
-		return (retval);
-	}
-
-	if ((sense->flags & SSD_KEY) == SKEY_NOT_READY &&
-	    sense->add_sense_code == 0x4) {
-		if (sense->add_sense_code_qual == 0x01)	{
-			printf("%s: ..is spinning up...waiting\n",
+	if (((sc_link->flags & SDEV_OPEN) == 0) ||
+	    (serr != 0x70 && serr != 0x71) ||
+	    ((sense->flags & SSD_KEY) != SKEY_NOT_READY) ||
+	    (sense->extra_len < 6) ||
+	    (sense->add_sense_code != 0x04))
+		return (EJUSTRETURN);
+
+	switch (sense->add_sense_code_qual) {
+	case 0x01:
+		printf("%s: ..is spinning up...waiting\n", sd->sc_dev.dv_xname);
+		/*
+		 * I really need a sdrestart function I can call here.
+		 */
+		delay(1000000 * 5);	/* 5 seconds */
+		retval = ERESTART;
+		break;
+	case 0x02:
+		if (sd->sc_link->flags & SDEV_REMOVABLE) {
+			printf("%s: removable disk stopped - not restarting\n",
 			    sd->sc_dev.dv_xname);
-			/*
-			 * I really need a sdrestart function I can call here.
-			 */
-			delay(1000000 * 5);	/* 5 seconds */
-			retval = ERESTART;
-		} else if (sense->add_sense_code_qual == 0x2) {
-			if (sd->sc_link->flags & SDEV_REMOVABLE) {
-				printf(
-				"%s: removable disk stopped - not restarting\n",
-				    sd->sc_dev.dv_xname);
+			retval = EIO;
+		} else {
+			printf("%s: respinning up disk\n", sd->sc_dev.dv_xname);
+			retval = scsi_start(sd->sc_link, SSS_START,
+			    SCSI_URGENT | SCSI_NOSLEEP);
+			if (retval != 0) {
+				printf("%s: respin of disk failed - %d\n",
+				    sd->sc_dev.dv_xname, retval);
 				retval = EIO;
 			} else {
-				printf("%s: respinning up disk\n",
-				    sd->sc_dev.dv_xname);
-				retval = scsi_start(sd->sc_link, SSS_START,
-				    SCSI_URGENT | SCSI_NOSLEEP);
-				if (retval != 0) {
-					printf(
-					    "%s: respin of disk failed - %d\n",
-					    sd->sc_dev.dv_xname, retval);
-					retval = EIO;
-				} else {
-					retval = ERESTART;
-				}
+				retval = ERESTART;
 			}
 		}
+		break;
+	default:
+		retval = EJUSTRETURN;
+		break;
 	}
+
 	return (retval);
 }
 
diff --git a/sys/scsi/st.c b/sys/scsi/st.c
index c073dc56308..1c3bad57f60 100644
--- a/sys/scsi/st.c
+++ b/sys/scsi/st.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: st.c,v 1.38 2004/05/17 23:57:51 krw Exp $	*/
+/*	$OpenBSD: st.c,v 1.39 2004/05/28 23:50:15 krw Exp $	*/
 /*	$NetBSD: st.c,v 1.71 1997/02/21 23:03:49 thorpej Exp $	*/
 
 /*
@@ -1807,13 +1807,29 @@ int
 st_interpret_sense(xs)
 	struct scsi_xfer *xs;
 {
-	struct scsi_link *sc_link = xs->sc_link;
 	struct scsi_sense_data *sense = &xs->sense;
-	struct buf *bp = xs->bp;
+	struct scsi_link *sc_link = xs->sc_link;
 	struct st_softc *st = sc_link->device_softc;
-	u_int8_t key;
+	struct buf *bp = xs->bp;
+	u_int8_t serr = sense->error_code & SSD_ERRCODE;
+	u_int8_t skey = sense->flags & SSD_KEY;
 	int32_t info;
 
+	if (((sense->flags & SDEV_OPEN) == 0) ||
+	    (serr != 0x70 && serr != 0x71))
+		return (EJUSTRETURN); /* let the generic code handle it */
+
+	switch (skey) {
+	case SKEY_NO_SENSE:
+	case SKEY_RECOVERED_ERROR:
+	case SKEY_MEDIUM_ERROR:
+	case SKEY_VOLUME_OVERFLOW:
+	case SKEY_BLANK_CHECK:
+		break;
+	default:
+		return (EJUSTRETURN);
+	}
+
 	/*
 	 * Get the sense fields and work out what code
 	 */
@@ -1821,8 +1837,6 @@ st_interpret_sense(xs)
 		info = _4btol(sense->info);
 	else
 		info = xs->datalen;	/* bad choice if fixed blocks */
-	if ((sense->error_code & SSD_ERRCODE) != 0x70)
-		return EJUSTRETURN; /* let the generic code handle it */
 	if (st->flags & ST_FIXEDBLOCKS) {
 		xs->resid = info * st->blksize;
 		if (sense->flags & SSD_EOM) {
@@ -1901,17 +1915,15 @@ st_interpret_sense(xs)
 			xs->resid = info;
 			if (bp)
 				bp->b_resid = info;
-			return 0;
+			return (0);
 		}
 	}
-	key = sense->flags & SSD_KEY;
 
-	if (key == 0x8) {
+	if (skey == SKEY_BLANK_CHECK) {
 		/*
-		 * This quirk code helps the drive read the
-		 * first tape block, regardless of format.  That
-		 * is required for these drives to return proper
-		 * MODE SENSE information.
+		 * This quirk code helps the drive read the first tape block,
+		 * regardless of format.  That is required for these drives to
+		 * return proper MODE SENSE information.
 		 */
 		if ((st->quirks & ST_Q_SENSE_HELP) &&
 		    !(sc_link->flags & SDEV_MEDIA_LOADED)) {
@@ -1924,10 +1936,11 @@ st_interpret_sense(xs)
 				bp->b_resid = xs->resid;
 				/* return an EOF */
 			}
-			return 0;
+			return (0);
 		}
 	}
-	return EJUSTRETURN;
+
+	return (EJUSTRETURN);
 }
 
 /*