Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BlueSCSI V2 returns wrong SCSI error codes for a non-existing LUN #112

Closed
uweseimet opened this issue Jan 15, 2024 · 13 comments · Fixed by #119
Closed

BlueSCSI V2 returns wrong SCSI error codes for a non-existing LUN #112

uweseimet opened this issue Jan 15, 2024 · 13 comments · Fixed by #119

Comments

@uweseimet
Copy link

Tested with the current firmware: When a SCSI driver sends REQUEST SENSE for LUN 7 to a BlueSCSI device configured with ID 0 and LUN 0 (i.e. no LUN specified in the image filename) BlueSCSI returns Sense Key $00, ASC $00.
The SCSI-2 specification says this shall be Sense Key $05, ASC $25:

7.5.3 Selection of an invalid logical unit

The target's response to selection of a logical unit that is not valid is
described in the following paragraphs.

The logical unit may not be valid because:
   a)     the target does not support the logical unit (e.g. some targets
          support only one peripheral device).  In response to an INQUIRY
          command, the target shall return the INQUIRY data with the
          peripheral qualifier set to the value required in 8.2.5.1.  In
          response to any other command except REQUEST SENSE, the target shall
          terminate the command with CHECK CONDITION status.  In response to
          a REQUEST SENSE command, the target shall return sense data.  The
          sense key shall be set to ILLEGAL REQUEST and the additional sense
          code shall be set to LOGICAL UNIT NOT SUPPORTED.
@erichelgeson
Copy link
Contributor

The SCSI2SD command code that we use has no support for LUNs at all, though this would be easy enough to add in. Does this have any practical effect on any current systems?

@uweseimet
Copy link
Author

I thought LUNs were supported because they were also supported in BlueSCSI v1? I don't know all current systems, but this issue interferes with correctly checking which devices are actually available. Otherwise I would not have stumbled upon it with the Atari ;-).
Actually, reporting that a LUN does not exist is not really an issue of LUN support, because when there is no LUN support at all the situation is essentially the same as if there was LUN support but a particular LUN is not present. In both cases the logical unit selected is not valid, only the reasons why are slightly different. The error message (SCSI error codes) would be the same.

@erichelgeson
Copy link
Contributor

So when you were checking with the Atari - what was the behavior you saw? (besides the wrong response)

Yes... I understand that LUN support is not needed to implement this.

@uweseimet
Copy link
Author

The problem is that depending on how the ID/LUN check to find existing devices is done it draws wrong conclusions of which LUN (device) is present or not.

@uweseimet
Copy link
Author

Regarding LUN support, this was available in BlueSCSI v1, wasn't it? See erichelgeson/BlueSCSI#191.
I am asking because if LUN support is missing in BlueSCSI v2 I need to update my website accordingly. I have already been contacted by users of Atari MegaSTEs. They need LUN support in order to use more than one emulated device with the MegaSTE's internal host adapter.

@erichelgeson
Copy link
Contributor

For MegaSTE there is the setting MapLunsToIDs https://github.com/BlueSCSI/BlueSCSI-v2/wiki/bluescsi.ini#maplunstoids

@uweseimet
Copy link
Author

uweseimet commented Jan 28, 2024

Sounds good, because this means that the information on my website is still correct.

@erichelgeson
Copy link
Contributor

Would this behavior match your expectations of the spec? If there are any other could you please share a s2pexec of the input/output you'd expect and I'll attempt to match that behavior.

REQUEST SENSE with valid lun

$ sudo s2pexec -i 1:0 -c 03:00:00:00:0E:00 -L trace 
[2024-01-28 17:00:26.391] [trace] Detected 'Raspberry Pi 4 Model B Rev 1.2'
[2024-01-28 17:00:26.392] [trace] Executing command REQUEST SENSE for device 1:0
[2024-01-28 17:00:26.392] [trace] Arbitration with initiator ID 7
[2024-01-28 17:00:26.393] [trace] Selection of target 1 with initiator ID 7
[2024-01-28 17:00:26.398] [trace] Handling MESSAGE OUT phase
[2024-01-28 17:00:26.401] [trace] Handling COMMAND phase
[2024-01-28 17:00:26.416] [trace] Handling DATA IN phase
[2024-01-28 17:00:26.427] [trace] Received 14 byte(s) in DATA IN phase, provided size was 14 bytes
[2024-01-28 17:00:26.427] [trace] Handling STATUS phase
[2024-01-28 17:00:26.430] [trace] Handling MESSAGE IN phase
[2024-01-28 17:00:26.430] [debug] Received 14 data bytes
00000000  f0:00:00:00:00:00:00:0a:00:00:00:00:00:00        '..............'

REQUEST SENSE with invalid lun

