search

PiSCSI / piscsi

PiSCSI allows a Raspberry Pi to function as emulated SCSI devices (hard disk, CD-ROM, and others) for vintage SCSI-based computers and devices. This is a fork of the RaSCSI project by GIMONS.
https://piscsi.org
BSD 3-Clause "New" or "Revised" License
539 stars 82 forks source link

LUN requires deprecated LUN field and does not evaluate the LUN in the IDENTIFY message #317

Closed uweseimet closed 3 years ago

uweseimet commented 3 years ago

This ticket results from https://github.com/akuker/RASCSI/issues/309.

The LUN field (bits in byte 1 in the CDB) was deprecated in the SCSI standard long ago. The LUN has to be selected by an IDENTIFY message instead. Rascsi ignores the LUN in the IDENTIFY message and always expects the LUN in the CDB. This is not compliant to SCSI and causes the issue reported in https://github.com/akuker/RASCSI/issues/309.

Oct 11 21:00:07 raspberrypi RASCSI[432]: [2021-10-11 21:00:07.648] [trace] Message code IDENTIFY $C3
Oct 11 21:00:07 raspberrypi RASCSI[432]: [2021-10-11 21:00:07.648] [trace] virtual void SASIDEV::Command() Command Phase
Oct 11 21:00:07 raspberrypi RASCSI[432]: [2021-10-11 21:00:07.648] [trace] virtual void SASIDEV::Command() Command Byte 0: $12
Oct 11 21:00:07 raspberrypi RASCSI[432]: [2021-10-11 21:00:07.648] [trace] virtual void SASIDEV::Command() Command Byte 1: $00
Oct 11 21:00:07 raspberrypi RASCSI[432]: [2021-10-11 21:00:07.648] [trace] virtual void SASIDEV::Command() Command Byte 2: $00
Oct 11 21:00:07 raspberrypi RASCSI[432]: [2021-10-11 21:00:07.648] [trace] virtual void SASIDEV::Command() Command Byte 3: $00
Oct 11 21:00:07 raspberrypi RASCSI[432]: [2021-10-11 21:00:07.648] [trace] virtual void SASIDEV::Command() Command Byte 4: $FF
Oct 11 21:00:07 raspberrypi RASCSI[432]: [2021-10-11 21:00:07.648] [trace] virtual void SASIDEV::Command() Command Byte 5: $00
Oct 11 21:00:07 raspberrypi RASCSI[432]: [2021-10-11 21:00:07.648] [trace] virtual void SCSIDEV::Execute() Execution phase command $12
Oct 11 21:00:07 raspberrypi RASCSI[432]: [2021-10-11 21:00:07.648] [debug] ++++ CMD ++++ virtual void SCSIDEV::Execute() Executing command $12
Oct 11 21:00:07 raspberrypi RASCSI[432]: [2021-10-11 21:00:07.649] [debug] virtual bool Disk::Dispatch(SCSIDEV*) Executing Inquiry ($12)

The command above has to be executed for LUN 3, but rascsi executes it for LUN 0.

uweseimet commented 3 years ago

@Pacjunk There is a first code update available, addressing support for multiple LUNs, and also support for getting the LUN from the IDENTIFY message. Would be great if you could check with your setup if something has improved. Please use the very lastest lun_handling_update branch. Please attach a log on trace level.

uweseimet commented 3 years ago

@akuker I'm afraid I'm getting these bogus email messages again ...

[akuker/RASCSI] Generate a RaSCSI OS image, based upon the official Rapsberry Pi
+OS workflow run

Repository: akuker/RASCSI
Workflow: Generate a RaSCSI OS image, based upon the official Rapsberry Pi OS
Duration: 11 minutes and 40.0 seconds
Finished: 2021-10-11 14:23:37 UTC

View results: https://github.com/akuker/RASCSI/actions/runs/1329317027

Jobs:
  * build failed (1 annotation)
rdmark commented 3 years ago

@uweseimet I think the bogus messages are from akuker testing and trying to set up github CI/CD for the repo, ultimately with the goal of having automated Pi image generation.

uweseimet commented 3 years ago

@rdmark I know, but it would be good if this was tested on a separate branch. Looks as if once again any branch is affected.

