7901 Devices that do not support setting WC keep generating ereports

Review Request #376 — Created Feb. 22, 2017 and submitted

yuripv
illumos-gate
master
7901
3786822...
general

Work by Jeffry Molanus.

When we issue the MODE SENSE(10) we should prior to that set the page control field to 1, and verify the setting is changeable before we just blindly modify the bit. the STEC zeusram does not support changing this bit and as result we generate sense data which triggers the ereport over and over and over again as we dont learn from our failures.

Use STEC ZeusRAM or other devices that don't support changing the WCE bit.

Tested also on:
pvscsi drives - don't support the Page Control "changeable".
seagate drives behind mpt_sas - support the Page Control "changeable" and report it as 1.

  • 0
  • 0
  • 6
  • 0
  • 6
Description From Last Updated
rm
  1. In general, this looks good, but could use a bit of clean up, especially since it's mostly a copy and paste of sd_get_write_cache_enabled().
  2. usr/src/uts/common/io/scsi/targets/sd.c (Diff revision 1)
     
     
    Why does this return an int if in the only place you use it you don't do anything with the error?
  3. usr/src/uts/common/io/scsi/targets/sd.c (Diff revision 1)
     
     
    Make the whitespace consistent? Either everything should just use tabs and be aligned or nothing should.
  4. usr/src/uts/common/io/scsi/targets/sd.c (Diff revision 1)
     
     
    This seems to all be copied and pasted from sd_cache_control(), but without any of the useful comments. Can we make the actual gathering of the data a common function then? This'll mean that the comments that explain why we do the test unit ready, etc. will actually exist and make sense.
    1. Moved the common code to sd_cache_common(), hopefully that's what you wanted to see.

  5. usr/src/uts/common/io/scsi/targets/sd.c (Diff revision 1)
     
     
    Ideally we'd modify sd_send_scsi_MODE_SENSE to actually take an argument for the PC field of this, rather than just oring in a magic number. Can we instead at least make the PC constants macros that are defined in sys/scsi/generic/mode.h rather than just a magic value?
  6. 
      
yuripv
yuripv
yuripv
rm
  1. In a couple places I noticed that you edited existing comments and transformed them into single line comments and stripped them of periods. Is there a reason you were changing that?

    1. That's just my (perverted) sense of style saying that single-line comments should look like that unless the are REALLY IMPORTANT - been reading freebsd's style(9) too much :-)

  2. usr/src/uts/common/io/scsi/targets/sd.c (Diff revision 4)
     
     
    I don't think you want the 'and' at the start of this line, replace it with a comma.
  3. usr/src/uts/common/io/scsi/targets/sd.c (Diff revision 4)
     
     
    You probably want to NULL and zero out the out pointers in the case of failure, rather than just leave pointers to freed memory around.
  4. 
      
yuripv
rm
  1. Ship It!
  2. 
      
yuripv
Review request changed

Status: Closed (submitted)

Loading...