Message-ID: <CAK7LNAQ7C+ZN+mxaiw3-10Lmn2f0nAYi+aou=CWraZwV=_iThw@mail.gmail.com>
Date: Wed, 17 Jan 2024 10:29:25 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Jose Ignacio Tornos Martinez <jtornosm@...hat.com>
Cc: dcavalca@...a.com, linux-kbuild@...r.kernel.org, 
	linux-kernel@...r.kernel.org, nathan@...nel.org, ndesaulniers@...gle.com, 
	nicolas@...sle.eu, stable@...r.kernel.org
Subject: Re: [PATCH V5 1/2] rpm-pkg: simplify installkernel %post

On Sun, Jan 14, 2024 at 5:07 PM Jose Ignacio Tornos Martinez
<jtornosm@...hat.com> 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 suitable
> (same release and build) vmlinuz, System.map and config files are present
> in /boot directory, and if necessary, copy manually for install operation
> or remmove manually for remove operation.
>
> 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@...r.kernel.org
> Co-Developed-by: Davide Cavalca <dcavalca@...a.com>
> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@...hat.com>
> ---
> 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.
>
>  scripts/package/kernel.spec | 39 +++++++++++++++++++++++++------------
>  1 file changed, 27 insertions(+), 12 deletions(-)
>
> diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
> index 89298983a169..74542af8cbfe 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,19 +70,35 @@ 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
> +if [ -e /boot/vmlinuz-%{KERNELRELEASE} ] && file -bL /boot/vmlinuz-%{KERNELRELEASE} | grep -q " #%{release} "; then
> +release_match=0
> +else
> +release_match=1
> +fi
> +for file in vmlinuz System.map config; do
> +if [ ! -e /boot/${file}-%{KERNELRELEASE} ] || [ ${release_match} != 0 ]; then
> +cp -v /lib/modules/%{KERNELRELEASE}/${file} /boot/${file}-%{KERNELRELEASE}
> +fi
> +done
>
>  %preun
>  if [ -x /sbin/new-kernel-pkg ]; then
>  new-kernel-pkg --remove %{KERNELRELEASE} --rminitrd --initrdfile=/boot/initramfs-%{KERNELRELEASE}.img
>  elif [ -x /usr/bin/kernel-install ]; then
> -kernel-install remove %{KERNELRELEASE}
> +/usr/bin/kernel-install remove %{KERNELRELEASE}
> +if [ -e /boot/vmlinuz-%{KERNELRELEASE} ] && file -bL /boot/vmlinuz-%{KERNELRELEASE} | grep -q " #%{release} "; then


I do not understand why this is needed.

Please explain.


And, is the output of 'file' standardized?


You need to understand that ARCH is not always x86,
and /boot/vmlinuz-%{KERNELRELEASE}
is not always arch/x86/boot/bzImage.



See arch/arm64/Makefile

KBUILD_IMAGE    := $(boot)/Image.gz


For arm64, /boot/vmlinuz-%{KERNELRELEASE} is Image.gz

'file' says it is gzip data, that's all.
You cannot read the build version.







> +release_match=0
> +else
> +release_match=1
> +fi
> +for file in vmlinuz System.map config; do
> +if [ -e /boot/${file}-%{KERNELRELEASE} ] && [ ${release_match} == 0 ]; then
> +rm -v /boot/${file}-%{KERNELRELEASE}
> +fi
> +done


Unreadable.

I suggested code with indentation and quotation,
but you got rid of them.






>  fi
>
>  %postun
> @@ -94,7 +110,6 @@ fi
>  %defattr (-, root, root)
>  /lib/modules/%{KERNELRELEASE}
>  %exclude /lib/modules/%{KERNELRELEASE}/build
> -/boot/*
>
>  %files headers
>  %defattr (-, root, root)
> --
> 2.43.0
>


-- 
Best Regards
Masahiro Yamada

Powered by blists - more mailing lists