Received-SPF: pass (google.com: domain of linux-kernel+bounces-4244-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20231108000749.GA3723879@dev-arch.thelio-3990X> <20231212171044.1108464-1-jtornosm@redhat.com>
In-Reply-To: <20231212171044.1108464-1-jtornosm@redhat.com>
From: Masahiro Yamada <masahiroy@kernel.org>
Date: Tue, 19 Dec 2023 03:26:52 +0900
Message-ID: <CAK7LNATu-4TSSWpyFyVQYrkS++fUQbfp2tVjEpf3oZBV8ihq8w@mail.gmail.com>
Subject: Re: [PATCH v2] rpm-pkg: simplify installkernel %post
To: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
Cc: nathan@kernel.org, dcavalca@meta.com, linux-kbuild@vger.kernel.org, 
	linux-kernel@vger.kernel.org, ndesaulniers@google.com, nicolas@fjasle.eu, 
	stable@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Dec 13, 2023 at 2:10=E2=80=AFAM Jose Ignacio Tornos Martinez
<jtornosm@redhat.com> wrote:
>
> A new installkernel application is now included in systemd-udev package
> and it has been improved to allow simplifications.
>
> 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 installkern=
el
> application from grubby package, does not allow this simplification and
> we need to be backward compatible to avoid issues with the different
> packages. So the easiest solution is to check the package that provides
> the installkernel application, and simplify (and fix for this
> application at the same time), only if the package is systemd-udev.
>
> cc: stable@vger.kernel.org
> Co-Developed-by: Davide Cavalca <dcavalca@meta.com>
> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
> ---
> V1 -> V2:
> - Complete to be backward compatible with the previous installkernel
> application.
>


I do not think this is the right fix.

The root cause is, vmlinuz and System.map already exist
in the destination before running installkernel.

Fedora ships vmlinux, config, System.map in the module directory.
Why don't you mimic it?


Change the %install section to install them to
/lib/modules/%{KERNELRELEASE}/.


Then, change %post section to copy them to /boot/.


If you take care of an unusual case where installkernel
is not found, you can support manual copy as a fallback.

%post
if [ -x /sbin/installkernel ]; then
    /sbin/installkernel %{KERNELRELEASE} \
       /lib/modules/%{KERNELRELEASE}/vmlinuz \
       /lib/modules/%{KERNELRELEASE}/System.map
else
    cp /lib/modules/%{KERNELRELEASE}/vmlinuz /boot/vmlinuz-%{KERNELRELEAE}
    cp /lib/modules/%{KERNELRELEASE}/System.map /boot/System.map-%{KERNELRE=
LEAE}
    cp /lib/modules/%{KERNELRELEASE}/config /boot/config-%{KERNELRELEAE}
fi


The ugly shuffling will go away, and this should work for
both fedora 38 and 39.

Maybe, you can also convert the installkernel syntax to
kernel-install while you are here.


>  scripts/package/kernel.spec | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
> index 3eee0143e0c5..d4276ddb6645 100644
> --- a/scripts/package/kernel.spec
> +++ b/scripts/package/kernel.spec
> @@ -77,12 +77,16 @@ rm -rf %{buildroot}
>
>  %post
>  if [ -x /sbin/installkernel -a -r /boot/vmlinuz-%{KERNELRELEASE} -a -r /=
boot/System.map-%{KERNELRELEASE} ]; then
> +if [ $(rpm -qf /sbin/installkernel --queryformat "%{n}") =3D systemd-ude=
v ];then
> +/sbin/installkernel %{KERNELRELEASE} /boot/vmlinuz-%{KERNELRELEASE} /boo=
t/System.map-%{KERNELRELEASE}
> +else
>  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-%{KERNELRELE=
ASE}-rpm
>  fi
> +fi
>
>  %preun
>  if [ -x /sbin/new-kernel-pkg ]; then
> --
> 2.43.0
>


--
Best Regards


Masahiro Yamada