Skip to content

rasdump and monitor updates, made rasdump work, improved bus abstraction#973

Merged
uweseimet merged 47 commits intodevelopfrom
feature-rasdump-and-monitor-update
Nov 9, 2022
Merged

rasdump and monitor updates, made rasdump work, improved bus abstraction#973
uweseimet merged 47 commits intodevelopfrom
feature-rasdump-and-monitor-update

Conversation

@uweseimet
Copy link
Copy Markdown
Contributor

@uweseimet uweseimet commented Nov 7, 2022

@akuker This PR makes rasdump work, based on the old bus code. I added a gpiobus factory like yours, which currently only returns a Raspberry Pi bus, and additionally initializes it so that the clients do not have to do this.
Re-adding your changes for the Banana Pi is a matter of 10 minutes. Essentially you just have to replace the files in the hal folder by the new code, and you have to ensure that the factory initializes the bus, i.e. calls bus->Init() and bus->Reset() because the clients don't need to do that mandatory work anymore.

Except for rasdump there are not that many changes, most of the changes are the replaced files in the hal folder.

From that perspective I think it makes sense to temporarily revert your Banana Pi changes. What you get in return is a working and improved rasdump, an better bus abstraction, SonarCloud analysis for rasdump and scsimon, and a lot of cleaned up and de-duplicated code. Sounds like a good deal to me ;-).

@uweseimet
Copy link
Copy Markdown
Contributor Author

uweseimet commented Nov 7, 2022

@rdmark In case @akuker agrees to proceed as I suggested it may be useful if the web UI supports creating drive images of real devices by calling the rasdump tool. As rasdump cannot use the bus at the same time rascsi does, rascsi would have to be told to relinquish bus control (remote interface).
In the ideal case rasdump would offer a protobuf remote interface, and would support scanning for connected devices. The UI could then display a list of available devices to dump. This means major changes to rasdump, though.
What's your opinion on that? A problem with that approach might be that a protobuf interface requires a running rasdump process which is listening on a socket. But the UI could just start rasdump when needed, and terminate it again after all work is done. A running process would also be able to receive commands from the UI while the dump process is running, so that the UI could display a progress bar, for instance. Doing this is probably mandatory, because otherwise it's impossible to know how long the dump will still take.

Even better: rasdump should be integrated into rascsi. If rascsi receives a command that requires to switch from target to initiator mode, it can just do that, and later switch back. All in all that is a major effort, but definitely less than adding a protobuf interface to rasdump and figuring out how to avoid conflicts between rascsi and rasdump, and starting/stopping either the one or the other. rascsi also knows the image folder, i.e. it knows where dumped images should be stored.
Integrating rasdump into rascsi without adding any remote functionality would be the first step towards this goal.

@uweseimet uweseimet marked this pull request as ready for review November 7, 2022 16:03
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Nov 7, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 4 Security Hotspots
Code Smell A 19 Code Smells

0.2% 0.2% Coverage
0.0% 0.0% Duplication

@rdmark
Copy link
Copy Markdown
Member

rdmark commented Nov 8, 2022

@uweseimet When you say rascsi would have to relinquish bus control, would that mean that the process has to be terminated and then started up again after rasdump is finished? Or do you have some other mechanism in mind, in case we don't go down the road of integrating rasdump into rascsi?

The one argument I could think of not to integrate rasdump into rascsi would be the Unix philosophy or small, modular programs that work in unison. Rascsi already has a huge amount of functionality built into it and is quite a memory intensive program. This is just my 2 cents however.

Regardless, having a user friendly front end for dumping physical hard disk images and immediately making them available to the user as image files sounds like a very powerful and handy feature, indeed. I would be up for trying to make that work in the Web Interface.

@uweseimet
Copy link
Copy Markdown
Contributor Author

uweseimet commented Nov 8, 2022

@rdmark Regarding relinquishing bus control: If there are two different processes only one can use the bus, the other has to relinquish it.
I agree with what you say about the Unix philosophy, but a process that listens on sockets is not a regular Unix tool that does something when being started and then pipes the output to the next tool and so on. We are dealing with server processes here. Having two separate processes is a much bigger effort and requires more hardware resources than having just one, and you would have to duplicate a lot of functionality, like dealing with the network and with protobuf messages. This is comparable to what you have with web servers: There are not several of them on the same machine just because you have several websites/domains. There is a single server instead with multiple virtual hosts.

