Date: Thu, 31 Jul 2014 11:14:22 +0200 From: Joerg Wunsch <freebsd-scsi@uriah.heep.sax.de> To: freebsd-scsi@freebsd.org, "Kenneth D. Merry" <ken@FreeBSD.ORG> Subject: Re: Bacula fails on FreeBSD 10.x / "mt fsf" infinitely proceeds Message-ID: <20140731091422.GA37239@uriah.heep.sax.de> In-Reply-To: <20140731060936.GB4095@uriah.heep.sax.de> References: <20140730035230.GA81800@nargothrond.kdm.org> <20140730060330.GA3272@uriah.heep.sax.de> <20140730153229.GA86368@nargothrond.kdm.org> <20140730191915.9B944267B@uriah.heep.sax.de> <20140730203315.0EED1295B@uriah.heep.sax.de> <20140730204200.4645729B8@uriah.heep.sax.de> <53D95F61.4080701@FreeBSD.org> <20140730215113.GA3564@uriah.heep.sax.de> <20140731035756.GA91452@nargothrond.kdm.org> <20140731060936.GB4095@uriah.heep.sax.de>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] As Joerg Wunsch wrote: > > I think it would be good to keep the ability to report to the application > > or the user what happened with control and data commands, especially since > > you may wind up having errors on both types of commands in some cases. > > (e.g. end of media notification on a write, and then perhaps an issue in > > writing the filemarks.) > > OK, then it needs some more work. Next attempt: in saerror(), rather than basing the decision whether this status report belongs to a read/write or control operation on some CCB flag that needs to be set explicitly, base it on the actual command that led to the status report (SA_READ or SA_WRITE vs. anything else). A similar decision is already done in saerror(). I think this keeps the spirit of the existing implementation as much as possible. So far, only compile-tested. -- cheers, Joerg .-.-. --... ...-- -.. . DL8DTL http://www.sax.de/~joerg/ Never trust an operating system you don't have sources for. ;-) [-- Attachment #2 --] Index: scsi_sa.c =================================================================== --- scsi_sa.c (revision 269317) +++ scsi_sa.c (working copy) @@ -113,18 +113,10 @@ #define ccb_pflags ppriv_field0 #define ccb_bp ppriv_ptr1 -#define SA_CCB_BUFFER_IO 0x0 -#define SA_CCB_TYPEMASK 0x1 -#define SA_POSITION_UPDATED 0x2 +/* bits in ccb_pflags */ +#define SA_POSITION_UPDATED 0x1 -#define Set_CCB_Type(x, type) \ - x->ccb_h.ccb_pflags &= ~SA_CCB_TYPEMASK; \ - x->ccb_h.ccb_pflags |= type -#define CCB_Type(x) (x->ccb_h.ccb_pflags & SA_CCB_TYPEMASK) - - - typedef enum { SA_FLAG_OPEN = 0x0001, SA_FLAG_FIXED = 0x0002, @@ -1835,7 +1827,6 @@ bp->bio_data, bp->bio_bcount, SSD_FULL_SIZE, IO_TIMEOUT); start_ccb->ccb_h.ccb_pflags &= ~SA_POSITION_UPDATED; - Set_CCB_Type(start_ccb, SA_CCB_BUFFER_IO); start_ccb->ccb_h.ccb_bp = bp; bp = bioq_first(&softc->bio_queue); xpt_action(start_ccb); @@ -1860,92 +1851,86 @@ { struct sa_softc *softc; struct ccb_scsiio *csio; + struct bio *bp; + int error; softc = (struct sa_softc *)periph->softc; csio = &done_ccb->csio; - switch (CCB_Type(csio)) { - case SA_CCB_BUFFER_IO: - { - struct bio *bp; - int error; - softc->dsreg = MTIO_DSREG_REST; - bp = (struct bio *)done_ccb->ccb_h.ccb_bp; - error = 0; - if ((done_ccb->ccb_h.status & CAM_STATUS_MASK) != CAM_REQ_CMP) { - if ((error = saerror(done_ccb, 0, 0)) == ERESTART) { - /* - * A retry was scheduled, so just return. - */ - return; - } + softc->dsreg = MTIO_DSREG_REST; + bp = (struct bio *)done_ccb->ccb_h.ccb_bp; + error = 0; + if ((done_ccb->ccb_h.status & CAM_STATUS_MASK) != CAM_REQ_CMP) { + if ((error = saerror(done_ccb, 0, 0)) == ERESTART) { + /* + * A retry was scheduled, so just return. + */ + return; } + } - if (error == EIO) { + if (error == EIO) { - /* - * Catastrophic error. Mark the tape as frozen - * (we no longer know tape position). - * - * Return all queued I/O with EIO, and unfreeze - * our queue so that future transactions that - * attempt to fix this problem can get to the - * device. - * - */ + /* + * Catastrophic error. Mark the tape as frozen + * (we no longer know tape position). + * + * Return all queued I/O with EIO, and unfreeze + * our queue so that future transactions that + * attempt to fix this problem can get to the + * device. + * + */ - softc->flags |= SA_FLAG_TAPE_FROZEN; - bioq_flush(&softc->bio_queue, NULL, EIO); + softc->flags |= SA_FLAG_TAPE_FROZEN; + bioq_flush(&softc->bio_queue, NULL, EIO); + } + if (error != 0) { + bp->bio_resid = bp->bio_bcount; + bp->bio_error = error; + bp->bio_flags |= BIO_ERROR; + /* + * In the error case, position is updated in saerror. + */ + } else { + bp->bio_resid = csio->resid; + bp->bio_error = 0; + if (csio->resid != 0) { + bp->bio_flags |= BIO_ERROR; } - if (error != 0) { - bp->bio_resid = bp->bio_bcount; - bp->bio_error = error; - bp->bio_flags |= BIO_ERROR; - /* - * In the error case, position is updated in saerror. - */ - } else { - bp->bio_resid = csio->resid; - bp->bio_error = 0; - if (csio->resid != 0) { - bp->bio_flags |= BIO_ERROR; - } - if (bp->bio_cmd == BIO_WRITE) { - softc->flags |= SA_FLAG_TAPE_WRITTEN; - softc->filemarks = 0; - } - if (!(csio->ccb_h.ccb_pflags & SA_POSITION_UPDATED) && - (softc->blkno != (daddr_t) -1)) { - if ((softc->flags & SA_FLAG_FIXED) != 0) { - u_int32_t l; - if (softc->blk_shift != 0) { - l = bp->bio_bcount >> - softc->blk_shift; - } else { - l = bp->bio_bcount / - softc->media_blksize; - } - softc->blkno += (daddr_t) l; + if (bp->bio_cmd == BIO_WRITE) { + softc->flags |= SA_FLAG_TAPE_WRITTEN; + softc->filemarks = 0; + } + if (!(csio->ccb_h.ccb_pflags & SA_POSITION_UPDATED) && + (softc->blkno != (daddr_t) -1)) { + if ((softc->flags & SA_FLAG_FIXED) != 0) { + u_int32_t l; + if (softc->blk_shift != 0) { + l = bp->bio_bcount >> + softc->blk_shift; } else { - softc->blkno++; + l = bp->bio_bcount / + softc->media_blksize; } + softc->blkno += (daddr_t) l; + } else { + softc->blkno++; } } - /* - * If we had an error (immediate or pending), - * release the device queue now. - */ - if (error || (softc->flags & SA_FLAG_ERR_PENDING)) - cam_release_devq(done_ccb->ccb_h.path, 0, 0, 0, 0); - if (error || bp->bio_resid) { - CAM_DEBUG(periph->path, CAM_DEBUG_INFO, - ("error %d resid %ld count %ld\n", error, - bp->bio_resid, bp->bio_bcount)); - } - biofinish(bp, softc->device_stats, 0); - break; } + /* + * If we had an error (immediate or pending), + * release the device queue now. + */ + if (error || (softc->flags & SA_FLAG_ERR_PENDING)) + cam_release_devq(done_ccb->ccb_h.path, 0, 0, 0, 0); + if (error || bp->bio_resid) { + CAM_DEBUG(periph->path, CAM_DEBUG_INFO, + ("error %d resid %ld count %ld\n", error, + bp->bio_resid, bp->bio_bcount)); } + biofinish(bp, softc->device_stats, 0); xpt_release_ccb(done_ccb); } @@ -2498,7 +2483,8 @@ info /= softc->media_blksize; } } - if (CCB_Type(csio) == SA_CCB_BUFFER_IO) { + if (csio->cdb_io.cdb_bytes[0] == SA_READ || + csio->cdb_io.cdb_bytes[0] == SA_WRITE) { bcopy((caddr_t) sense, (caddr_t) &softc->last_io_sense, sizeof (struct scsi_sense_data)); bcopy(csio->cdb_io.cdb_bytes, softc->last_io_cdb,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140731091422.GA37239>