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

Add manual scsi probe of a Lun #84

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

alessandrocarminati
Copy link
Collaborator

This commit introduces manual probing of a specified SCSI LUN. The LUN can be specified either via the kernel command line scsi.addr=<host>:<channel>:<target>:<lun> or by adding a line to the initoverlayfs.config file scsi.addr <host>:<channel>:<target>:<lun>. In case both are present, the configuration takes precedence. Any operation depends on the presence of the scsi_mod.scan=manual kernel argument.
If it is not present in the kernel command line, no action will occur. The directory scsi_probe contains the main functions needed to complete the task and some related tests.
To run the tests, simply run make in the scsi_probe subdirectory.

@ericcurtin
Copy link
Collaborator

ericcurtin commented Apr 22, 2024

I can't wait to give this a run!

I'm probably taking a few day off from Wednesday (I'll be out around 7 days), so I dunno if I will get the opportunity to look at this in detail.

But it's pretty ifdef'd out, so I don't mind merging this if it helps things progress.

@alessandrocarminati
Copy link
Collaborator Author

Take your time; I took mine, so I can't complain if things go long.
In any case, to be more confident about the code quality, I bothered to add tests to my functions.
I hope it helps to maintain the quality of this contribution and not degrade the rest.

@@ -518,7 +526,7 @@ static int switchroot(const char* newroot) {
}

static int mount_proc_sys_dev(void) {
if (false) {
if (true) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This statement here is not ifdef'd.
For some reason, I thought it should be enabled, no matter what.
On a second thought, I wonder if you prefer this to be ifdef dependent.

Copy link
Collaborator

@ericcurtin ericcurtin Apr 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was false deliberately because in the use case this code was intended for initoverlayfs is the init system, but exec's itself as a systemd process really quickly (and then systemd mounts /proc).

But tbh, it was a micro-optimization, so I don't mind it being true, it was just one less mount call.

Copy link
Collaborator

@ericcurtin ericcurtin Apr 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, some things are broken here in general (not in this PR, in initoverlayfs main branch) that I have to fix when I come back.

I think if you run the upstream version of this now on a Fedora Workstation machine, it doesn't boot anymore and they are really trivial silly bugs.

I just got distracted by other things the last week or two.

This commit introduces manual probing of a specified SCSI LUN.
The LUN can be specified either via the kernel command line
`scsi.addr=<host>:<channel>:<target>:<lun>` or by adding a line to the
`initoverlayfs.config` file `scsi.addr <host>:<channel>:<target>:<lun>`.
In case both are present, the configuration takes precedence.
Any operation depends on the presence of the `scsi_mod.scan=manual` kernel
argument.
If it is not present in the kernel command line, no action will occur.
The directory `scsi_probe` contains the main functions needed to complete the
task and some related tests.
To run the tests, simply run `make` in the `scsi_probe` subdirectory.

Signed-off-by: Alessandro Carminati (Red Hat) <[email protected]>
@alessandrocarminati
Copy link
Collaborator Author

I updated my PR because of a typo I discovered. It doesn't change much in practice, but it bothered me. In scsi_probe/tests/parse_kernel_cmdline.c, the Italian version of Donald Duck, paperino, was misspelled.
Now, it's fixed! :-P

@ericcurtin ericcurtin merged commit 07fd62a into containers:main Apr 24, 2024
5 checks passed
@ericcurtin
Copy link
Collaborator

ericcurtin commented Apr 24, 2024

Merging before I go on PTO, I didn't give the most thorough review, but it's all ifdef'd out anyway, better to merge to not block progress...

@dougsland
Copy link
Collaborator

agreed, sorry i couldn't review before. If we find anything later in the road we improve it.

@ericcurtin
Copy link
Collaborator

It will be interesting to try the boot direct to erofs idea you had @alessandrocarminati via "root=" karg and then pivot_rooting to the real rootfs using "systemd.root=" karg @rrendec is working on.

Of course this is only suitable for embedded because of the dependency on the storage drivers being directly into the kernel, but it's one of the cases we care about.

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.

3 participants