On Tue, Jan 23, 2024 at 3:23 AM Jose Ignacio Tornos Martinez <jtornosm@xxxxxxxxxx> wrote: > > The new installkernel application that is now included in systemd-udev > package allows installation although destination files are already present > in the boot directory of the kernel package, but is failing with the > implemented workaround for the old installkernel application from grubby > package. > > For the new installkernel application, as Davide says: > <<The %post currently does a shuffling dance before calling installkernel. > This isn't actually necessary afaict, and the current implementation > ends up triggering downstream issues such as > https://github.com/systemd/systemd/issues/29568 > This commit simplifies the logic to remove the shuffling. For reference, > the original logic was added in commit 3c9c7a14b627("rpm-pkg: add %post > section to create initramfs and grub hooks").>> > > But we need to keep the old behavior as well, because the old installkernel > application from grubby package, does not allow this simplification and > we need to be backward compatible to avoid issues with the different > packages. > > Mimic Fedora shipping process and store vmlinuz, config amd System.map > in the module directory instead of the boot directory. In this way, we will > avoid the commented problem for all the cases, because the new destination > files are not going to exist in the boot directory of the kernel package. > > Replace installkernel tool with kernel-install tool, because the latter is > more complete. > > Besides, after installkernel tool execution, check to complete if the > correct package files vmlinuz, System.map and config files are present > in /boot directory, and if necessary, copy manually for install operation. > In this way, take into account if files were not previously copied from > /usr/lib/kernel/install.d/* scripts and if the suitable files for the > requested package are present (it could be others if the rpm files were > replace with a new pacakge with the same release and a different build). > > Tested with Fedora 38, Fedora 39, RHEL 9, Oracle Linux 9.3, > openSUSE Tumbleweed and openMandrive ROME, using dnf/zypper and rpm tools. > > cc: stable@xxxxxxxxxxxxxxx > Co-Developed-by: Davide Cavalca <dcavalca@xxxxxxxx> > Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@xxxxxxxxxx> > --- > V1 -> V2: > - Complete to be backward compatible with the previous installkernel > application. > V2 -> V3: > - Follow the suggestions from Masahiro Yamada and change the installation > destination to avoid problems instead of checking the package. > V3 -> V4: > - Make the patch applicable to linux-kbuild/for-next (ia64 support was > already removed). > V4 -> V5: > - Complete for other Linux distributions. > V5 -> V6 > - Simplify and do more compatible checks when copied files wants to be > replaced. > - Remove %preun because it will be better done with another patch. > - Add indentation and quotation > > scripts/package/kernel.spec | 28 ++++++++++++++-------------- > 1 file changed, 14 insertions(+), 14 deletions(-) > > diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec > index 89298983a169..0bff257ec3d4 100644 > --- a/scripts/package/kernel.spec > +++ b/scripts/package/kernel.spec > @@ -55,12 +55,12 @@ patch -p1 < %{SOURCE2} > %{make} %{makeflags} KERNELRELEASE=%{KERNELRELEASE} KBUILD_BUILD_VERSION=%{release} > > %install > -mkdir -p %{buildroot}/boot > -cp $(%{make} %{makeflags} -s image_name) %{buildroot}/boot/vmlinuz-%{KERNELRELEASE} > +mkdir -p %{buildroot}/lib/modules/%{KERNELRELEASE} > +cp $(%{make} %{makeflags} -s image_name) %{buildroot}/lib/modules/%{KERNELRELEASE}/vmlinuz > %{make} %{makeflags} INSTALL_MOD_PATH=%{buildroot} modules_install > %{make} %{makeflags} INSTALL_HDR_PATH=%{buildroot}/usr headers_install > -cp System.map %{buildroot}/boot/System.map-%{KERNELRELEASE} > -cp .config %{buildroot}/boot/config-%{KERNELRELEASE} > +cp System.map %{buildroot}/lib/modules/%{KERNELRELEASE} > +cp .config %{buildroot}/lib/modules/%{KERNELRELEASE}/config > ln -fns /usr/src/kernels/%{KERNELRELEASE} %{buildroot}/lib/modules/%{KERNELRELEASE}/build > %if %{with_devel} > %{make} %{makeflags} run-command KBUILD_RUN_COMMAND='${srctree}/scripts/package/install-extmod-build %{buildroot}/usr/src/kernels/%{KERNELRELEASE}' > @@ -70,31 +70,31 @@ ln -fns /usr/src/kernels/%{KERNELRELEASE} %{buildroot}/lib/modules/%{KERNELRELEA > rm -rf %{buildroot} > > %post > -if [ -x /sbin/installkernel -a -r /boot/vmlinuz-%{KERNELRELEASE} -a -r /boot/System.map-%{KERNELRELEASE} ]; then > -cp /boot/vmlinuz-%{KERNELRELEASE} /boot/.vmlinuz-%{KERNELRELEASE}-rpm > -cp /boot/System.map-%{KERNELRELEASE} /boot/.System.map-%{KERNELRELEASE}-rpm > -rm -f /boot/vmlinuz-%{KERNELRELEASE} /boot/System.map-%{KERNELRELEASE} > -/sbin/installkernel %{KERNELRELEASE} /boot/.vmlinuz-%{KERNELRELEASE}-rpm /boot/.System.map-%{KERNELRELEASE}-rpm > -rm -f /boot/.vmlinuz-%{KERNELRELEASE}-rpm /boot/.System.map-%{KERNELRELEASE}-rpm > +if [ -x /usr/bin/kernel-install ]; then > + /usr/bin/kernel-install add %{KERNELRELEASE} /lib/modules/%{KERNELRELEASE}/vmlinuz > fi > +for file in vmlinuz System.map config; do > + if [ ! -e "/boot/${file}-%{KERNELRELEASE}" ] || ! cmp --silent "/lib/modules/%{KERNELRELEASE}/${file}" "/boot/${file}-%{KERNELRELEASE}"; then > + cp "/lib/modules/%{KERNELRELEASE}/${file}" "/boot/${file}-%{KERNELRELEASE}" > + fi > +done I am fine with this approach, but [ ! -e "/boot/${file}-%{KERNELRELEASE}" ] is a redundant check now. When the file does not exist, "cmp --silent" exits with 2 instead of 1. Anyway, "cmp --silent" fails. You can simplify the conditional to: if ! cmp --silent "/lib/modules/%{KERNELRELEASE}/${file}" "/boot/${file}-%{KERNELRELEASE}"; then > %preun > if [ -x /sbin/new-kernel-pkg ]; then > -new-kernel-pkg --remove %{KERNELRELEASE} --rminitrd --initrdfile=/boot/initramfs-%{KERNELRELEASE}.img > + new-kernel-pkg --remove %{KERNELRELEASE} --rminitrd --initrdfile=/boot/initramfs-%{KERNELRELEASE}.img This is a noise change. Please drop. > elif [ -x /usr/bin/kernel-install ]; then > -kernel-install remove %{KERNELRELEASE} > + /usr/bin/kernel-install remove %{KERNELRELEASE} > fi > > %postun > if [ -x /sbin/update-bootloader ]; then > -/sbin/update-bootloader --remove %{KERNELRELEASE} > + /sbin/update-bootloader --remove %{KERNELRELEASE} Ditto. Please create a separate patch if you change the coding style. But, rather, I am thinking of reverting 6ef41e22a320 and 27c3bffd230ab > fi > > %files > %defattr (-, root, root) > /lib/modules/%{KERNELRELEASE} > %exclude /lib/modules/%{KERNELRELEASE}/build > -/boot/* > > %files headers > %defattr (-, root, root) > -- > 2.43.0 > -- Best Regards Masahiro Yamada