But instead of adding new tickets with features like that it might be better to first clean up/resolve the existing ones. Or to reject them, because they are not realistic. There are roughly these categories of existing backend tickets:

  • Adding additional SCSI commands: This is the minority because at least for the existing device types there is not that much that can be added. As big as it is, the SCSI standard has a limited scope.
  • Completely new features: Tickets like tape drive support, SCSI/USB bridge, BIN/CUE support, action hooks etc. require man weeks or even man months. Tickets of that kind should not be accepted unless the author is also going to implement them. Who else than the author is going to work on those, considering that a lot of expert knowledge (SCSI, Samplers, other protocols, Linux kernel potentially) is required? As far as I can tell since this project started regarding C++ it has essentially been a one-man-show, and for some of these tickets you even need SCSI hardware you can verify against, e.g. a streamer.
  • Features just one or two users are interested in or plan to maybe be interested in. The X68000 stuff in particular, which is broken beyond repair. https://sonarcloud.io/component_measures?id=akuker_RASCSI&branch=develop says more than 1000 words. Who is going to maintain this code nightmare, which has more SonarCloud issues than it has lines of code? Who is going to add unit tests? This code should be discarded, because just like the SASI controller it binds resources and potentially blocks further development of other features from which more than just two users might profit from.
  • Tickets dealing with low-level protocol issues like Mac Plus write error: Not able to receive 512 bytes of data #656. These are tricky and might require signal analyzer hardware. Also see Mac Plus write error: Not able to receive 512 bytes of data #656 (comment). Fix/verify sleep during DATA IN/DATA OUT/STATUS phase #949 may also be relevant in this context.
  • Code improvements: Not that much left to do IMO when you do not take the X68000 stuff and scsimon into account. About 70-80% of the non-X68000 code are essentially new by now, and this new code is covered with unit tests very well. The code not covered often deals with files/sockets or not mockable (static) methods, where unit testing is hardly possible.

Just my two cents.

@akuker
Copy link
Copy Markdown
Member

akuker commented Nov 8, 2022

@rdmark In case @akuker agrees to proceed as I suggested it may be useful if the web UI supports creating drive images of real devices by calling the rasdump tool.

@uweseimet - in my opinion, I don't think we need to easily switch back and forth between rascsi mode and rasdump mode. IMHO, these are completely different use cases and most likely involve switching cables around (and we ALWAYS shut down our hardware before switching SCSI cables, right? ;-) )

Even better: rasdump should be integrated into rascsi. If rascsi receives a command that requires to switch from target to initiator mode, it can just do that, and later switch back. All in all that is a major effort, but definitely less than adding a protobuf interface to rasdump and figuring out how to avoid conflicts between rascsi and rasdump, and starting/stopping either the one or the other. rascsi also knows the image folder, i.e. it knows where dumped images should be stored. Integrating rasdump into rascsi without adding any remote functionality would be the first step towards this goal.

This scares me. I would have no objection to updating the web interface to be able to control rasdump. But IMHO, it should stop RASCSI, then run rasdump. Maybe a completely different tab or some way to differentiate that this is completely different functionality from normal rascsi operation.

Creating a wiki page with detailed instructions, screenshots and troubleshooting tips would accomplish about the same goal (IMHO).

@uweseimet
Copy link
Copy Markdown
Contributor Author

@akuker The problem with the cables is a very good point, which I did not think of yet ;-). Let's just forget the whole thing. From a software perspective this would have been interesting, but it causes too many practical issues.

@uweseimet uweseimet merged commit 1c0179e into develop Nov 9, 2022
@uweseimet uweseimet deleted the feature-rasdump-and-monitor-update branch November 9, 2022 07:40
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 this pull request may close these issues.

Test, fix and improve rasdump rasdump and monitor folders are not analyzed by SonarCloud No usable documentation on initiator mode

3 participants