uweseimet commented 3 years ago

@Pacjunk When testing, can you please also run a test with more than 8 LUNs for one of your devices? Up to 32 LUNs (just as supported by the SCSI standard) should now work with rascsi. It would be interesting to see of your computer makes use of more than 8 LUNs.

Pacjunk commented 3 years ago

Not sure how I would do that. I just get the machine to list devices, I don't get to control how many it looks for (OK just read #318 - I understand now). I'll update now and test a bit later today (sorry timezones are slowing things down!!)

Pacjunk commented 3 years ago

OK. It is looking good! With only 1 drive presented the console and OS only show one LUN (as LUN 0). Log attached. Trace1.log

Pacjunk commented 3 years ago

Another log with 2 images (2 & 3) with the OS running Trace2.log .

Pacjunk commented 3 years ago

Next up I created 8 LUNs for ID2, so all populated. All the LUNs show up and are confirmed as individual drives since I gave each a unique filesystem label. The trace log is huge though rascsi.log !

Pacjunk commented 3 years ago

All looking good and performing as expected. The system cannot see more than 8 LUNs per ID. I actually created 10 using the rasctl command. When I got to create LUN 10, it complains that LUN 1 already exists. Doesnt matter since I can only use up to LUN 7 anyway.

One concern that I have is that the log has a lot of these messages:

[warning] ID 2 LUN 5 received unsupported command: $3E

Pacjunk commented 3 years ago

... and for my final party trick - here is the system with 32 LUNs mounted. (4 IDs x 8 LUNs)

Device               Device           Error    Volume         Free  Trans Mnt
 Name                Status           Count     Label        Blocks Count Cnt
$1$$DKA0:            Mounted              0  BF0728A4BA   140200868   315   1
$1$$DKA100:          Mounted              0  LUN10            20444     1   1
$1$$DKA101:          Mounted              0  LUN11            20444     1   1
$1$$DKA102:          Mounted              0  LUN12            20444     1   1
$1$$DKA103:          Mounted              0  LUN13            20444     1   1
$1$$DKA104:          Mounted              0  LUN14            20444     1   1
$1$$DKA105:          Mounted              0  LUN15            20444     1   1
$1$$DKA106:          Mounted              0  LUN16            20444     1   1
$1$$DKA107:          Mounted              0  LUN17            20444     1   1
$1$$DKA200:          Mounted              0  LUN20           648927     1   1
$1$$DKA201:          Mounted              0  LUN21            20444     1   1
$1$$DKA202:          Mounted              0  LUN22            20444     1   1
$1$$DKA203:          Mounted              0  LUN23            20444     1   1
$1$$DKA204:          Mounted              0  LUN24            20444     1   1
$1$$DKA205:          Mounted              0  LUN25            20444     1   1
$1$$DKA206:          Mounted              0  LUN26            20444     1   1
$1$$DKA207:          Mounted              0  LUN27            20444     1   1
$1$$DKA300:          Mounted              0  LUN30            20444     1   1
$1$$DKA301:          Mounted              0  LUN31            20444     1   1
$1$$DKA302:          Mounted              0  LUN32            20444     1   1
$1$$DKA303:          Mounted              0  LUN33            20444     1   1
$1$$DKA304:          Mounted              0  LUN34            20444     1   1
$1$$DKA305:          Mounted              0  LUN35            20444     1   1
$1$$DKA306:          Mounted              0  LUN36            20444     1   1
$1$$DKA307:          Mounted              0  LUN37            20444     1   1
$1$$DKA400:          Online wrtlck        0
$1$$DKA600:          Mounted              0  LUN60            20444     1   1
$1$$DKA601:          Mounted              0  LUN61            20444     1   1
$1$$DKA602:          Mounted              0  LUN62            20444     1   1
$1$$DKA603:          Mounted              0  LUN63            20444     1   1
$1$$DKA604:          Mounted              0  LUN64            20444     1   1
$1$$DKA605:          Mounted              0  LUN65            20444     1   1
$1$$DKA606:          Mounted              0  LUN66            20444     1   1
$1$$DKA607:          Mounted              0  LUN67            20444     1   1
$1$$DVA0:            Online               0
$
pi@raspberrypi:~/RASCSI $ rasctl -l
+----+----+------+-------------------------------------
| ID | UN | TYPE | DEVICE STATUS
+----+----+------+-------------------------------------
|  1 |  0 | SCHD | /home/pi/images/k.hds
|  1 |  1 | SCHD | /home/pi/images/l.hds
|  1 |  2 | SCHD | /home/pi/images/m.hds
|  1 |  3 | SCHD | /home/pi/images/n.hds
|  1 |  4 | SCHD | /home/pi/images/o.hds
|  1 |  5 | SCHD | /home/pi/images/p.hds
|  1 |  6 | SCHD | /home/pi/images/q.hds
|  1 |  7 | SCHD | /home/pi/images/r.hds
|  2 |  0 | SCHD | /home/pi/images/DEC_RZ55.hds
|  2 |  1 | SCHD | /home/pi/images/a.hds
|  2 |  2 | SCHD | /home/pi/images/b.hds
|  2 |  3 | SCHD | /home/pi/images/c.hds
|  2 |  4 | SCHD | /home/pi/images/d.hds
|  2 |  5 | SCHD | /home/pi/images/e.hds
|  2 |  6 | SCHD | /home/pi/images/f.hds
|  2 |  7 | SCHD | /home/pi/images/g.hds
|  2 |  8 | SCHD | /home/pi/images/h.hds
|  2 |  9 | SCHD | /home/pi/images/i.hds
|  3 |  0 | SCHD | /home/pi/images/s.hds
|  3 |  1 | SCHD | /home/pi/images/t.hds
|  3 |  2 | SCHD | /home/pi/images/u.hds
|  3 |  3 | SCHD | /home/pi/images/v.hds
|  3 |  4 | SCHD | /home/pi/images/w.hds
|  3 |  5 | SCHD | /home/pi/images/x.hds
|  3 |  6 | SCHD | /home/pi/images/y.hds
|  3 |  7 | SCHD | /home/pi/images/z.hds
|  6 |  0 | SCHD | /home/pi/images/1.hds
|  6 |  1 | SCHD | /home/pi/images/2.hds
|  6 |  2 | SCHD | /home/pi/images/3.hds
|  6 |  3 | SCHD | /home/pi/images/4.hds
|  6 |  4 | SCHD | /home/pi/images/5.hds
|  6 |  5 | SCHD | /home/pi/images/6.hds
|  6 |  6 | SCHD | /home/pi/images/7.hds
|  6 |  7 | SCHD | /home/pi/images/8.hds
+----+----+------+-------------------------------------

Pretty impressive!

uweseimet commented 3 years ago

@Pacjunk Thanks a lot for your extensive testing. Regarding SCSI command 0x3E (READ LONG), this is not implemented by rascsi, and it is a command where the data transferred are vendor-specific. This is why it cannot really be emulated. This command is not supported by recent SCSI standards anymore, by the way. I changed the log level of this warning to trace, because there will always be cases where a command is not supported, but also not really needed by the computer. Sometimes a driver just uses certain commands for optimizations, and if these commands are not available falls back to others. I fixed the wrong LUN evaluation of rasctl, so that one can now also attach LUNs > 9 with rasctl.

Correct me if I am wrong, but all of your issues are resolved, aren't they?

Pacjunk commented 3 years ago

Pretty much. I did notice a few other things. If I accidentally removed a LUN using the GUI (bad I know), I got the error message about consecutive LUNs etc, but it would stop me from attaching a device to a totally different ID. e.g. removed 3:0, then couldnt mount a CD image on 5:0. Also could not put the deleted one back. Also If I attempted to create a non-consecutive LUN it would give me the error message, but then prevented me from attaching that file to another id:lun. Said it was already in use, when it wasn't. Luckily I had a saved configuration, so restoring that got me out of it. When I had all 32 HDDs attached, I had trouble adding a CD device with a profile. It ignored the profile and mounted as 2048 which doesnt work in my situation. Once I deleted a few LUNs, the CD attached fine. Boundary case, I know. What idiot would try to have 32 LUNs attached simulatenously then try to add more!

uweseimet commented 3 years ago

