svn commit: r253274 - head/sys/cam/scsi
Ulrich Spörlein
uqs at FreeBSD.org
Mon Jul 15 15:14:02 UTC 2013
On Fri, 2013-07-12 at 17:09:50 +0000, Kenneth D. Merry wrote:
> Author: ken
> Date: Fri Jul 12 17:09:50 2013
> New Revision: 253274
> URL: http://svnweb.freebsd.org/changeset/base/253274
>
> Log:
> Fix a problem with READ ELEMENT STATUS that occurs on some
> changers that don't support the DVCID and CURDATA bits that were
> introduced in the SMC spec.
>
> These changers will return an Illegal Request type error if the
> bits are set. This causes "chio status" to fail.
>
> The fix is two-fold. First, for changers that claim to be SCSI-2
> or older, don't set the DVCID and CURDATA bits for READ ELEMENT
> STATUS. For newer changers (SCSI-3 and newer), we default to
> setting the new bits, but back off and try the READ ELEMENT STATUS
> without the bits if we get an Illegal Request type error.
>
> This has been tested on a Qualstar TLS-8211, which is a SCSI-2
> changer that does not support the new bits, and a Spectra T-380,
> which is a SCSI-3 changer that does support the new bits. In the
> absence of a SCSI-3 changer that does not support the bits, I
> tested that with some error injection code. (The SMC spec says
> that support for CURDATA is mandatory, and DVCID is optional.)
>
> scsi_ch.c: Add a new quirk, CH_Q_NO_DVCID that gets set for
> SCSI-2 and older libraries, or newer libraries that
> report errors when the DVCID/CURDATA bits are set.
>
> In chgetelemstatus(), use the new quirk to
> determine whether or not to set DVCID and CURDATA.
> If we get an error with the bits set, back off and
> try without the bits. Set the quirk flag if the
> read element status succeeds without the bits set.
>
> Increase the READ ELEMENT STATUS timeout to 60
> seconds after testing with a Spectra T-380. The
> previous value was 10 seconds, and too short for
> the T-380. This may be decreased later after
> some additional testing and investigation.
>
> Tested by: Andre Albsmeier <Andre.Albsmeier at siemens.com>
> Sponsored by: Spectra Logic
> MFC after: 3 days
>
> Modified:
> head/sys/cam/scsi/scsi_ch.c
>
> Modified: head/sys/cam/scsi/scsi_ch.c
> ==============================================================================
> --- head/sys/cam/scsi/scsi_ch.c Fri Jul 12 16:41:58 2013 (r253273)
> +++ head/sys/cam/scsi/scsi_ch.c Fri Jul 12 17:09:50 2013 (r253274)
> @@ -1284,8 +1342,8 @@ chgetelemstatus(struct cam_periph *perip
> /* voltag */ want_voltags,
> /* sea */ softc->sc_firsts[chet]
> + cesr->cesr_element_base,
> - /* dvcid */ 1,
> - /* curdata */ 1,
> + /* dvcid */ dvcid,
> + /* curdata */ curdata,
> /* count */ cesr->cesr_element_count,
> /* data_ptr */ data,
> /* dxfer_len */ size,
Are you sure? Coverity flags this as being in the wrong argument order
(there's no CID for this yet).
CID null (#2 of 2): Arguments in wrong order (SWAPPED_ARGUMENTS)
swapped_arguments: The positions of arguments curdata and dvcid are inconsistent with the positions of the corresponding parameters for "scsi_read_element_status(struct ccb_scsiio *, u_int32_t, void (*)(struct cam_periph *, union ccb *), u_int8_t, int, u_int32_t, int, int, u_int32_t, u_int8_t *, u_int32_t, u_int8_t, u_int32_t)". [show details]
1338 scsi_read_element_status(&ccb->csio,
1339 /* retries */ 1,
1340 /* cbfcnp */ chdone,
1341 /* tag_action */ MSG_SIMPLE_Q_TAG,
1342 /* voltag */ want_voltags,
1343 /* sea */ softc->sc_firsts[chet]
1344 + cesr->cesr_element_base,
1345 /* dvcid */ dvcid,
1346 /* curdata */ curdata,
1347 /* count */ cesr->cesr_element_count,
1348 /* data_ptr */ data,
1349 /* dxfer_len */ size,
1350 /* sense_len */ SSD_FULL_SIZE,
1351 /* timeout */ CH_TIMEOUT_READ_ELEMENT_STATUS);
And this is the definition:
1860void
1861scsi_read_element_status(struct ccb_scsiio *csio, u_int32_t retries,
1862 void (*cbfcnp)(struct cam_periph *, union ccb *),
1863 u_int8_t tag_action, int voltag, u_int32_t sea,
1864 int curdata, int dvcid,
1865 u_int32_t count, u_int8_t *data_ptr,
1866 u_int32_t dxfer_len, u_int8_t sense_len,
1867 u_int32_t timeout)
Looks like you want to swap those two lines above, thanks!
Cheers,
UlI
More information about the svn-src-head
mailing list