#49 Issues with REQUEST SENSE

[Philip Pritchard]

Hi,

I'm currently running SCST (cloned from https://github.com/bvanassche/scst) on the svc-3.4.x branch where the latest commit is a30623edbc908763cc4987024e995babafe197b6.

And using mhvtl (from https://github.com/markh794/mhvtl) 1.6.3 - 05d4f6f293d9efb6f589232736632433dc5ec10b to create an IBM ULT3580-TD5 tape drive, then presenting that tape drive through scst_local to present it locally (just helped find where the issue was).

On a Ubuntu Server 18.04.5 with kernel 4.15.0-132-generic

Then running a test (size of file doesn't matter) to write a file to the tape and read back the data.
dd if=/dev/urandom of=testfile bs=1M count=8
dd if=testfile of=/dev/st1
dd if=/dev/st1 of=testfile.copy

The read fails with an I/O error as a filemark is hit:
dd: error reading '/dev/st1': Input/output error

Observations:

1. scst_prepare_request_sense() isn't returning a success status since its default return became -ENOMEM, which causes a spam of errors from scst_pre_dev_done().

Adding a 'res = 0;' just before the out label got past this.

1. When the REQUEST SENSE is processed and scst_request_sense_local() is called it seems to be finding a stored "Power-on or device reset" which was returned by an earlier TEST UNIT READY. I don't think the sense data should be returned on this command, but it gets copied instead of SCST requesting the correct sense data from mhvtl for the filemark. Then the sense data gets copied to the original command in scst_complete_request_sense() and eventually scst_store_sense() gets called and places the check condition back onto the target device (so the issue repeats each read).

I'm unsure what purpose storing sense data has outside of UNIT ATTENTION, if it has none could tgt_dev_sense/tgt_dev_valid_sense_len be removed in favour of only taking sense data off of the tgt_dev->UA_list?

Any help with this issue would be appreciated.

Thanks

[Bart Van Assche]

Thanks for having reported this.

A fix for scst_prepare_request_sense() has been checked in.

Storing sense data just before completing a command seems weird to me too. Does removing the scst_store_sense() call from scst_process_check_condition() help?

[Philip Pritchard]

Yes, if I remove the call to scst_store_sense() then it works as expected.

[Bart Van Assche]

The scst_store_sense() call has been removed from the trunk.