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

fstab-generator: Add systemd.root= karg #32702

Closed
wants to merge 1 commit into from
Closed

Conversation

rrendec
Copy link

@rrendec rrendec commented May 8, 2024

The new "systemd.root" parameter has similar semantics to the existing "root" parameter, and controls the device that is mounted as sysroot during boot. More specifically, this is the device that is used in the sysroot.mount unit, generated by systemd-fstab-generator.

The existing parameter is not removed, and it can be used interchange- ably with the new parameter. If both "root" and "systemd.root" are present on the kernel command line, "systemd.root" takes precedence.

The goal is to isolate the root device specification into the systemd namespace. This can be useful when the kernel starts with an initramfs and systemd needs to use a different device than what is specified in the "root" parameter, for example when the boot firmware injects an incorrect or incomplete root device into the kernel command line.

The new "systemd.root" parameter has similar semantics to the existing
"root" parameter, and controls the device that is mounted as sysroot
during boot. More specifically, this is the device that is used in the
sysroot.mount unit, generated by systemd-fstab-generator.

The existing parameter is not removed, and it can be used interchange-
ably with the new parameter. If both "root" and "systemd.root" are
present on the kernel command line, "systemd.root" takes precedence.

The goal is to isolate the root device specification into the systemd
namespace. This can be useful when the kernel starts with an initramfs
and systemd needs to use a different device than what is specified in
the "root" parameter, for example when the boot firmware injects an
incorrect or incomplete root device into the kernel command line.
@github-actions github-actions bot added fstab-generator please-review PR is ready for (re-)review by a maintainer labels May 8, 2024
Copy link

github-actions bot commented May 8, 2024

Important

An -rc1 tag has been created and a release is being prepared, so please note that PRs introducing new features and APIs will be held back until the new version has been released.

Copy link
Member

@YHNdnzj YHNdnzj left a comment

Choose a reason for hiding this comment

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

Why not just use rd.systemd.mount-extra=? If that doesn't work, please fix that rather than introducing new options needlessly.

@YHNdnzj
Copy link
Member

YHNdnzj commented May 8, 2024

So we even have a test case for mounting /sysroot/ using rd.systemd.mount-extra=. Therefore this new cmdline option should be completely unnecessary.

@YHNdnzj YHNdnzj added needs-discussion 🤔 and removed please-review PR is ready for (re-)review by a maintainer labels May 8, 2024
@ericcurtin
Copy link
Contributor

ericcurtin commented May 8, 2024

One of the main use cases for this is an embedded use case where you want to use both "root=" karg and "systemd.root=" karg.

In embedded it's often faster to boot directly to rootfs, but we lose a lot of functionality that exists in an initramfs when we do this. It's useful for initialization tasks to have an initial filesystem of some sort and for better or worse a lot of tools are designed with an initial filesystem implied.

So in an embedded case, if storage drivers are built in for optimisation purposes, we can tell the kernel to boot to an erofs via root= karg, which behaves like an initramfs once a writable overlay is applied on top. This provides a scalable solution for an initial filesystem so the size of the filesystem does not deteriorate performance, only the amount of bytes you actually use are read on boot. The systemd.root= karg then tells systemd that this is the final root filesystem to switch to and not the initial filesystem.

rd.systemd.mount-extra= unfortunately doesn't work here, it almost works, but when you have both "root=" karg and "systemd.root=" karg it doesn't, @rrendec tested.

We briefly discussed this "systemd.root=" karg with @DaanDeMeyer in the systemd matrix channel.

Tagging others that may be interested for visibility @alessandrocarminati
@landgraf @alexlarsson @rhatdan @smooge @iamianmullins @masneyb @eric-ch

@alessandrocarminati
Copy link

I'd like to explain why, in my opinion, this could be helpful:

The use case arises when the system requires an initramfs-like boot, necessitating an initial file system before switching to a different target file system.
If, for any reason, it's preferable for this initial file system not to be an initramfs but an actual filesystem on a storage device, you may need to inform the kernel about this file system using root=, but then inform systemd you'll need to transition to the target file system.
This is where this PR comes into play.

