Bug 200619
| Summary: | camcontrol trashes the drive modepage if it's changed | ||
|---|---|---|---|
| Product: | Base System | Reporter: | Enji Cooper <ngie> |
| Component: | bin | Assignee: | Enji Cooper <ngie> |
| Status: | Closed FIXED | ||
| Severity: | Affects Some People | CC: | benno, imp, scottl |
| Priority: | --- | Flags: | ngie:
mfc-stable10+
ngie: mfc-stable9+ |
| Version: | CURRENT | ||
| Hardware: | Any | ||
| OS: | Any | ||
Scott/Warner: does the attached patch look ok for commit? A commit references this bug: Author: ngie Date: Sun Oct 25 04:04:25 UTC 2015 New revision: 289915 URL: https://svnweb.freebsd.org/changeset/base/289915 Log: Use 't' (bit-field) not 'b' (bit-sized integral type) for describing MRIE (aka "Method of Reporting Informational Exceptions") in the SCSI mode database. T10/04-371 revision 2 (revision 4; page 2, table 1) describes it as a bit-field of 4 bits wide. 1. http://www.t10.org/ftp/t10/document.04/04-371r2.pdf This a recommit of head@r289913 to fix the original commit message, in particular: - I incorrectly claimed that unit change was 'i' -> 't'. - The spec I reference in this commit is 2 decades newer than the one noted in r289913. The fields in the SCSI mode database are more complete in the newer spec, so it'll be easier for someone to decipher this commit if need be later. - I screwed up the bug entry in the previous commit message Pointyhat to: ngie (for botching up r289913) PR: 200619 Reported by: Michael Baptist Submitted by: Lars Skodje Sponsored by: EMC / Isilon Storage Divisionf Changes: head/share/misc/scsi_modes A commit references this bug: Author: ngie Date: Sun Oct 25 04:37:01 UTC 2015 New revision: 289916 URL: https://svnweb.freebsd.org/changeset/base/289916 Log: Limit RESOLUTION_MAX to INT_MAX, not UINT_MAX (all spelled out) so the mode value isn't always clipped to -1 when (resolution * size) == 32, which would have been the case with values => {4i,32b,32t}. This seems to have been broken in r64382. MFC after: 1 week X-MFC with: r289915 PR: 200619 Reported by: Michael Baptist Submitted by: Lars Skodje Sponsored by: EMC / Isilon Storage Division Changes: head/sbin/camcontrol/modeedit.c A commit references this bug: Author: ngie Date: Thu Nov 5 07:48:48 UTC 2015 New revision: 290385 URL: https://svnweb.freebsd.org/changeset/base/290385 Log: MFC r289913,r289916: r289913: Use 't' (bits) not 'i' (bytes) for describing MRIE (aka "Method of Reporting Informational Exceptions") in the SCSI mode database as the field described in X3T10/94-190 (revision 4; page 2, table 1) [1.] is 4 bits wide, not 4 bytes wide 1. http://ftp.t10.org/ftp/t10/document.94/94-190r4.pdf Bug 200619 Reported by: Michael Baptist <mbaptist@isilon.com> Submitted by: Lars Skodje <lskodje@isilon.com> Sponsored by: EMC / Isilon Storage Division r289916: Limit RESOLUTION_MAX to INT_MAX, not UINT_MAX (all spelled out) so the mode value isn't always clipped to -1 when (resolution * size) == 32, which would have been the case with values => {4i,32b,32t}. This seems to have been broken in r64382. PR: 200619 Reported by: Michael Baptist Submitted by: Lars Skodje Sponsored by: EMC / Isilon Storage Division Changes: _U stable/10/ stable/10/sbin/camcontrol/modeedit.c stable/10/share/misc/scsi_modes A commit references this bug: Author: ngie Date: Sun Nov 8 00:50:46 UTC 2015 New revision: 290527 URL: https://svnweb.freebsd.org/changeset/base/290527 Log: MFstable/10 r290385: MFC r289913,r289916: r289913: Use 't' (bits) not 'i' (bytes) for describing MRIE (aka "Method of Reporting Informational Exceptions") in the SCSI mode database as the field described in X3T10/94-190 (revision 4; page 2, table 1) [1.] is 4 bits wide, not 4 bytes wide 1. http://ftp.t10.org/ftp/t10/document.94/94-190r4.pdf Bug 200619 Reported by: Michael Baptist <mbaptist@isilon.com> Submitted by: Lars Skodje <lskodje@isilon.com> Sponsored by: EMC / Isilon Storage Division r289916: Limit RESOLUTION_MAX to INT_MAX, not UINT_MAX (all spelled out) so the mode value isn't always clipped to -1 when (resolution * size) == 32, which would have been the case with values => {4i,32b,32t}. This seems to have been broken in r64382. PR: 200619 Reported by: Michael Baptist Submitted by: Lars Skodje Sponsored by: EMC / Isilon Storage Division Changes: _U stable/9/ _U stable/9/sbin/ _U stable/9/sbin/camcontrol/ stable/9/sbin/camcontrol/modeedit.c _U stable/9/share/ _U stable/9/share/misc/ stable/9/share/misc/scsi_modes |
Description =========== Something is broken in camcontrol in bsd10. It trashes a drive modepage if it changed. Here is some sample output. ls-10-min252-1# isi_radish -e camcontrol: value 0 is out of range for entry Report Count; clipping to -1 This will happen for other values if you attempt to change them as well. ls-10-min252-1# echo -e "Interval Timer: 1800" | camcontrol modepage da4 -m 0x1c -P 3 -e camcontrol: value 1800 is out of range for entry Interval Timer; clipping to -1 ls-10-min252-1# camcontrol modepage da4 -m 0x1c -P 3 Interval Timer: -1258291201 The line of code that fixes this is changing.... #define RESOLUTION_MAX(size) ((resolution * (size) == 32)? \ (int)0xffffffff: (1 << (resolution * (size))) - 1) to #define RESOLUTION_MAX(size) ((resolution * (size) == 32)? \ INT_MAX: (1 << (resolution * (size))) - 1) Even with this change in place the error goes away, but the value reported by cam control after "changing" the modepage field does not change. Essentially changing mode page fields is broken. Proposed Fix ============ diff --git a/sbin/camcontrol/modeedit.c b/sbin/camcontrol/modeedit.c index 00ab974..8262c3c 100644 --- a/sbin/camcontrol/modeedit.c +++ b/sbin/camcontrol/modeedit.c @@ -246,7 +246,7 @@ editentry_set(char *name, char *newvalue, int editonly) * currently workaround it (even for int64's), so we have to kludge it. */ #define RESOLUTION_MAX(size) ((resolution * (size) == 32)? \ - (int)0xffffffff: (1 << (resolution * (size))) - 1) + INT_MAX: (1 << (resolution * (size))) - 1) assert(newvalue != NULL); if (*newvalue == '\0') diff --git a/share/misc/scsi_modes b/share/misc/scsi_modes index 781b8f1..80752e7 100644 --- a/share/misc/scsi_modes +++ b/share/misc/scsi_modes @@ -106,7 +106,7 @@ {EBACKERR} t1 {LogErr} t1 {Reserved} *t4 - {MRIE} b4 + {MRIE} t4 {Interval Timer} i4 {Report Count} i4 } Reported by: Michael Baptist Submitted by: Lars Skodje Sponsored by: EMC / Isilon Storage Division