Add support for READ LONG(10) and WRITE LONG(10) ? #479
Labels
compatibility
Compatibility with particular computing platforms
major effort
New feature/change with significant implementation effort
Comments
|
@Pacjunk Can you please point out which function is calling itself? I don't think this happens anywhere. Of course, code for similar functionailty like READ(6/10/16) is now reused. In order to add READ/WRITE LONG you might try to copy Read10 and Write10 (see disk.cpp) first, then modify the copied code as needed (maybe no modification is needed at all in your case), and then add the respective AddCommand() calls to the Disk class constructor. In sasidev_ctrl.cpp you have to add eCmdWriteLong10 to the switch in the FlushUnit() method. Note that the difference between READ/WRITE and READ/WRITE LONG is NOT that the length is in bytes. This is from SBC-3: By the way, READ LONG (but not WRITE LONG) is legacy and not supported anymore since SBC-4 from 2020. |
|
I know it's legacy, but we're generally dealing with legacy machines here! From your text above:
I can have a go at it, but it's going to be a mess... I find this stuff confusing. I can see that there are two functions called "Read", this must be referring to the second one. As I say, my C++ skills are poor, and this sort of thing doesn't help. Thanks, |
|
@Pacjunk When I mentioned that these commands are legacy I was not my intention to say they should not be implemented. This was just meant to be a general note, but I should have been clearer on that. Yes, there are two Reads(). From their signature one can see which is the right one. Unfortunately, the RaSCSI project appears to lack C++ contributors, even though almost everything is coded in C++. @Pacjunk Can you attach a log on trace level that shows some of the READ LONG commands being sent? |
|
@Pacjunk I just had a quick look at the existing code. Currently the code can only deal with full blocks and not just a part of a block sequence (limited by the LONG transfer length). Changing this in order to support READ/WRITE LONG looks like a major change to me. Testing this would be difficult because you may be one of the few or even the only one with the required hardware. Unit tests would help, but there are none, and the automated testing issue (#17) has not been updated for more than a year. Nevertheless, adding a log (see my last comment) to this ticket would definitely be useful, in order to see the CDB parameters used. Implementation note: In case somebody wants to work on this ticket, I suggest not to change the existing READ/WRITE code, including the DATA IN and DATA OUT phase code at all, at least not in the first implementation iteration. The danger of breaking the existing code is simply too high, in particular because unit tests and test automation are missing. |
|
Here is the log extract... It is not reading anything meaningful, just testing the capability. It requests LBA0, length 0, then length 1, then length $0200, then seems to increment by 1 all the way up to $0400. So lots of commands. Don't know why it ignores the returned error. |
|
@Pacjunk Did you notice that $0204 is omitted during these incremental reads? Most likely this means that a sector size of 516 bytes is something special for OpenVMS. Any idea? I dimly remember that there were operating systems using/supporting unusual sector sizes. Maybe the additional 4 bytes are supposed to contain a checksum for the respective sector or other meta data related to that sector. It's interesting that a READ LONG with a transfer length of 0 bytes is sent. I wonder which information OpenVMS derives from the result. If it were just for finding out whether READ LONG is supported at all, the SCSI error response should tell. On the other hand, if you wanted to find out whether READ LONG is supported you would probably try with a length of 0. If you deduce from the result that it is actually supported you might continue with the minimum length you would need in order to store data, e.g. 512 bytes. And if you wanted to also find out whether you can store sector-related meta data, you might try to find out how many excess bytes there are by reading again and again, and each time incrementing the length by 1. All in all you would end up with a sequence similar to what we see in the logs. I wonder whether OpenVMS just expects a different SCSI error message in order to deduce that READ LONG is not available at all. https://forums.gentoo.org/viewtopic-p-3502086.html?sid=5a023a282ae59b47c4591923e546e80e mentions drives formatted with a block sizes of 516 bytes, by the way. I have a FUJITSU drive which can be formatted with different sector sizes, but I never had any use for sizes other than 512, 1024 and 2048. |
|
@Pacjunk I re-read the spec and this may be important: This is not what RaSCSI currently does. I wonder if I should give it a try and implement a partial READ LONG, which in this special case does not return an error, but does in all other cases. This would not require substantial code changes because no data have to be returned. |
|
@Pacjunk Now it's your turn: Feature branch is 'feature_read_long'. The implementations for READ/WRITE LONG 10 accept a transfer length of 0 and in this case do not return an error. Any other length results in INVALID_FIELD_IN_CDB, which is IMO compliant with the standard. |
|
Yes, I was thinking of this as a first step. Just return a success for the 0 length request and see what happens. I just had a look at the dump I did from bluescsi, and the $0204 length is in there so I don't think there is anything tricky going on. Maybe too much logging and it skipped one - it does take a long time to do things when trace logging is turned on. The READ (10) command also specifies that 0 blocks is not an error! I will checkout your change, but don't think I'll be able to test it tonight (10pm now). Will get to it in the morning. |
|
@Pacjunk I don't think that READ(10) currently reports an error if 0 blocks is specified, i.e. the implementation of READ(10) should be fine. @Pacjunk When looking at your log's timestamps I don't see that the command with length $0204 is missing in the logs, but it was never sent. Please double-check, but I don't see a gap in the timestamps between the last log message of $0203 and the first in $0205, i.e. there cannot have been any command in between. |
|
Log excerpts from testing with an Atari, trying to transfer 0, 1 and 512 bytes: I ran the same tests against a real IBM DDRS-39130 drive. The drive was fine with a transfer length of 0, but refused to transfer 1 or 512 bytes. For these refused transfer lengths the IBM drive reported the same SCSI error codes as RaSCSI, i.e Sense Key $05 and ASC $24. |
|
Here is the next dump... I don't know why $0204 is still missing. It still steps through to $0400, and the drive is marked as not compatible. |
|
The strange thing is that the debug from bluescsi shows $0204! Strange... |
|
@uweseimet I tentatively added this to the 21.11 roadmap. If you can figure out a stable solution in the next day or two we can get it into the upcoming release. |
|
@Pacjunk I'm afraid there is nothing more I can do. In case BlueSCSI behaves better somebody else might compare logs, SCSI error codes and source code in order to proceed. As already mentioned, the fact that $0204 is missing with RaSCSI but not with BlueSCSI may be an important detail, caused by a difference in behavior between BlueSCSI and RaSCSI. It might even be the case that the difference in behavior ($0204) is not related to READ LONG at all, but to results from other commands sent before. |
|
OK, Thanks for your help |
|
I have implemented READ/WRITE LONG(10) on the bluescsi, and now the host only sends readlong(10) commands for length 0, 1 and 200. It seems to be happy with that and does not do the command again. My implementation just transfers bytes instead of blocks (as the spec indicated). I can now successfully shadow (mirror) two drives under the operating system! Probably not a lot of use, but good for learning/experimentation. |
uweseimet
added
the
major effort
|
@Pacjunk I would like to get back to this ticket. I am still wondering how to improve READ/WRITE LONG support. Without explicit support for variable length byte transfers (rascsi essentially can only transfer full blocks) it cannot be properly resolved. The SCSI printer device is also suffering from this and currently uses a work-around. |
|
My code reads/writes and transfers the requested number of bytes from/to the LBA specified. I just wrote another function similar to the one that transfers blocks, but transfers bytes instead. I don't know what data the machine is trying to get, but I supply real data. I think the mirroring feature that requires this will transfer different quantities when required. The 0,1 & 200 byte transfers are only done when the volumes are mounted to test compatibility. Supplying dummy data may work at this stage, but definitely won't work when a drive mirroring operation is required! |
|
@Pacjunk Please correct me if I am wrong: This means that when a sequence of sectors is copied this is done in chunks of 200 bytes, instead of copying in chunks of multiples of the sector size? |
|
The 200 bytes is just a compatibility test at mount time. The actual transfers are all different lengths. |
|
@Pacjunk So there are no additional data (checksums, vendor-specific data) contained in the data transferred by READ/WRITE LONG in your case? The only difference between copying a drive with regular read/write operations is that sequences of bytes instead of blocks are copied? |
|
@uweseimet Yes, I just read/write bytes from the file and transfer bytes to/from the system. No other data. What the data contains, I don't know, and don't care! The drive shadowing (mirroring) operation works successfully on my system, so what I am doing must be sufficient in this case. |
|
@Pacjunk I see. Any rascsi implementation would most likely have to bypass the sector-based cache and directly access the image file. Before each READ LONG the sector cache woulld have to be flushed in order for the file to be up to date before reading. (This is also required by the specification.) Any WRITE LONG would have to somehow clear the cache, so that the modified data are immediately available for a subsequent sector-oriented access. |
|
Another advantage of bluescsi - No cache! (which also means you can turn it off without corrupting images) |
|
Well, I think I already indicated that I would not mind for the cache to be removed completely. @akuker should probably comment on that because he is/was working on #335. Removing the cache would quite likely also help resolving #807. As far as I can tell the cache is negatively affecting 3 tickets so far. |
|
These are the alternatives I would like to propose in order to make progress on this ticket and on the related ones:
You won't be surprised that I am in favor of the first alternative ;-). |
uweseimet
added
the
feedback missing
|
While improving the code I noticed that there is no cache for SCMO, but only for other mass storage devices. Except for some mode pages SCMO is identical with SCHD/SCRM. In order to evaluate the effectiveness of the cache I used the same image file for SCHD and SCMO and measured the read transfer rates. You would have expected SCHD transfers (of the same data, so that they are cached) to be faster than SCMO transfers, but there was no difference. @akuker By the way, this ticket and others (assigned to you) are still waiting for your feedback. |
|
@uweseimet Unfortunately I am away for a week (possibly more) and I cannot check at the moment... |
|
@Pacjunk No problem I'll wait :). |
|
@Pacjunk After digging deeper into the code I may have been mistaken regarding the MO device not using the cache. Please do not spend any time on this (yet). |
This was referenced Oct 22, 2022
uweseimet
removed
the
feedback missing
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Info
Describe the issue
The OpenVMS SCSI driver "tests" the capability of the drive for the volume shadowing feature by sending hundreds of READ LONG(10) commands when mounting a volume. It seems to ignore the unsupported error message that this creates and keeps trying.
Is it possible to implement these commands - 1. To stop the above issue and 2. It might be fun to support volume shadowing.
I had a look at the code as I thought it might be easy since the CDB layout seems to be the same as the READ(10) and WRITE(10) commands, the only difference being that the length is in bytes, rather than blocks. I'm not a very good C++ programmer, so I find it confusing the way the code is spread amongst several files, there are multiple definitions of functions (which one to use?) and even functions calling themselves. I could start hacking it, but I fear the code will soon become a mess.
Can someone who is confident with the code have a look at the viability of this?
Thanks,
The text was updated successfully, but these errors were encountered: