Bug 1215410 - Regression in kernel-install 254.3+suse.3.gb6b4e5a8a8 breaks downstream scripts
Summary: Regression in kernel-install 254.3+suse.3.gb6b4e5a8a8 breaks downstream scripts
Status: RESOLVED UPSTREAM
Alias: None
Product: openSUSE Tumbleweed
Classification: openSUSE
Component: Bootloader (show other bugs)
Version: Current
Hardware: x86-64 openSUSE Tumbleweed
: P5 - None : Normal (vote)
Target Milestone: ---
Assignee: dracut maintainers
QA Contact: E-mail List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-09-17 21:05 UTC by Gene Snider
Modified: 2023-09-20 20:28 UTC (History)
2 users (show)

See Also:
Found By: ---
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Gene Snider 2023-09-17 21:05:43 UTC
Quoting from the man page for kernel-install:

COMMANDS
       The following commands are understood:

       add KERNEL-VERSION KERNEL-IMAGE [INITRD-FILE ...]
           This command expects a kernel version string and a path to a kernel image file as
           arguments. Optionally, one or more initrd images may be specified as well (note that
           plugins might generate additional ones).  kernel-install calls the executable files from
           /usr/lib/kernel/install.d/*.install and /etc/kernel/install.d/*.install (i.e. the plugins)
           with the following arguments:

               add KERNEL-VERSION $BOOT/ENTRY-TOKEN/KERNEL-VERSION/ KERNEL-IMAGE [INITRD-FILE ...]

When the script /usr/lib/kernel/install.d/50-dracut.install changed behavior, I created a debug version of it in /etc/kernel/install.d/.  Here are the arguments passed to 50-dracut.install by the old and new versions of kernel-install.

50-dracut.install.old:$1=add
50-dracut.install.old:$2=6.5.3-1-default
50-dracut.install.old:$3=/efi/2ebf7cdd6cdb4335b16f89f43f467a48/6.5.3-1-default
50-dracut.install.old:$4=/boot/vmlinuz-6.5.3-1-default

50-dracut.install:$1=add
50-dracut.install:$2=6.5.3-1-default
50-dracut.install:$3=/efi/2ebf7cdd6cdb4335b16f89f43f467a48/6.5.3-1-default
50-dracut.install:$4=/usr/lib/modules/6.5.3-1-default/vmlinuz

Here is the entry for that kernel image in /boot:

> ls -l /boot/vmlinuz-6.5.3-1-default
lrwxrwxrwx 1 root root 42 Sep 15 22:52 /boot/vmlinuz-6.5.3-1-default -> ../usr/lib/modules/6.5.3-1-default/vmlinuz

As you can see, the new version no longer passes the link name in $4, it passes what the link points to.  This break any downstream script that depends on the documented behavior.  In particular, this script fragment from /usr/lib/kernel/install.d/50-dracut.install no longer copies the pregenerated initrd to the efi partition:

case "$COMMAND" in
    add)
        if [[ $IMAGE == "uki.efi" ]]; then
            IMAGE_PREGENERATED=${KERNEL_IMAGE%/*}/uki.efi
        else
            IMAGE_PREGENERATED=${KERNEL_IMAGE%/*}/initrd
        fi
        if [[ -f ${IMAGE_PREGENERATED} ]]; then
            # we found an initrd or uki.efi at the same place as the kernel
            # use this and don't generate a new one
            [[ $KERNEL_INSTALL_VERBOSE == 1 ]] && echo \
                "There is an ${IMAGE} image at the same place as the kernel, skipping generating a new one"
            cp --reflink=auto "$IMAGE_PREGENERATED" "$BOOT_DIR_ABS/$IMAGE" \
                && chown root:root "$BOOT_DIR_ABS/$IMAGE" \
                && chmod 0600 "$BOOT_DIR_ABS/$IMAGE" \
                && exit 0
        fi

Please restore the old documented behavior of argument passing to the new version of kernel-install.

Thank you,
Gene
Comment 1 Gene Snider 2023-09-18 04:40:40 UTC
I forgot to mention, the working version of kernel-install shipped as part of udev-253.8-1.1.  It was written as a POSIX shell script, the following versions are compiled.

Gene
Comment 2 Antonio Feijoo 2023-09-18 08:46:53 UTC
(In reply to Gene Snider from comment #0)
> Quoting from the man page for kernel-install:
> 
> COMMANDS
>        The following commands are understood:
> 
>        add KERNEL-VERSION KERNEL-IMAGE [INITRD-FILE ...]
>            This command expects a kernel version string and a path to a
> kernel image file as
>            arguments. Optionally, one or more initrd images may be specified
> as well (note that
>            plugins might generate additional ones).  kernel-install calls
> the executable files from
>            /usr/lib/kernel/install.d/*.install and
> /etc/kernel/install.d/*.install (i.e. the plugins)
>            with the following arguments:
> 
>                add KERNEL-VERSION $BOOT/ENTRY-TOKEN/KERNEL-VERSION/
> KERNEL-IMAGE [INITRD-FILE ...]
> 
> ...
> 
> Please restore the old documented behavior of argument passing to the new
> version of kernel-install.

Unfortunately, after reading the documentation, I don't see anywhere that if the arguments are symlinks, they will be preserved and not converted to the real file path. I guess this new behavior is related to the new `--esp-path`/`--boot-path` options. And this is a change introduced upstream with systemd-v254, so we cannot restore the previous behavior, an issue should be raised there.

> As you can see, the new version no longer passes the link name in $4, it
> passes what the link points to.  This break any downstream script that
> depends on the documented behavior.  In particular, this script fragment
> from /usr/lib/kernel/install.d/50-dracut.install no longer copies the
> pregenerated initrd to the efi partition:
> 
> case "$COMMAND" in
>     add)
>         if [[ $IMAGE == "uki.efi" ]]; then
>             IMAGE_PREGENERATED=${KERNEL_IMAGE%/*}/uki.efi
>         else
>             IMAGE_PREGENERATED=${KERNEL_IMAGE%/*}/initrd
>         fi
>         if [[ -f ${IMAGE_PREGENERATED} ]]; then
>             # we found an initrd or uki.efi at the same place as the kernel
>             # use this and don't generate a new one
>             [[ $KERNEL_INSTALL_VERBOSE == 1 ]] && echo \
>                 "There is an ${IMAGE} image at the same place as the kernel,
> skipping generating a new one"
>             cp --reflink=auto "$IMAGE_PREGENERATED" "$BOOT_DIR_ABS/$IMAGE" \
>                 && chown root:root "$BOOT_DIR_ABS/$IMAGE" \
>                 && chmod 0600 "$BOOT_DIR_ABS/$IMAGE" \
>                 && exit 0
>         fi

IIUC this part of the script is only affected if you follow the BLS, and I think the initrd file in the ESP is not a symlink, and it's the only case where it's named `initrd` and not `initrd-<kver>`.
Comment 3 Gene Snider 2023-09-18 19:20:55 UTC
Is it possible for /usr/lib/kernel/install.d/50-dracut.install to discover that the vmlinuz-<kver> and initrd, or initrd-<kver>, are mounted under /boot?  If so, the dracut team could modify /usr/lib/kernel/install.d/50-dracut.install to find the pregenerated initrd file.  If not, do you think upstream would consider restoring the old behavior?

Thanks,
Gene
Comment 4 Gene Snider 2023-09-18 19:26:20 UTC
Sorry, "are mounted under /boot?" should read "are located under /boot?"

Gene
Comment 5 Antonio Feijoo 2023-09-19 07:11:37 UTC
(In reply to Gene Snider from comment #3)
> Is it possible for /usr/lib/kernel/install.d/50-dracut.install to discover
> that the vmlinuz-<kver> and initrd, or initrd-<kver>, are mounted under
> /boot?  If so, the dracut team could modify
> /usr/lib/kernel/install.d/50-dracut.install to find the pregenerated initrd
> file.  If not, do you think upstream would consider restoring the old
> behavior?

I don't think systemd upstream wants the old behavior, because the new one is not wrong, but you can open an issue with your request there to see what they think about it.

Regarding the 50-dracut.install script, the current feature of not building a new initrd if there is already one next to the kernel image is intended to work only with the boot loader specification, and the new behavior of the kernel-install tool is not breaking it. I don't think there is an easy way to guess whether a new initrd should be generated or not when 50-dracut.install is invoked by kernel-install, so a new one it's always built (except in the case mentioned before).

In conclusion, I don't think there is any SUSE specific issue here, if there is any concerns with the implementation of kernel-install/50-dracut.install, it must first be raised upstream systemd/dracut. I hope this makes sense. Thanks!
Comment 6 Gene Snider 2023-09-19 17:31:36 UTC
If what you mean by following the Boot Loader Specification is mounting the efi partition in /efi, rather than /boot/efi, I believe I am doing that.  I read the Boot Loader Specification when I decided to try systemd boot, and here is my partitioning information:

lsblk -f /dev/sdb
NAME   FSTYPE FSVER LABEL UUID                                 FSAVAIL FSUSE% MOUNTPOINTS
sdb                                                                           
├─sdb1 vfat   FAT32 ESP   9541-A20B                             913.7M    11% /efi
└─sdb2 f2fs   1.15  root  3551cc64-079e-44a7-b2b6-e15340d5d652  229.9G     3% /

From your last comment, I think 50-dracut.install script should be able to take advantage of the pregenerated kernel?  If so, how do I open an issue with upstream udev/kernel-install and upstream systemd/dracut?

Thanks,

Gene
Comment 7 Gene Snider 2023-09-20 03:07:06 UTC
I ran kernel-install with the -v option and got this:

Using plugins: 
  /usr/lib/kernel/install.d/50-depmod.install
  /etc/kernel/install.d/50-dracut.install
  /usr/lib/kernel/install.d/51-dracut-rescue.install
  /usr/lib/kernel/install.d/90-loaderentry.install
  /usr/lib/kernel/install.d/90-uki-copy.install
Plugin environment: 
  LC_COLLATE=C.UTF-8
  KERNEL_INSTALL_VERBOSE=1
  KERNEL_INSTALL_IMAGE_TYPE=pe
  KERNEL_INSTALL_MACHINE_ID=2ebf7cdd6cdb4335b16f89f43f467a48
  KERNEL_INSTALL_ENTRY_TOKEN=2ebf7cdd6cdb4335b16f89f43f467a48
  KERNEL_INSTALL_BOOT_ROOT=/efi
  KERNEL_INSTALL_LAYOUT=bls
  KERNEL_INSTALL_INITRD_GENERATOR=
  KERNEL_INSTALL_UKI_GENERATOR=
  KERNEL_INSTALL_STAGING_AREA=/tmp/kernel-install.staging.fX3Mi5

You will notice I have overridden 50-dracut.install with my own version that detects and copies the pregenerated initrd file.  The plugin environment confirms that kernel-install agrees that my installation layout is bls.

I think this contradicts your statement:

"Regarding the 50-dracut.install script, the current feature of not building a new initrd if there is already one next to the kernel image is intended to work only with the boot loader specification, and the new behavior of the kernel-install tool is not breaking it."

To me, it seems that the new kernel-install behavior is breaking the behavior in a system that conforms with the boot loader specification.  Please let me know how to communicate this to upstream.

Thank you,
Gene
Comment 8 Antonio Feijoo 2023-09-20 06:34:52 UTC
Here (In reply to Gene Snider from comment #6)
> If what you mean by following the Boot Loader Specification is mounting the
> efi partition in /efi, rather than /boot/efi, I believe I am doing that.

Not exactly, you need to use a bootloader that follows the BLS. Tumbleweed by default installs GRUB, which is not ready (yet). If you follow a typical Tumbleweed installation using GRUB, with the kernel and initrd placed in /boot, the purpose of the 50-dracut.install script is to build a new initrd every time `kernel-install add` is called.

> I read the Boot Loader Specification when I decided to try systemd boot, and
> here is my partitioning information:

Ok, that's another topic. The integration of systemd-boot in Tumbleweed is still WIP. Useful links:
- https://en.opensuse.org/Systemd-boot
- https://media.ccc.de/v/all-systems-go-2023-189-systemd-boot-integration-in-opensuse

> From your last comment, I think 50-dracut.install script should be able to
> take advantage of the pregenerated kernel?  If so, how do I open an issue
> with upstream udev/kernel-install and upstream systemd/dracut?

- udev/kernel-install belongs to systemd: https://github.com/systemd/systemd/issues
- dracut: https://github.com/dracutdevs/dracut/issues

> I think this contradicts your statement:
> 
> "Regarding the 50-dracut.install script, the current feature of not building
> a new initrd if there is already one next to the kernel image is intended to
> work only with the boot loader specification, and the new behavior of the
> kernel-install tool is not breaking it."

I said "is intended to work only with the BLS", but I didn't say "it works with the specific way you're implementing systemd-boot". The WIP page mentioned above does not use kernel-install, but rather a custom tool called sdbootutil, which (for now) is more suitable for working with Tumbleweed.
Comment 9 Gene Snider 2023-09-20 20:28:10 UTC
My /efi is setup as shown in that video you linked.  Since my root file system is f2fs, I can't use snapshots.  So I set LOADER_TYPE in /etc/sysconfig/bootloader to systemd-boot, which is provided by the perl-Bootloader package.  So when the scripts in /usr/lib/module-init-tools/kernel-scriptlets call /usr/lib/bootloader/bootloader_entry, /usr/bin/pbl is called due to this link:

ll /usr/lib/bootloader/bootloader_entry 
lrwxrwxrwx 1 root root 14 Sep  8 12:45 /usr/lib/bootloader/bootloader_entry -> ../../sbin/pbl

As I mentioned earlier, the package perl-Bootloader provides the scripts in /usr/lib/bootloader/systemd-boot.  These scripts call "kernel-install add ..." and "kernel-install remove ..." to add and remove entries in the efi partition.

All of these tools are provided by different upstream packages, and I assume they are meant to function on any distribution.  Of course they don't support snapshots, but many distributions do not use snapshots.  It seems like this use case shouldn't be broken by a new version of kernel-install, as you stated in Comment #5.  I'll go ahead and post an issue with kernel-install git-hub link you provided and see what they think.

Thanks for the links,

Gene