@ericcurtin
Copy link
Contributor

ericcurtin commented May 9, 2024

A side issue with the rd.systemd.mount-extra= approach is that it's not clear what that does to a reader, it's clearer what the intent of systemd.root= is

@YHNdnzj
Copy link
Member

YHNdnzj commented May 9, 2024

The use case arises when the system requires an initramfs-like boot, necessitating an initial file system before switching to a different target file system.
If, for any reason, it's preferable for this initial file system not to be an initramfs but an actual filesystem on a storage device, you may need to inform the kernel about this file system using root=, but then inform systemd you'll need to transition to the target file system.

Isn't this just describing a initrd (as opposed to initramfs)? Why not use a proper initrd then?

@ericcurtin
Copy link
Contributor

The use case arises when the system requires an initramfs-like boot, necessitating an initial file system before switching to a different target file system.
If, for any reason, it's preferable for this initial file system not to be an initramfs but an actual filesystem on a storage device, you may need to inform the kernel about this file system using root=, but then inform systemd you'll need to transition to the target file system.

Isn't this just describing a initrd (as opposed to initramfs)? Why not use a proper initrd then?

initrd is difficult you need bootloader support etc. so doesn't work in a lot of cases, doing root= works everywhere because it's just a plain old partition.

Copy link
Member

@keszybz keszybz left a comment

Choose a reason for hiding this comment

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

The proposed semantics don't seem well defined. OTOH, the new param is described as almost the same as root=, but the intended scenario is quite different: it is initially ignored, because the kernel doesn't understand it, and then it's used later after the machine has booted in the original root specified with root=. I can see how that could work in your specific case, but it would also fail badly in general case where the initrd is provided and runs systemd internally. So despite the "easy meaning", this new param is ad-hoc and ambiguous.

Dunno, I think it'd be much more reasonable to handle this explicitly, and have a reboot_into_this_root_next= option and handle that using a custom service in the first root. (Name TBD.)

A general comment: any param addition must be accompanied by an addition to docs.

if (proc_cmdline_value_missing(key, value))
if (arg_root_what || proc_cmdline_value_missing(key, value))
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, but that breaks that semantics of root=. The general rule is that (at least for kernel params which can only have one value), the value specified last wins.

Copy link
Author

Choose a reason for hiding this comment

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

Fair enough. I will rewrite it in such a way that the new parameter takes precedence over root= but in case the new parameter is not used at all, the last value of root= is used if it's included multiple times in the command line. But let's agree on what that new parameter looks like first.

@rrendec
Copy link
Author

rrendec commented May 14, 2024

The proposed semantics don't seem well defined. OTOH, the new param is described as almost the same as root=, but the intended scenario is quite different: it is initially ignored, because the kernel doesn't understand it, and then it's used later after the machine has booted in the original root specified with root=. I can see how that could work in your specific case, but it would also fail badly in general case where the initrd is provided and runs systemd internally. So despite the "easy meaning", this new param is ad-hoc and ambiguous.

Can you please explain how/why you think it would fail if an initrd was provided?

Dunno, I think it'd be much more reasonable to handle this explicitly, and have a reboot_into_this_root_next= option and handle that using a custom service in the first root. (Name TBD.)

Why does this have to be different (or implemented differently) from the root= semantics? The use case is similar - you have an initial filesystem that uses systemd, and you need a way to specify what gets mounted in /sysroot and becomes the next (final) rootfs. It just so happens that in this particular case, the initial filesystem is on a real device (not an inird), so you need to use root= to tell the kernel where the initial filesystem is. But for all intents and purposes, that initial filesystem works like an initrd that uses systemd.

A general comment: any param addition must be accompanied by an addition to docs.

Thanks for pointing that out. To be honest, I already knew, but I had a feeling there would be a few iterations before a change like this is accepted. So, I wanted to start the discussion and reach an agreement on what the code looks like. The final PR submission will have a docs addition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

5 participants