@Pacjunk I will try to address these special cases as far as the rascsi backend is concerned. In particular I will prevent detaching a LUN if this results in non-consecutive LUNs, because such a setup is incompatible with the SCSI standard. The REPORT LUNS command, for instance, would not work properly anymore in such a case.

Pacjunk commented 3 years ago

As far as my systems go, the Alpha is all good. No phantom LUNs, and detects them if extra LUNs are created. Occasionally the console would not detect them, but a quick reset fixed that, so an issue on the host side. The VAXstation can only see the LUNs on LUN 0, both at the console and the OS. This is OK and probably expected given that it uses an older SCSI controller. Thanks,

uweseimet commented 3 years ago

@Pacjunk If a platform only supports LUN 0 this is usually a driver issue, not a controller issue. I guess Atari 16/32 bit computers are even older than yours (first models arrived around 1985), but they support multiple LUNs. I use a SCSI memory card reader with them, and with that reader each slot (there are 5 of them for different memory card types) uses a separate LUN.

Pacjunk commented 3 years ago

Oh well, thats the way it is. These date from 88-89 and use a NCR5380. As you say, its more to do with the drivers both in the firmware and the OS. The alpha firmware is a lot more advanced and has to support fancy stuff like clustering over SCSI etc.

Any plans to put some LUN support into the GUI or is it rasctl only? Not a problem for me, though, as I would rarely use LUNs other than 0.

uweseimet commented 3 years ago

@Pacjunk The Atari computers with SCSI bus also use the NCR5380. Regarding the web UI @rdmark should comment. The upcoming Rascsi Control app has LUN support.

Pacjunk commented 3 years ago

Looks good. Thanks for all your hard work.

uweseimet commented 3 years ago

@Pacjunk The latest code in the lun_handling_update branch should address the issues with attaching/detaching from your previous comment, at least as far as the backend is concerned. @rdmark I don't think that the limitation to 32 devices/LUNs is a backend issue. Maybe the web UI blocks adding more?

uweseimet commented 3 years ago

@rdmark I think when detaching LUN 0 rascsi should automatically detach all other LUNs of the same device. This is equivalent to disconnecting a physical device with several LUNs: You can only disconnect the complete device, not individual LUNs. Would you agree? A more flexible alternative is to detach the specified LUN and all LUNs with higher IDs. Strictly speaking it should not even be possible to attach or detach individual LUNs with rasctl, because a real device always has a fixed number of LUNs right from the start. But in practice (also for testing purposes) it's probably useful if rasctl has this feature. All in all I don't expect major problems here. I guess that most drivers with LUN support would even cope with a LUN vanishing, because a SCSI error is reported in such a case when accesing this LUN.

Pacjunk commented 3 years ago

@rdmark I don't think that the limitation to 32 devices/LUNs is a backend issue. Maybe the web UI blocks adding more?

I was able to successfully attach the ISO (33rd LUN), it just didn't take the profile across with it. So instead or being an RRD42 with 512 sectors, it kept attaching as a generic scsi cd with 2048.

rdmark commented 3 years ago

@uweseimet The web ui makes assumptions in several places about having exactly 8 IDs available, and no explicit handling of LUNs, so no wonder there are bugs there. Once this ticket has been resolved, let's create another one for improving the web ui.

And if you want my opinion about detaching LUNs, speaking without deep knowledge on the topic, I like the idea of having more flexibility in rasctl, but in the GUI allow only detaching LUNs attached to one device as a group.

rdmark commented 3 years ago

@rdmark I don't think that the limitation to 32 devices/LUNs is a backend issue. Maybe the web UI blocks adding more?

I was able to successfully attach the ISO (33rd LUN), it just didn't take the profile across with it. So instead or being an RRD42 with 512 sectors, it kept attaching as a generic scsi cd with 2048.

This is definitely a web ui bug, since the profile reading is done by rascsi-web. If you don't mind, let me address this in another ticket since the web code will require a significant overhaul to support LUNs properly.

uweseimet commented 3 years ago

