On 2019-10-21 5:52 a.m., Hannes Reinecke wrote:
Hi all,
the 'result' field in the SCSI command is defined as having
4 fields. The top byte is declared as a 'driver_byte', where the
driver can signal some internal status back to the midlayer.
However, there is quite a bit of overlap between the driver_byte
and the host_byte, resulting in the driver_byte being used in
very few places, and mostly in legacy drivers.
Additionally, we have _two_ sets of definitions for the
last byte (status byte), which can specified either in SAM terms
or in the linux-specific terms, which are shifted right by one
from the SAM ones.
Needless to say, the linux-specific ones are declared obsolete
for years now.
And to make the confusion complete, both the status byte and
the driver byte have a byte for a valid sense code, resulting
in quite some confusion which of those bits to check.
This patchset does several things:
- remove the linux-specific status byte definitions, and use
the SAM values throughout
- replace the driver-byte values with either SAM ones (for sense
code checking) or host-byte definitions
- remove the driver-byte definitions
This is a brave change proposal. The masked_status has been tricked
up so it won't break user code. However the driver byte is exposed
by the sg v2, v3 and v4 interfaces which means via bsg device nodes,
sg devices nodes and many other block storage device nodes (e.g.
/dev/sdc and /dev/st1) via:
ioctl(<storage_dev>, SG_IO, ptr_to_v3_interface) .
So if there is any user space code out there that checks the
driver byte (e.g. 'sg_io_hdr::driver_status & DRIVER_SENSE') do we
have a problem?
If so, we could hack the DRIVER_SENSE case *** by putting it back
for the user space to see when the driver (e.g. sg) knows there
is sense data. What about the other values?
As usual, comments and reviews are welcome.
It is hard to make an omelette without breaking some eggs.
Doug Gilbert
Please note, commit 66cf50e65b18 ("scsi: qla2xxx: fixup incorrect
usage of host_byte") from 5.4/scsi-fixes is a prerequisite for
this patch series.
<snip>
*** Here is a snippet from sg3_utils library code:
if ((SAM_STAT_CHECK_CONDITION == scsi_status) ||
(SAM_STAT_COMMAND_TERMINATED == scsi_status) ||
(SG_LIB_DRIVER_SENSE == masked_driver_status))
return sg_err_category_sense(sense_buffer, sb_len);
Due to the logical OR, as long as SAM_STAT_CHECK_CONDITION is set
whenever there is sense, then we don't care about DRIVER_SENSE.
I believe this code comes from the days before auto-sense when say a
READ(6) would fail, send back a CHECK_CONDITION and the host would then
need to issue a REQUEST SENSE command to get the sense data. However
REQUEST SENSE could itself yield a CHECK_CONDITION. Hence DRIVER_SENSE
set, SAM_STAT_CHECK_CONDITION clear could be interpreted as the
initial command failing and the follow-up REQUEST SENSE succeeded; if
they are both set, then both commands failed (e.g. the disk has gone
away).
