-
Notifications
You must be signed in to change notification settings - Fork 343
boot/dracut: fix installation when sysroot dir is not / #3557
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
base: main
Are you sure you want to change the base?
Conversation
Dracut can build initramfs for other system than the currently running by
setting $dracutsysrootdir environment variable. But, since dracut still runs
in host system context, all modules must be preceed paths with
${dracutsysrootdir-} or they won't work properly - may fail during, or when
host system has ostree installed, silently take files from the host instead of
sysroot.
This patch fixes module-setup.sh so this scenario behaves properly, tested on
yocto.
Signed-off-by: Artur Kowalski <[email protected]>
|
Hi @arturkow2000. Thanks for your PR. I'm waiting for a ostreedev member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request aims to fix the dracut module for installations where the sysroot directory is not / by prefixing paths with $dracutsysrootdir. The change in the check function is correct, though I've suggested quoting a variable for robustness. However, the changes in the install function appear to be incorrect for a standard dracut environment, as dracut's installation functions (dracut_install, inst_simple) handle the $dracutsysrootdir prefix automatically. Manually adding the prefix could lead to errors. I've provided a detailed comment and a code suggestion to correct this, assuming standard dracut behavior.
| dracut_install "${dracutsysrootdir-}/usr/lib/ostree/ostree-prepare-root" | ||
| for r in "${dracutsysrootdir-}/usr/lib" "${dracutsysrootdir-}/etc"; do | ||
| if test -f "$r/ostree/prepare-root.conf"; then | ||
| inst_simple "$r/ostree/prepare-root.conf" | ||
| fi | ||
| done | ||
| if test -f "/etc/ostree/initramfs-root-binding.key"; then | ||
| inst_simple "/etc/ostree/initramfs-root-binding.key" | ||
| if test -f "${dracutsysrootdir-}/etc/ostree/initramfs-root-binding.key"; then | ||
| inst_simple "${dracutsysrootdir-}/etc/ostree/initramfs-root-binding.key" | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dracut_install and inst_simple functions in standard dracut automatically prepend the $dracutsysrootdir path when locating files. Your change manually prepends this path, which will likely cause an error due to double-prefixing (e.g., /sysroot/sysroot/...).
While the test -f checks do need the prefix, the dracut installation functions should be called with paths relative to the sysroot. If you are using a patched version of dracut in Yocto that changes this behavior, please clarify. Otherwise, the logic should be adjusted.
Here's a suggested correction that should work with standard dracut:
| dracut_install "${dracutsysrootdir-}/usr/lib/ostree/ostree-prepare-root" | |
| for r in "${dracutsysrootdir-}/usr/lib" "${dracutsysrootdir-}/etc"; do | |
| if test -f "$r/ostree/prepare-root.conf"; then | |
| inst_simple "$r/ostree/prepare-root.conf" | |
| fi | |
| done | |
| if test -f "/etc/ostree/initramfs-root-binding.key"; then | |
| inst_simple "/etc/ostree/initramfs-root-binding.key" | |
| if test -f "${dracutsysrootdir-}/etc/ostree/initramfs-root-binding.key"; then | |
| inst_simple "${dracutsysrootdir-}/etc/ostree/initramfs-root-binding.key" | |
| fi | |
| dracut_install /usr/lib/ostree/ostree-prepare-root | |
| for r in /usr/lib /etc; do | |
| if test -f "${dracutsysrootdir-}$r/ostree/prepare-root.conf"; then | |
| inst_simple "$r/ostree/prepare-root.conf" | |
| fi | |
| done | |
| if test -f "${dracutsysrootdir-}/etc/ostree/initramfs-root-binding.key"; then | |
| inst_simple "/etc/ostree/initramfs-root-binding.key" | |
| fi |
|
|
||
| check() { | ||
| if [[ -x $systemdutildir/systemd ]] && [[ -x /usr/lib/ostree/ostree-prepare-root ]]; then | ||
| if [[ -x $systemdutildir/systemd ]] && [[ -x "${dracutsysrootdir-}/usr/lib/ostree/ostree-prepare-root" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable $systemdutildir should be quoted to prevent issues if the path contains spaces or other special characters. This is a general shell scripting best practice for robustness.
| if [[ -x $systemdutildir/systemd ]] && [[ -x "${dracutsysrootdir-}/usr/lib/ostree/ostree-prepare-root" ]]; then | |
| if [[ -x "$systemdutildir/systemd" ]] && [[ -x "${dracutsysrootdir-}/usr/lib/ostree/ostree-prepare-root" ]]; then |
cgwalters
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Dracut can build initramfs for other system than the currently running by setting
$dracutsysrootdirenvironment variable. But, since dracut still runs in host system context, all modules must be preceed paths with${dracutsysrootdir-}or they won't work properly - may fail during, or when host system has ostree installed, silently take files from the host instead of sysroot.This patch fixes module-setup.sh so this scenario behaves properly, tested on yocto.