@rdmark I agree. Even with more flexibility in rasctl the UI can be more elaborate. rasctl simply was not designed to work on more than one device/LUN at the same time. Are you OK with the approach that rascsi detaches any LUN bigger or equal to the one that was explicitly detached? This ensures that there are no gaps, and at the same time does not prevent the UI from dealing with all LUNs for a device as a group. If it is documented behavior that rascsi detaches all LUNs bigger or equal to the one specified the UI could simply rely on everything being detached if LUN 0 is detached.

rdmark commented 3 years ago

@uweseimet Sounds like a good approach to me.

BTW, I'm looking forward to you officially announcing that Android app. It looks great. :)

uweseimet commented 3 years ago

@rdmark I just committed the change that detaches all LUNs equal to or higher than the one specified:

rasctl -l
+----+----+------+-------------------------------------
| ID | UN | TYPE | DEVICE STATUS
+----+----+------+-------------------------------------
|  0 |  0 | SCHD | /home/pi/images/test.hds (WRITEPROTECT)
|  1 |  0 | SCRM | /home/pi/images/zip.hds
|  3 |  0 | SCMO | /home/pi/images/test.mos
|  4 |  0 | SCCD | /home/pi/images/test.iso (WRITEPROTECT)
|  5 |  0 | SCDP | DaynaPort SCSI/Link
|  5 |  1 | SCHD | /home/us/hatari/slave.hds
|  5 |  2 | SCHD | /home/us/hatari/master.hds
+----+----+------+-------------------------------------
>rasctl -i 5 -c d
>rasctl -l
+----+----+------+-------------------------------------
| ID | UN | TYPE | DEVICE STATUS
+----+----+------+-------------------------------------
|  0 |  0 | SCHD | /home/pi/images/test.hds (WRITEPROTECT)
|  1 |  0 | SCRM | /home/pi/images/zip.hds
|  3 |  0 | SCMO | /home/pi/images/test.mos
|  4 |  0 | SCCD | /home/pi/images/test.iso (WRITEPROTECT)
+----+----+------+-------------------------------------

[2021-10-12 19:28:40.873] [info] Detached SCHD device with ID 5, unit 2
[2021-10-12 19:28:40.873] [info] Detached SCHD device with ID 5, unit 1
[2021-10-12 19:28:40.873] [info] Detached SCDP device with ID 5, unit 0
uweseimet commented 3 years ago

@rdmark Regarding the app, you might be the one to write such an app for iOS ;-). I guess there are more rascsi users with iOS than Android, and I have no plans to offer an iOS app.

rdmark commented 3 years ago

@uweseimet I've been testing the logic today, and it seems to work as expected. I also pushed webapp code to your branch to make the UI sensitive to these changes, as well as enabling attaching/detaching to specific LUNs.

@Pacjunk has promised to run some tests in their environment today. I suggest we merge the PR once they give the green light. :)

rdmark commented 3 years ago

@rdmark Regarding the app, you might be the one to write such an app for iOS ;-). I guess there are more rascsi users with iOS than Android, and I have no plans to offer an iOS app.

Haha, maybe I should. I am indeed an iPhone user. While it's a good platform for users, it's a frustrating platform to develop on in my experience, will all the restrictions, so we will see if I can muster the energy.

Pacjunk commented 3 years ago

OK, I've been playing around with this build this afternoon, trying to break things (as suggested) and I have been successful!

The issues I was having yesterday have been fixed, but I have found some more.

