svn commit: r252214 - in stable/9: bin/chio sys/cam/scsi sys/sys
Kenneth D. Merry
ken at freebsd.org
Thu Jul 11 22:02:14 UTC 2013
On Thu, Jul 11, 2013 at 14:53:32 -0600, Kenneth D. Merry wrote:
> On Wed, Jul 10, 2013 at 20:58:33 +0200, Andre Albsmeier wrote:
> > On Wed, 10-Jul-2013 at 20:49:15 +0200, Kenneth D. Merry wrote:
> > > On Wed, Jul 03, 2013 at 07:43:49 +0200, Andre Albsmeier wrote:
> > > > On Wed, 03-Jul-2013 at 07:22:36 +0200, Kenneth D. Merry wrote:
> > > > > On Tue, Jul 02, 2013 at 07:53:33 +0200, Andre Albsmeier wrote:
> > > > > > On Tue, 25-Jun-2013 at 23:43:50 +0200, Kenneth D. Merry wrote:
> > > > > > > Author: ken
> > > > > > > Date: Tue Jun 25 21:43:49 2013
> > > > > > > New Revision: 252214
> > > > > > > URL: http://svnweb.freebsd.org/changeset/base/252214
> > > > > >
> > > > > > Hi Ken,
> > > > > >
> > > > > > > Log:
> > > > > > > MFC: 249658, 249701
> > > > > > >
> > > > > > > Update chio(1) and ch(4) to support reporting element designators.
> > > > > > >
> > > > > > > This allows mapping a tape drive in a changer (as reported by
> > > > > > > 'chio status') to a sa(4) driver instance by comparing the
> > > > > > > serial numbers.
> > > > > > >
> > > > > > > The designators can be ASCII (which is printed out directly), binary
> > > > > > > (which is printed in hex format) or UTF-8, which is printed in either
> > > > > > > native UTF-8 format if the terminal can support it, or in %XX notation
> > > > > > > for non-ASCII characters. Thanks to Hiroki Sato <hrs@> for the
> > > > > > > explanation and example UTF-8 printing code.
> > > > > > >
> > > > > > > chio.h: Modify the changer_element_status structure to add new
> > > > > > > fields and definitions from the SMC3r16 spec.
> > > > > > >
> > > > > > > Rename the original CHIOGSTATUS ioctl to OCHIOGTATUS and
> > > > > > > define a new CHIOGSTATUS ioctl.
> > > > > > >
> > > > > > > Clean up some tab/space issues.
> > > > > > >
> > > > > > > chio.c: For the 'status' subcommand, print the designator field
> > > > > > > if it is supplied by a device.
> > > > > > >
> > > > > > > scsi_ch.h: Add new flags for DVCID and CURDATA to the READ
> > > > > > > ELEMENT STATUS command structure.
> > > > > > >
> > > > > > > Add a read_element_status_device_id structure
> > > > > > > for the data fields in the new standard. Add new
> > > > > > > unions, dt_or_obsolete and voltage_devid, to hold
> > > > > > > and address data from either SCSI-2 or newer devices.
> > > > > > >
> > > > > > > scsi_ch.c: Implement support for fetching device IDs with READ
> > > > > > > ELEMENT STATUS data.
> > > > > > >
> > > > > > > Add new arguments to scsi_read_element_status() to
> > > > > > > allow the user to request the DVCID and CURDATA bits.
> > > > > >
> > > > > > This broke "chio status" when talking to my QUALSTAR TLS-8211 library:
> > > > > >
> > > > > > Jul 2 07:08:22 <kern.crit> server kernel: (ch0:ahd3:0:1:0): READ ELEMENT STATUS. CDB: b8 10 fd e8 00 01 03 00 04 00 00 00
> > > > > > Jul 2 07:08:22 <kern.crit> server kernel: (ch0:ahd3:0:1:0): CAM status: SCSI Status Error
> > > > > > Jul 2 07:08:22 <kern.crit> server kernel: (ch0:ahd3:0:1:0): SCSI status: Check Condition
> > > > > > Jul 2 07:08:22 <kern.crit> server kernel: (ch0:ahd3:0:1:0): SCSI sense: ILLEGAL REQUEST asc:24,0 (Invalid field in CDB)
> > > > > > Jul 2 07:08:22 <kern.crit> server kernel: (ch0:ahd3:0:1:0): Command byte 6 is invalid
> > > > > > Jul 2 07:08:22 <kern.crit> server kernel: (ch0:ahd3:0:1:0): Error 22, Unretryable error
> > > > > >
> > > > > > scsi_cmd->flags (Byte 6) must be zero for this library so I had to use
> > > > > > this workaround to make it work again until a better solution is available:
> > > > > >
> > > > > > --- sys/cam/scsi/scsi_ch.c.ORI 2013-06-26 13:38:54.000000000 +0200
> > > > > > +++ sys/cam/scsi/scsi_ch.c 2013-07-02 07:42:24.000000000 +0200
> > > > > > @@ -1245,8 +1245,8 @@
> > > > > > /* tag_action */ MSG_SIMPLE_Q_TAG,
> > > > > > /* voltag */ want_voltags,
> > > > > > /* sea */ softc->sc_firsts[chet],
> > > > > > - /* dvcid */ 1,
> > > > > > - /* curdata */ 1,
> > > > > > + /* dvcid */ 0,
> > > > > > + /* curdata */ 0,
> > > > > > /* count */ 1,
> > > > > > /* data_ptr */ data,
> > > > > > /* dxfer_len */ 1024,
> > > > > > @@ -1284,8 +1284,8 @@
> > > > > > /* voltag */ want_voltags,
> > > > > > /* sea */ softc->sc_firsts[chet]
> > > > > > + cesr->cesr_element_base,
> > > > > > - /* dvcid */ 1,
> > > > > > - /* curdata */ 1,
> > > > > > + /* dvcid */ 0,
> > > > > > + /* curdata */ 0,
> > > > > > /* count */ cesr->cesr_element_count,
> > > > > > /* data_ptr */ data,
> > > > > > /* dxfer_len */ size,
> > > > > >
> > > > > > -Andre
> > > > >
> > > > > Oops, sorry.
> > > > >
> > > > > We need to check the SCSI version to see whether to set those bits, and
> > > > > also fall back in case it doesn't work.
> > > > >
> > > > > I am on vacation and have very spotty net access. I can't do anything
> > > > > about it until I get back next week.
> > > > >
> > > > > Justin and Alan (CCed) can work on the fix, though.
> > > >
> > > > Take your time, for me it's working right now
> > > > with the above patch...
> > >
> > > Okay, I'm back and can take a look at this.
> >
> > Welcome back ;-)
> >
> > >
> > > Can you send me:
> >
> > Sure, I am happy to help.
> >
> > >
> > > camcontrol inquiry ch0 -v
> >
> > pass17: <QUALSTAR TLS-8211 227d> Removable Changer SCSI-2 device
> > pass17: Serial Number 0021009613
> > pass17: 3.300MB/s transfers
> >
> > > camcontrol cmd ch0 -v -c "12 0 0 0 ff 0" -i 255 "s58 i2 i2 i2 i2 i2 i2 i2 i2"
> >
> > 0 0 0 0 0 0 0 0
>
> Okay, that was helpful, thanks!
>
> > >
> > > I want to see what SCSI version this changer reports. The second command
> > > will print out the SCSI version descriptors, if any, that the changer
> > > reports.
> >
> > In case you need more info just drop me a note.
> >
> > Thanks for looking into this.
>
> Can you try the attached patch, and see how it works for you?
>
> It defaults to not setting those bits for changers that claim to be SCSI-2
> or below. And if a SCSI-3 or higher changer returns an Illegal Request
> type error, it will retry with the bits cleared to see whether that works.
>
> I also bumped the read element status timeout after some testing with a
> Spectra T-380.
>
> The patch is against head, but stable/9 shouldn't be much different.
Oops, here is the patch.
Ken
--
Kenneth Merry
ken at FreeBSD.ORG
-------------- next part --------------
==== //depot/users/kenm/FreeBSD-test4/sys/cam/scsi/scsi_ch.c#6 - /usr/home/kenm/perforce4/kenm/FreeBSD-test4/sys/cam/scsi/scsi_ch.c ====
*** /tmp/tmp.49464.18 Thu Jul 11 14:45:15 2013
--- /usr/home/kenm/perforce4/kenm/FreeBSD-test4/sys/cam/scsi/scsi_ch.c Thu Jul 11 14:43:14 2013
***************
*** 102,108 ****
static const u_int32_t CH_TIMEOUT_MOVE_MEDIUM = 100000;
static const u_int32_t CH_TIMEOUT_EXCHANGE_MEDIUM = 100000;
static const u_int32_t CH_TIMEOUT_POSITION_TO_ELEMENT = 100000;
! static const u_int32_t CH_TIMEOUT_READ_ELEMENT_STATUS = 10000;
static const u_int32_t CH_TIMEOUT_SEND_VOLTAG = 10000;
static const u_int32_t CH_TIMEOUT_INITIALIZE_ELEMENT_STATUS = 500000;
--- 102,108 ----
static const u_int32_t CH_TIMEOUT_MOVE_MEDIUM = 100000;
static const u_int32_t CH_TIMEOUT_EXCHANGE_MEDIUM = 100000;
static const u_int32_t CH_TIMEOUT_POSITION_TO_ELEMENT = 100000;
! static const u_int32_t CH_TIMEOUT_READ_ELEMENT_STATUS = 60000;
static const u_int32_t CH_TIMEOUT_SEND_VOLTAG = 10000;
static const u_int32_t CH_TIMEOUT_INITIALIZE_ELEMENT_STATUS = 500000;
***************
*** 122,133 ****
typedef enum {
CH_Q_NONE = 0x00,
! CH_Q_NO_DBD = 0x01
} ch_quirks;
#define CH_Q_BIT_STRING \
"\020" \
! "\001NO_DBD"
#define ccb_state ppriv_field0
#define ccb_bp ppriv_ptr1
--- 122,135 ----
typedef enum {
CH_Q_NONE = 0x00,
! CH_Q_NO_DBD = 0x01,
! CH_Q_NO_DVCID = 0x02
} ch_quirks;
#define CH_Q_BIT_STRING \
"\020" \
! "\001NO_DBD" \
! "\002NO_DVCID"
#define ccb_state ppriv_field0
#define ccb_bp ppriv_ptr1
***************
*** 396,401 ****
--- 398,411 ----
periph->softc = softc;
softc->quirks = CH_Q_NONE;
+ /*
+ * The DVCID and CURDATA bits were not introduced until the SMC
+ * spec. If this device claims SCSI-2 or earlier support, then it
+ * very likely does not support these bits.
+ */
+ if (cgd->inq_data.version <= SCSI_REV_2)
+ softc->quirks |= CH_Q_NO_DVCID;
+
bzero(&cpi, sizeof(cpi));
xpt_setup_ccb(&cpi.ccb_h, periph->path, CAM_PRIORITY_NORMAL);
cpi.ccb_h.func_code = XPT_PATH_INQ;
***************
*** 1208,1213 ****
--- 1218,1225 ----
caddr_t data = NULL;
size_t size, desclen;
int avail, i, error = 0;
+ int curdata, dvcid, sense_flags;
+ int try_no_dvcid = 0;
struct changer_element_status *user_data = NULL;
struct ch_softc *softc;
union ccb *ccb;
***************
*** 1239,1252 ****
cam_periph_lock(periph);
ccb = cam_periph_getccb(periph, CAM_PRIORITY_NORMAL);
scsi_read_element_status(&ccb->csio,
/* retries */ 1,
/* cbfcnp */ chdone,
/* tag_action */ MSG_SIMPLE_Q_TAG,
/* voltag */ want_voltags,
/* sea */ softc->sc_firsts[chet],
! /* dvcid */ 1,
! /* curdata */ 1,
/* count */ 1,
/* data_ptr */ data,
/* dxfer_len */ 1024,
--- 1251,1281 ----
cam_periph_lock(periph);
ccb = cam_periph_getccb(periph, CAM_PRIORITY_NORMAL);
+ sense_flags = SF_RETRY_UA;
+ if (softc->quirks & CH_Q_NO_DVCID) {
+ dvcid = 0;
+ curdata = 0;
+ } else {
+ dvcid = 1;
+ curdata = 1;
+ /*
+ * Don't print anything for an Illegal Request, because
+ * these flags can cause some changers to complain. We'll
+ * retry without them if we get an error.
+ */
+ sense_flags |= SF_QUIET_IR;
+ }
+
+ retry_einval:
+
scsi_read_element_status(&ccb->csio,
/* retries */ 1,
/* cbfcnp */ chdone,
/* tag_action */ MSG_SIMPLE_Q_TAG,
/* voltag */ want_voltags,
/* sea */ softc->sc_firsts[chet],
! /* dvcid */ dvcid,
! /* curdata */ curdata,
/* count */ 1,
/* data_ptr */ data,
/* dxfer_len */ 1024,
***************
*** 1254,1262 ****
/* timeout */ CH_TIMEOUT_READ_ELEMENT_STATUS);
error = cam_periph_runccb(ccb, cherror, /*cam_flags*/ CAM_RETRY_SELTO,
! /*sense_flags*/ SF_RETRY_UA,
softc->device_stats);
if (error)
goto done;
cam_periph_unlock(periph);
--- 1283,1320 ----
/* timeout */ CH_TIMEOUT_READ_ELEMENT_STATUS);
error = cam_periph_runccb(ccb, cherror, /*cam_flags*/ CAM_RETRY_SELTO,
! /*sense_flags*/ sense_flags,
softc->device_stats);
+ /*
+ * An Illegal Request sense key (only used if there is no asc/ascq)
+ * or 0x24,0x00 for an ASC/ASCQ both map to EINVAL. If dvcid or
+ * curdata are set (we set both or neither), try turning them off
+ * and see if the command is successful.
+ */
+ if ((error == EINVAL)
+ && (dvcid || curdata)) {
+ dvcid = 0;
+ curdata = 0;
+ error = 0;
+ /* At this point we want to report any Illegal Request */
+ sense_flags &= ~SF_QUIET_IR;
+ try_no_dvcid = 1;
+ goto retry_einval;
+ }
+
+ /*
+ * In this case, we tried a read element status with dvcid and
+ * curdata set, and it failed. We retried without those bits, and
+ * it succeeded. Suggest to the user that he set a quirk, so we
+ * don't go through the retry process the first time in the future.
+ * This should only happen on changers that claim SCSI-3 or higher,
+ * but don't support these bits.
+ */
+ if ((try_no_dvcid != 0)
+ && (error == 0))
+ softc->quirks |= CH_Q_NO_DVCID;
+
if (error)
goto done;
cam_periph_unlock(periph);
***************
*** 1284,1291 ****
/* voltag */ want_voltags,
/* sea */ softc->sc_firsts[chet]
+ cesr->cesr_element_base,
! /* dvcid */ 1,
! /* curdata */ 1,
/* count */ cesr->cesr_element_count,
/* data_ptr */ data,
/* dxfer_len */ size,
--- 1342,1349 ----
/* voltag */ want_voltags,
/* sea */ softc->sc_firsts[chet]
+ cesr->cesr_element_base,
! /* dvcid */ dvcid,
! /* curdata */ curdata,
/* count */ cesr->cesr_element_count,
/* data_ptr */ data,
/* dxfer_len */ size,
More information about the svn-src-all
mailing list