search

uweseimet / scsi2pi

Advanced performant SCSI/SASI emulation and tools for the PiSCSI/RaSCSI board
https://www.scsi2pi.net
BSD 3-Clause "New" or "Revised" License
10 stars 2 forks source link

Default location for image file folder is doesn't match documentation when running s2p as root #40

Closed fdanapfel closed 10 months ago

fdanapfel commented 10 months ago

When creating a new ticket please provide information on your environment.

Describe the issue

According to the man-page for s2p the default location for the image file folder should be ˜/images’: The default folder is ’˜/images’ Since s2p is running as root the default location should therefore be /root/images.

But when checking the default settings of the running s2p process it shows /home/pi/images instead: $ /opt/scsi2pi/bin/s2pctl -s|grep "image file folder" Default image file folder: /home/pi/images

Since the pi user doesn't exist anymore by default on bookworm it would be better to not have /home/pi/images as the default location, but maybe use just /images instead.

uweseimet commented 10 months ago

@fdanapfel IMO there is a problem with staying compatible with piscsi here. When you change the behavior as you suggest the default folder when running s2p as a service (or launching it as root) would not be "/home/pi/images" anymore, which breaks the compatibility. Any idea how to address this properly? (If this cannot be resolved I may have to update the manpage.)

uweseimet commented 10 months ago

@fdanapfel I might have a resolution: s2p should support a comma-separated list of default image folders. When searching for an image s2p would search in these folders, in the order in which they are listed. Non-existing folders are ignored and not treated as errors. This mean you could have a default list like "~/images, /home/pi/images", which would be backwards compatible but at the same time consistent also for the root user. Would you agree that this is a possible solution?

fdanapfel commented 10 months ago

@uweseimet The main reason I've opened this issue was because the information provided in the documentation does not match what actually used in the binary.

So the basic fix would be to either change the documentation to match the default used in the binary, or change the binary to use the default location mentioned in the documentation.

I don't know how to address this properly with regards to staying compatible with the current setup used by piscsi.

But I think piscsi will have to be changed as well and move away from the assumption that the pi user will always exist and everything is installed /home/pi (I saw that there is already an open issue when trying to use a user other than pi when installing piscsi: https://github.com/PiSCSI/piscsi/issues/1366).

So having a default location or a list of locations for the images folder hardcoded in the binary might not be the best option long term. And since s2p now has the option to configure the properties via the configuration file it might be better to tell users to define the location for the images folder in the properties file if they want to run s2p as a system service.

uweseimet commented 10 months ago

@fdanapfel Thank you for your feedback. I have not made up my mind, but the minimum I will do is update the documentation. Having a list of folders might also have other benefits, and it will not impose additional complexity on the configuration because a list with just a single file is also valid, like before. What I am not going to do is wait for PiSCSI to address topics like that. A major reason why I decided to set up SCSI2Pi was kind of a "things should stay as they are" approach in the PiSCSI project. There were other tickets dealing with making the setup more standard and flexible, and they were all ignored.

uweseimet commented 10 months ago

I have updated the s2p manpage to reflect the current behavior. For a future release (not 1.2) I will create a follow-up ticket dealing with a better solution, based on what has been outlined in this ticket.