Subject: Re: opinion sought about minor change to error reporting in scsi_base
To: Matthew Jacob <mjacob@feral.com>
From: Manuel Bouyer <bouyer@antioche.lip6.fr>
List: tech-kern
Date: 07/02/1998 19:02:16
On Jul 2, Matthew Jacob wrote
> > Why not fix st.c's error handler to return 0 instead of -1 in this case ?
> 
> Umm- it is returning zero. That's the point- I actually have some
> fixes to st.c (see below) to quiesce dopey FMK and ILI non-error
> errors. I only discovered that scsi_base.c needs fixing after
> I did this.

It doens't always return 0 when it should: at the end you have a return(-1).
I think the last return should be 0 or -1, depending on the sense reported.
I've done a quick hack to implement this - look at the end.
This needs to be dounble-checked, I've done this quickly and I may have break
the logic.

> 
> Uh, how do you figure that? 

If the handler returned -1, we shall continue the generic error handling,
regardless if we have a special handler or not.
If I write another driver with an error handler, I will have to handle
SKEY_RECOVERABLE_ERROR and SKEY_EQUAL myself, even if the generic error
handling would have done what I need.
Actually your change will break some kind of delayed errors handling for tapes.

st.c:

/*
 * Look at the returned sense and act on the error and detirmine
 * The unix error number to pass back... (0 = report no error)
 *                            (-1 = continue processing)
 */     
int     
st_interpret_sense(xs)
        struct scsipi_xfer *xs;
{
        struct scsipi_link *sc_link = xs->sc_link;
        struct scsipi_sense_data *sense = &xs->sense.scsi_sense;
        struct buf *bp = xs->bp;
        struct st_softc *st = sc_link->device_softc;
        u_int8_t key; 
        int32_t info;
	int retval = -1; 

        /*
         * Get the sense fields and work out what code
         */
        if (sense->error_code & SSD_ERRCODE_VALID)
                info = _4btol(sense->info);
        else 
                info = xs->datalen;     /* bad choice if fixed blocks */
        if ((sc_link->flags & SDEV_OPEN) == 0)
                return (-1);            /* not open yet, let generic handle */
        else if ((sense->error_code & SSD_ERRCODE) != 0x70)
                return (-1);            /* let the generic code handle it */


	/*
	 * Don't ask the generic handler to print error if it's not
	 * a real error.
	 */
	switch (sense->flags & SSD_KEY) {
	case SKEY_NO_SENSE:
                if (((sense->flags & SSD_ILI) &&
                    (st->flags & ST_FIXEDBLOCKS) == 0) ||
		    (sense->flags & SSD_FILEMARK)  != 0)
#ifndef SCSIDEBUG
                        retval = 0;
#endif
			break;
                }
		if (xs->resid == xs->datalen)
			xs->resid = 0;  /* not short read */
		break;
	case SKEY_RECOVERABLE_ERROR:
		if (xs->resid == xs->datalen)
			xs->resid = 0;  /* not short read */
		/* FALLTHROUGH */
	case SKEY_EQUAL:
#ifndef SCSIDEBUG
		retval = 0; /* Error processing is done */
#endif

        }

And at the end you return (retval) instead of (-1).

--
Manuel Bouyer, LIP6, Universite Paris VI.           Manuel.Bouyer@lip6.fr
--