Subject: Re: opinion sought about minor change to error reporting in scsi_base
To: Manuel Bouyer <bouyer@antioche.lip6.fr>
From: Matthew Jacob <mjacob@feral.com>
List: tech-kern
Date: 07/02/1998 08:02:52
This is a multi-part message in MIME format.

--------------69DB80ACA0DBA6868899B6A
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Manuel Bouyer wrote:
> 
> On Jul 1, Matthew Jacob wrote
> > I would like to make the following change to scsi_base.c- the
> > comment covers most of it, but basically it's to quiesce the
> > tape driver from getting sense data *always* printed out
> > for non-error 'errors' (e.g., filemarks and short reads).
> >
> > Opinions?
> 
> 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.

I realize now I should have given more context for both changes.
See attached...

> I think your change will break the semantic of per-driver error handler.
> If error messages are printed when they should'nt be, it's a problem in
> st_interpret_sense(), not in the generic handler.
> 

Uh, how do you figure that? 

-matt

--------------69DB80ACA0DBA6868899B6A
Content-Type: text/plain; charset=us-ascii; name="MORE_CONTEXT"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="MORE_CONTEXT"

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;

        /*
         * 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 */
#ifdef SCSIVERBOSE
        else if ((xs->flags & SCSI_SILENT) == 0) {
#ifndef SCSIDEBUG
                /*
                 * print the error, except for the ones which are
                 * pretty much just noise- FMKS and ILI from
                 * variable mode reads.
                 */
                if ((sense->flags & SSD_KEY) == SKEY_NO_SENSE) {
                        if ((sense->flags & SSD_ILI) &&
                            (st->flags & ST_FIXEDBLOCKS) == 0) {
                                ;
                        } else if ((sense->flags & SSD_FILEMARK)  != 0) {
                                ;
                        } else { 
                                scsi_print_sense(xs, 0);
                        }
                } else  
#endif
                scsi_print_sense(xs, 0);
        }
#endif


scsi_base.c:


        /* otherwise use the default */
        switch (sense->error_code & SSD_ERRCODE) {
                /*  
                 * If it's code 70, use the extended stuff and
                 * interpret the key
                 */
        case 0x71:              /* delayed error */
                sc_link->sc_print_addr(sc_link);
                key = sense->flags & SSD_KEY;
                printf(" DEFERRED ERROR, key = 0x%x\n", key);
                /* FALLTHROUGH */
        case 0x70:
                if ((sense->error_code & SSD_ERRCODE_VALID) != 0)
                        info = _4btol(sense->info);
                else
                        info = 0;
                key = sense->flags & SSD_KEY;
   
                switch (key) {
                case SKEY_NO_SENSE:
                case SKEY_RECOVERABLE_ERROR:
                        if (xs->resid == xs->datalen)
                                xs->resid = 0;  /* not short read */
                        /* FALL THROUGH */
                case SKEY_EQUAL:
#ifndef SCSIDEBUG
                        /*
                         * If we had a handler and we got here (which
                         * means that there really was no error),
                         * we just return rather than print any more
                         * (and just confusing) error messages below. 
                         */
                        if (sc_link->device->err_handler) {
                                return (0);
                        }
#endif 
                        error = 0; 
                        break;

--------------69DB80ACA0DBA6868899B6A--