On Wed, 2022-04-27 at 14:32 -0400, David Jeffery wrote:
> With tape devices, the SCF_TREAT_READ_AS_NORMAL flag is used by the
> target
> subsystem to mark commands which have both data to return as well as
> sense data. But with pscsi, SCF_TREAT_READ_AS_NORMAL can be set even
> if
> there is no data to return. The SCF_TREAT_READ_AS_NORMAL flag causes
> the
> target core to call iscsit datain callbacks even if there is no data,
> which
> iscsit does not support. This results in iscsit going into an error
> state
> requiring recovery and being unable to complete the command to the
> initiator.
>
> This issue can be resolved by fixing pscsi to only set
> SCF_TREAT_READ_AS_NORMAL if there is valid data to return along side
> the
> sense data.
>
> Fixes: bd81372065fa ("scsi: target: transport should handle st
> FM/EOM/ILI reads")
> Signed-off-by: David Jeffery <djeffery@xxxxxxxxxx>
> Tested-by: Laurence Oberman <loberman@xxxxxxxxxx>
> ---
> drivers/target/target_core_pscsi.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/target/target_core_pscsi.c
> b/drivers/target/target_core_pscsi.c
> index ff292b75e23f..60dafe4c581b 100644
> --- a/drivers/target/target_core_pscsi.c
> +++ b/drivers/target/target_core_pscsi.c
> @@ -588,7 +588,7 @@ static void pscsi_destroy_device(struct se_device
> *dev)
> }
>
> static void pscsi_complete_cmd(struct se_cmd *cmd, u8 scsi_status,
> - unsigned char *req_sense)
> + unsigned char *req_sense, int
> valid_data)
> {
> struct pscsi_dev_virt *pdv = PSCSI_DEV(cmd->se_dev);
> struct scsi_device *sd = pdv->pdv_sd;
> @@ -681,7 +681,7 @@ static void pscsi_complete_cmd(struct se_cmd
> *cmd, u8 scsi_status,
> * back despite framework assumption that a
> * check condition means there is no data
> */
> - if (sd->type == TYPE_TAPE &&
> + if (sd->type == TYPE_TAPE && valid_data &&
> cmd->data_direction == DMA_FROM_DEVICE) {
> /*
> * is sense data valid, fixed format,
> @@ -1032,6 +1032,7 @@ static void pscsi_req_done(struct request *req,
> blk_status_t status)
> struct se_cmd *cmd = req->end_io_data;
> struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(req);
> enum sam_status scsi_status = scmd->result & 0xff;
> + int valid_data = cmd->data_length - scmd->resid_len;
> u8 *cdb = cmd->priv;
>
> if (scsi_status != SAM_STAT_GOOD) {
> @@ -1039,12 +1040,11 @@ static void pscsi_req_done(struct request
> *req, blk_status_t status)
> " 0x%02x Result: 0x%08x\n", cmd, cdb[0], scmd-
> >result);
> }
>
> - pscsi_complete_cmd(cmd, scsi_status, scmd->sense_buffer);
> + pscsi_complete_cmd(cmd, scsi_status, scmd->sense_buffer,
> valid_data);
>
> switch (host_byte(scmd->result)) {
> case DID_OK:
> - target_complete_cmd_with_length(cmd, scsi_status,
> - cmd->data_length - scmd->resid_len);
> + target_complete_cmd_with_length(cmd, scsi_status,
> valid_data);
> break;
> default:
> pr_debug("PSCSI Host Byte exception at cmd: %p CDB:"
We added a bucnh of debug to track this down, and the fix is solid and
makes perfect sense.
Reviewed-by: Laurence Oberman <loberman@xxxxxxxxxx>