$ sudo s2pexec -i 1:1 -c 03:00:00:00:0E:00 -L trace 
[2024-01-28 17:00:35.604] [trace] Detected 'Raspberry Pi 4 Model B Rev 1.2'
[2024-01-28 17:00:35.605] [trace] Executing command REQUEST SENSE for device 1:1
[2024-01-28 17:00:35.605] [trace] Arbitration with initiator ID 7
[2024-01-28 17:00:35.605] [trace] Selection of target 1 with initiator ID 7
[2024-01-28 17:00:35.610] [trace] Handling MESSAGE OUT phase
[2024-01-28 17:00:35.613] [trace] Handling COMMAND phase
[2024-01-28 17:00:35.633] [trace] Handling DATA IN phase
[2024-01-28 17:00:35.643] [trace] Received 14 byte(s) in DATA IN phase, provided size was 14 bytes
[2024-01-28 17:00:35.643] [trace] Handling STATUS phase
[2024-01-28 17:00:35.646] [trace] Handling MESSAGE IN phase
[2024-01-28 17:00:35.646] [debug] Received 14 data bytes
00000000  f0:00:05:00:00:00:00:0a:00:00:00:00:25:00        '............%.'```

TEST UNIT READY with invalid lun (just showing the CHECK CONDITION is working)

$ sudo s2pexec -i 1:1 -c 00:00:00:00:00:00 -L trace 
[2024-01-28 17:00:02.193] [trace] Detected 'Raspberry Pi 4 Model B Rev 1.2'
[2024-01-28 17:00:02.194] [trace] Executing command TEST UNIT READY for device 1:1
[2024-01-28 17:00:02.194] [trace] Arbitration with initiator ID 7
[2024-01-28 17:00:02.194] [trace] Selection of target 1 with initiator ID 7
[2024-01-28 17:00:02.199] [trace] Handling MESSAGE OUT phase
[2024-01-28 17:00:02.202] [trace] Handling COMMAND phase
[2024-01-28 17:00:02.219] [trace] Handling STATUS phase
[2024-01-28 17:00:02.222] [trace] Handling MESSAGE IN phase
[2024-01-28 17:00:02.222] [warning] Device reported CHECK CONDITION (status code $02)
[2024-01-28 17:00:02.222] [trace] Executing command REQUEST SENSE for device 1:1
[2024-01-28 17:00:02.222] [trace] Arbitration with initiator ID 7
[2024-01-28 17:00:02.223] [trace] Selection of target 1 with initiator ID 7
[2024-01-28 17:00:02.230] [trace] Handling MESSAGE OUT phase
[2024-01-28 17:00:02.233] [trace] Handling COMMAND phase
[2024-01-28 17:00:02.249] [trace] Handling DATA IN phase
[2024-01-28 17:00:02.249] [trace] Received 14 byte(s) in DATA IN phase, provided size was 14 bytes
[2024-01-28 17:00:02.259] [trace] Handling STATUS phase
[2024-01-28 17:00:02.262] [trace] Handling MESSAGE IN phase
Error: ILLEGAL REQUEST (Sense Key $05), LOGICAL UNIT NOT SUPPORTED (ASC $25), ASCQ $00

@uweseimet
Copy link
Author

uweseimet commented Jan 30, 2024

Thank you for addressing this. My expectation is that BlueSCSI works just like the SCSI specification says, in order to prevent potential problems ;-).

I did not run my original test with s2pexec but with a tool on the Atari. This is why I can't run exactly the same test for verification, because that would require using an updated BlueSCSI firmware with the Atari.
But as far as I can tell the behavior is now correct. For double-checking I ran your commands against a real SCSI device (Plextor CD-ROM), and it reports almost the same:

>sudo ./s2pexec -i 5 -c 03:00:00:00:0E:00
00000000  70:00:00:00:00:00:00:0a:00:00:00:00:00:00        'p.............'
>sudo ./s2pexec -i 5:1 -c 03:00:00:00:0E:00
00000000  70:00:05:00:00:00:00:0a:00:00:00:00:25:00        'p...........%.'
>sudo ./s2pexec -i 5:1 -c 00:00:00:00:00:00
[2024-01-30 15:11:11.725] [warning] Device reported CHECK CONDITION (status code $02)
Error: ILLEGAL REQUEST (Sense Key $05), LOGICAL UNIT NOT SUPPORTED (ASC $25), ASCQ $00

s2p reports the same as the CD-ROM.

What I noticed is that BlueSCSI sets the Valid field, whereas the CD-ROM and s2p do not. Is this intended? I have to consider also doing this in s2p. I would have expected the CD-ROM to also set it.

@erichelgeson
Copy link
Contributor

My expectation is that BlueSCSI works just like the SCSI specification says, in order to prevent potential problems ;-).

Yes, my question was more - does your interpretation match my interpretation. Specs are long and full of info. Such the valid field:

What I noticed is that BlueSCSI sets the Valid field, whereas the CD-ROM and s2p do not. Is this intended?

Not intended - seems like it's been this way since the beginning of scsi2sd. Seems to make sense to adjust here for direct-access devices - some other devices require different Information in the field.

Side note - a full set of s2pexe commands to test compatibility against the scsi spec would be an interesting endeavor.

@uweseimet
Copy link
Author

On first sight it may be fine to set the Valid field, but maybe one should be (overly?) careful and not set it.

Regarding s2pexec, what do you mean with "full set of commands"? You can (at least I hope so) send any command.

@erichelgeson
Copy link
Contributor

On first sight it may be fine to set the Valid field, but maybe one should be (overly?) careful and not set it.

I agree - I'll only set it for direct-access device responses.

Regarding s2pexec, what do you mean with "full set of commands"? You can (at least I hope so) send any command.

eg: An integration test suite - that is all.

@uweseimet
Copy link
Author

Regarding s2pexec I am considering adding an interactive mode (also see uweseimet/scsi2pi#49), like I did it for s2pctl. This helps with using scripts for sending commands. But checking the results would probably not be very convenient, because these are either strings or files. The latter for any data returned in a DATA IN phase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants