Make better use of dracut functions when building initramfs

[savyajha]

Setting up the module involves multiple redundant calls to a bunch of
dracut functions wheich can be combined into one. Additionally, the mass
of code required to load libgcc_s.so* can be replaced with one dracut
function. This has the additional effect of removing the error:

dracut-install: ERROR: installing '/usr/lib/gcc/*/*/libgcc_s.so*'
dracut: FAILED: /usr/lib/dracut/dracut-install -D /var/tmp/dracut.ELz0Yb/initramfs -a /usr/lib/gcc/*/*/libgcc_s.so*

Which is seen on debian bullseye when using version 2.1.2-1~bpo11+1 from
the backports repository.

Signed-off-by: Savyasachee Jha [email protected]

Motivation and Context

I made this change because I kept getting the error (mostly cosmetic, but annoying):

dracut-install: ERROR: installing '/usr/lib/gcc/*/*/libgcc_s.so*'
dracut: FAILED: /usr/lib/dracut/dracut-install -D /var/tmp/dracut.ELz0Yb/initramfs -a /usr/lib/gcc/*/*/libgcc_s.so*

whenever I could build a new initramfs on debian bullseye using dracut. This caused no issues in booting but seeing that error message all the time was jarring.

Description

The module-setup.sh.in file defines how zfs modules are setup in the initramfs and pulls in all the required dependencies. The script (in master) calls the instmods, inst_rules and dracut_install functions once for each module, rule and binary which are to be added to the initramfs respectively. Since each function is capable of installing multiple files, I have reduced the number of function calls to just one each.

In addition, there is a long if-else statement to find libgcc_s.so* on the filesystem and install it into the initramfs. Dracut has a function inst_libdir_file which does this quite well, so I replaced the if-else statements with one call to inst_libdir_file.

How Has This Been Tested?

I have tested this on Debian stable, Fedora 35, and Arch Linux using zfs 2.1.2 by building the initramfs using this change and booting into the system. No changes were observed in terms of booting - the systems booted perfectly well with no errors. The upside was that the cosmetic error seen in Debian described above went away.

This change should not affect any other areas of code.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

Since this does not touch the core zfs files, I did not run the test suite or add any tests. I also do not believe this change requires any addition to the documentation.

[savyajha]

I realised I hadn't separated the systemd binaries out and they would be pulled in regardless of whether the systemd module was being used by dracut or not. I've separated them out.

[savyajha]

Changed commit message because one line was over 72 characters resulting in it failing the style check.

[zdykstra]

Is it within the scope of this PR to also start checking the return code of the Dracut helper functions, in certain situations? Installation failures of 'required' kernel modules, binaries, etc are simply ignored. If everything works as expected, Dracut will produce a working initramfs. If something does go wrong - say, zfs.ko can't be found - this module happily carries on and allows the generation of the initramfs. In situations of root-on-ZFS systems, this will have disastrous results during the next reboot.

Ideally, dfatal and then exit 1 should be called if anything can't be installed, blocking the creation of a faulty initramfs.

[savyajha]

If the creation of an initramfs is blocked for whatever reason after, say, a kernel upgrade, and the user happens to ignore that warning, then you end up having to boot with a new kernel without a functional initramfs anyway. Imo, in practically all situations, the initramfs is built after a kernel upgrade or a zfs upgrade. There is no practical difference between the initramfs not getting created and a faulty initramfs being created. Both of them will log errors onto the command line during creation time, neither of them will produce a functional initramfs.

In the binaries/modules we install, the only command which does not log errors is the kernel module installer, and a simple -c flag can be added to it to make sure it fails is the zfs kernel module is not there. For the rest of the functions we call, dracut will report errors onto the console and then continue building the initramfs.

[ahesford]

This is not true. Failing to produce an initramfs tends to trigger a bunch of error output. Many times, this is probably even noticed by the package manager, which will complain in addition to dracut.

The dracut module makes a covenant with the user to provide functional ZFS support in the initramfs. If it can't honor that, the right thing to do is fail early and loudly, not say "LOL, fuck you" and swallow the failures.

People have been bitten by this in the wild. User reports are the reason that we are aggressive about these failures in ZFSBootMenu. Maybe not everything is strictly necessary, but much of what is currently installed unchecked is a hard requirement and should properly trigger an initramfs failure when it can't be installed.

[ahesford]

Note also that it isn't uncommon for post-installation hooks that would automate the initramfs generation tend to use --quiet because dracut is incredibly noisy without it. A lot of warnings and otherwise ignorable errors tend to flood the screen and would drown out any helpful ZFS failures if we rely on normal error output. You really want dracut to hard abort when necessary things are missing.

(Dracut upstream also does a horrible job at this; even 99base happily ignores failures and will gladly create an initramfs missing, e.g., /init!)

[savyajha]

@ahesford I've made most of the changes you suggest (I've kept absolute paths until I get confirmation to remove them). I've added exit 1 throughout so hopefully failure in installing a necessary binary/rule will lead to initramfs generation failing loudly.

[savyajha]

Force pushed a change which resolved a non-fatal error I had missed in previous testing.

[ahesford]

I think the latest changes definitely reduce the likelihood that users will be caught by surprise with a faulty initramfs image. Thanks!

[savyajha]

Force pushed a change to resolve issue detected by a test on gentoo.

[savyajha]

@nabijaczleweli @ahesford Apologies for bothering the two of you, but what other changes would you recommend here? Should the absolute paths be kept or not?

[nabijaczleweli]

I think the @config@/file paths need to remain, for compatibility. The whole point of having a configurable for them is that if you install it outside of the current/default PATH &a., or for a slightly different system, or to override the upstream from your system, everything will still work, and you can configure PATH in the initrd through a few mechanisms that are outside the scope of this dracut integration.

[ahesford]

The hard-coded paths never should have existed, period. Dracut knows how to find exectuables, and its install program even respects a DRACUT_INSTALL_PATH environment variable to override the default PATH search for non-standard paths. Hard-coding the paths in the module setup at installatiton time completely breaks the --sysroot mechanism for creating an initramfs image from an alternative root filesystem.

Yes, there will be some user out there who will be adversely affected by dropping the paths. However:

1. Now that the module does the right thing and fails hard when components are missing, users should immediately be alerted to the consequences of this change for their non-standard system, instead of spitting out incomplete initramfs images with breakage that won't be discovered until reboot; and
2. Because this PR already attempts to do the right thing wrt ZFS component installation, let's go all the way and fix a long-standing wrong.

[nabijaczleweli]

I was not aware of DRACUT_INSTALL_PATH, and the --sysroot problem is damning enough in its own right – you're right, the absolute paths need to go.

[savyajha]

Since we're using dracut_install to install the udev rules, there isn't any dependency resolution. Thus @udevdir@/vdev_id and @udevdir@/zvol_id need to be kept as is if we wish to abort initramfs creation if a udev rule cannot be found. dracut_install cannot find the udev rules if I remove the absolute paths (I have tested them on a debian system and verified that absolute paths are needed there) and inst_multiple does not check in the udev dir at all.

The best way of doing this would be to use inst_rules to install the udev rules, but that function always returns 0 and cannot be used to check for failure. Should I remove the checks on installing udev rules? Or should I keep the absolute paths for them? Just for reference, the function in question is defined thusly:

# udev rules always get installed in the same place, so
# create a function to install them to make life simpler.
inst_rules() {
    local _target=/etc/udev/rules.d _rule _found

    inst_dir "${udevdir}/rules.d"
    inst_dir "$_target"
    for _rule in "$@"; do
        if [ "${_rule#/}" = "$_rule" ]; then
            for r in "$dracutsysrootdir${udevdir}/rules.d" ${hostonly:+"$dracutsysrootdir"/etc/udev/rules.d}; do
                [[ -e $r/$_rule ]] || continue
                _found="$r/$_rule"
                inst_rule_programs "$_found"
                inst_rule_group_owner "$_found"
                inst_rule_initqueue "$_found"
                inst_simple "$_found"
            done
        fi
        for r in '' "$dracutsysrootdir$dracutbasedir/rules.d/"; do
            # skip rules without an absolute path
            [[ "${r}$_rule" != /* ]] && continue
            [[ -f ${r}$_rule ]] || continue
            _found="${r}$_rule"
            inst_rule_programs "$_found"
            inst_rule_group_owner "$_found"
            inst_rule_initqueue "$_found"
            inst_simple "$_found" "$_target/${_found##*/}"
        done
        [[ $_found ]] || dinfo "Skipping udev rule: $_rule"
    done
}

[ahesford]

Unfortunately, both alternatives for rule installation seem kind of lousy at this point. The function inst_rules is itself a mess badly in need of a rewrite. I guess I favor dropping the hard-coded path and relying on inst_rules despite it swallowing the error code, for the following reasons:

• inst_rules should really be improved upstream, simplifying the logic and not swallowing installation failures. If that work is done, the ZFS module inherits those fixes automatically.
• Relying on the dependency scan allows us to drop the explicit installation of the zvol_id and vdev_id helpers.
• I believe, based on the udev manual page and the comment above inst_rules, that udev is pretty rigid about where it will look for rules. Thus, any user who attempts to install the ZFS udev rules somewhere other than where inst_rules will find them will most likely not have functional ZFS rules on the running system; ipso facto, the user shouldn't care whether the rules make it into the initramfs.

I guess if we really cared about confirming that the udev rules made it in but still wanted to let inst_rules do its thing, one could verify the existence of one of the following two files after the installation call:

${initdir}/etc/udev/rules.d/90-zfs.rules
${initdir}/${udevdir}/rules.d/90-zfs.rules

However, this would be an ugly hack that is probably no better than just hard-coding the rule paths and manually satisfying the helper dependencies.

Either drop the paths and use inst_rules or, if others (esp. @nabijaczleweli) provide a good defense of keeping the rule paths, leave them in.

[savyajha]

Removed absolute paths as far as possible (I've left in a couple of @sysconfdir@s) and cleaned up the logic in some other parts of the file.

[savyajha]

@nabijaczleweli @ahesford It is done from my side. I don't think there's anything else left to change in this. Do have a look at the new changes and see if they work for you. I've tested the patch on an Arch, Debian and Fedora machine. Seems to work well.

[ahesford]

Looks good to me. This is a nice improvement over the current state.

[savyajha]

Made the requested changes.

[prometheanfire]

That question was asked tongue in cheek, I'm going to test with the file inclusion removed and submit a patch if it works.