-stable vs -current handling of SSD_KEY_MISCOMPARE SCSI sense data

[Scott Long]

fox at vader.aacc.edu wrote:
> While trying to diagnose an unrelated hardware problem, I stumbled on 
> the following code in RELENG_4 sys/cam/scsi/scsi_all.c (1.14.2.11):
> 
> scsi_interpret_sense(...)
> {
> 	...
> 	switch (error_code) {
> 	...
> 	case SSD_CURRENT_ERROR:
> 	{
> 		switch (sense_key) {
> 		...
> 		case SSD_KEY_MISCOMPARE:
> 			/* decrement the number of retries */
> 			retry = ccb->ccb_h.retry_count > 0;
> 			if (retry) {
> 				error = ERESTART;
> 				ccb->ccb_h.retry_count--;
> 			} else {
> 				error = EIO;
> 			}
> 		case SSD_KEY_RECOVERED_ERROR:
> 			error = 0;	/* not an error */
> 			break;
> 
> The lack of a break before "case SSD_KEY_RECOVERED_ERROR:" means that 
> all assignments to error in case "SSD_KEY_MISCOMPARE:" are expensive 
> comments, so I doubt that a /* FALLTHROUGH */ was intended. Also, this 
> code was added as part of 1.18, then MFC'd in 1.14.2.3, so it was most 
> likely deliberate.
> 
> However, -current seems to ignore this error. sys/cam/scsi/scsi_all.c 1.44 
> has:
> 
> const struct sense_key_table_entry sense_key_table[] =
> {
> 	...
> 	{ SSD_KEY_MISCOMPARE, SS_NOP, "MISCOMPARE" },
> 
> If I read the code in sys/cam/cam_periph.c correctly, this matches what
> -stable does, but should have s/SS_NOP/SS_RDEF/ to match what I suspect
> -stable was meant to do.
> 
> OTOH, if -stable is really meant to do nothing in that case, perhaps it 
> could do so in a more straightforward way, or have a comment added.
> 
> OTGH, I may be missing something obvious.
> 
> Perusal of cvs-all and freebsd-scsi archives from around the time 
> scsi_all.c 1.21 was commited failed to elicit obvious clues.
> 
> I eagerly await clue imparting, sarcasm, flamage, or LARTing.
> 

Thanks for bringing this up.  I'll review it today or tomorrow and
commit whatever is appropriate.

Scott