These are my findings so far:

  1. There seems to be some major problem with disk caching. Changes on the volumes are not being written through to the disk files on the SD card. This happens on single LUNs (0), or multiple. If I write changes to a disk or disks, and then restart the rascsi service or reboot the raspberry pi, then changes are not written to disk. After the restart, the mounted disks contain the old contents. The only way to flush the changes is to detach the disks. If you detach (or preferably use the "detach all" button, then the changes are flushed to the card. It is not the host that I am using. Everything is fine between reboots of the host. It is only when rascsi is restarted. Another indicator is the timestamps on the files don't update unless you detach.

I dont know what the caching algorithm is for rascsi, but all the changes I made when testing yesterday had vanished when I started the pi today. Either there needs to be some timed flush (for the situations where the pi crashes, or loses power etc), or at a minimum detach all the disks before the rascsi services is restarted or shutdown so that changes are flushed.

  1. In the web gui, if I try to attach a LUN number = 32, I get an error message saying range is 0-31. If I try to attach a LUN number >= 33, I get a page with "Internal Server Error" and all the drives are detached. Maybe your code is checking for =32, instead of >=32.

  2. The properties file doesn't care about extensions. If I create a properties file for an RRD42 CD-ROM drive Z.ISO, and then create Z.HDS and mount it, the hard drive will also be marked as a RRD42 CD-ROM. OK - don't do stupid things you say, but you told me to break things!

Comments on visual style, not bugs...

  1. The current list of attached drives seems to have a thicker horizontal line every 4 devices (on some browsers). Would it be better to have the thicker line between every SCSI ID so that LUNs are grouped?

  2. In the "Create Named Drive" section, the text says "Ceate a named disk image..." - just a typo

  3. In the list of valid image files, it is possible to flag which ones are already mounted? I know there is an error message when you try to mount one that is already mounted, but some sort of flag would make it easier to know which ones are in use.

I think that's it for now. I'll add more if I find more. Cheers,

uweseimet commented 3 years ago

@Pacjunk Since I did not touch any caching code, can you please check whether you stumble upon the same caching issues with the develop branch? I guess that any issue you may have found with the caching is an old issue. I would not be surprised if (for performance reasons) the caches are not constantly flushed, and they may simply not be flushed when you restart or reboot, because the rascsi process cannot react on that.

Pacjunk commented 3 years ago

Not expecting constantly, they don't seem to be flushed at all! I'll leave the system idle for a little while to see if it flushes. Nothing seen in the last hour though.

It will take me some time to get back to dev, but I'll give it a go.

Pacjunk commented 3 years ago

OK, it flushed when I copied about 1.5MB to the disk. My tests were only copying about 0.25MB to each disk just to check for writability, uniqueness etc.

uweseimet commented 3 years ago

@Pacjunk This confirms that this is just how rascsi works, and is not related to the LUN handling.

Pacjunk commented 3 years ago

Sorry, some of my other issues aren't related to LUN handling either. I just put a list together of stuff that I found.

uweseimet commented 3 years ago

@Pacjunk Anything else is related to the web UI, isn't it? I guess that @rdmark is going to take a look at it.

Pacjunk commented 3 years ago

Yep, pretty much

Pacjunk commented 3 years ago

Just a noob question, to get back to develop, I just do a "checkout develop"? Does this revert the files from this branch?

uweseimet commented 3 years ago

@Pacjunk Yes, just run "git checkout develop".

Pacjunk commented 3 years ago

OK, I've only ever checked out a later branch, never tried to go "backwards" so didn't know whether it would revert changed files. Thats what I did, so all good then.

Pacjunk commented 3 years ago

Yep, you're right. Same cache issue in develop branch.

uweseimet commented 3 years ago

@Pacjunk I suggest that you create a ticket for this, with information on how to reproduce the caching issue. Note, though, that I am most likely not going to address this ticket, but somebody else might.

rdmark commented 3 years ago

@Pacjunk Some top notch testing there! Thanks for the thorough report. :) Addressing each issue in order:

  1. This is related to a known issue with RaSCSI in that it doesn't shut down gracefully, which is tracked as https://github.com/akuker/RASCSI/issues/3. I've commented in the ticket that you created https://github.com/akuker/RASCSI/issues/321 with a quick fix workaround suggestion.

  2. I could actually not reproduce this. Perhaps you got a space or something in the input field? Anyhow, I'll add some HTML5 validation to all the text input fields that should simply prevent invalid inputs.

  3. Good catch, I didn't consider that. What I'd suggest is keeping the original file ending and just appending .properties. It breaks compatibility with properties files created with earlier develop and you'll have to rename your existing properties files to keep using them, but I think that should be acceptable...

  4. This is a bug in Chrome. It doesn't handle 1px / thin border calculation correctly. I tried finding a better style a few weeks ago, but whenever something looked better in Chrome, it looked bad in other browsers. Quick fix: Use Firefox instead. ;)

  5. Good catch, I'll correct the spelling.

  6. I've had this as an improvement idea in the back of my mind for a while. Let me see if it can be done without too much overhead.

rdmark commented 3 years ago

All the above have been addressed now in separate PRs!