[PATCH v2] rpm-pkg: simplify installkernel %post

[PATCH v2] rpm-pkg: simplify installkernel %post

[Jose Ignacio Tornos Martinez]

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 installkernel
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.

 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}") = systemd-udev ];then
+/sbin/installkernel %{KERNELRELEASE} /boot/vmlinuz-%{KERNELRELEASE} /boot/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-%{KERNELRELEASE}-rpm
 fi
+fi
 
 %preun
 if [ -x /sbin/new-kernel-pkg ]; then
-- 
2.43.0

Re: [PATCH v2] rpm-pkg: simplify installkernel %post

[Nathan Chancellor]

On Tue, Dec 12, 2023 at 06:10:44PM +0100, Jose Ignacio Tornos Martinez 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 installkernel
> 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>

Thanks, I can confirm that installing the same RPM package produced by
binrpm-pkg on both Fedora 38 and 39 works as expected now and the check
seems reasonable to me but I'll defer to Masahiro for further comments.

Tested-by: Nathan Chancellor <nathan@kernel.org>

> ---
> V1 -> V2:
> - Complete to be backward compatible with the previous installkernel
> application.
> 
>  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}") = systemd-udev ];then
> +/sbin/installkernel %{KERNELRELEASE} /boot/vmlinuz-%{KERNELRELEASE} /boot/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-%{KERNELRELEASE}-rpm
>  fi
> +fi
>  
>  %preun
>  if [ -x /sbin/new-kernel-pkg ]; then
> -- 
> 2.43.0
> 

Re: [PATCH v2] rpm-pkg: simplify installkernel %post

[Masahiro Yamada]

On Wed, Dec 13, 2023 at 2:10 AM 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 installkernel
> 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-%{KERNELRELEAE}
    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}") = systemd-udev ];then
> +/sbin/installkernel %{KERNELRELEASE} /boot/vmlinuz-%{KERNELRELEASE} /boot/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-%{KERNELRELEASE}-rpm
>  fi
> +fi
>
>  %preun
>  if [ -x /sbin/new-kernel-pkg ]; then
> --
> 2.43.0
>


--
Best Regards


Masahiro Yamada

Re: [PATCH v2] rpm-pkg: simplify installkernel %post

[Jose Ignacio Tornos Martinez]

Hello Masahiro,

> 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}/.
Ok, I did not dare to change a lot of things, overall in other sections.
I like the idea of imitating Fedora and making it easier.

> 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.
Ok, much clear in this way (and independent of software packages).
Again, good idea.

> Maybe, you can also convert the installkernel syntax to
> kernel-install while you are here.
Ok, that tool is more complete, I will try.

The next version will include all of this.

Thank you

Best rgards
José Ignacio

[PATCH v3] rpm-pkg: simplify installkernel %post

[Jose Ignacio Tornos Martinez]

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. Suitable manual actions are added as a default if tool is not
present (unusual).

Special installation case for discontinued architecture ia64 has been
removed.

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.
V2 -> V3:
- Follow the suggestions from Masahiro Yamada and change the installation
destination to avoid problems instead of checking the package.

 scripts/package/kernel.spec | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
index 3eee0143e0c5..17e7196c9be1 100644
--- a/scripts/package/kernel.spec
+++ b/scripts/package/kernel.spec
@@ -55,18 +55,12 @@ patch -p1 < %{SOURCE2}
 %{make} %{makeflags} KERNELRELEASE=%{KERNELRELEASE} KBUILD_BUILD_VERSION=%{release}
 
 %install
-mkdir -p %{buildroot}/boot
-%ifarch ia64
-mkdir -p %{buildroot}/boot/efi
-cp $(%{make} %{makeflags} -s image_name) %{buildroot}/boot/efi/vmlinuz-%{KERNELRELEASE}
-ln -s efi/vmlinuz-%{KERNELRELEASE} %{buildroot}/boot/
-%else
-cp $(%{make} %{makeflags} -s image_name) %{buildroot}/boot/vmlinuz-%{KERNELRELEASE}
-%endif
+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}
 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}'
@@ -76,12 +70,12 @@ 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
+kernel-install add %{KERNELRELEASE} /lib/modules/%{KERNELRELEASE}/vmlinuz
+else
+cp /lib/modules/%{KERNELRELEASE}/vmlinuz /boot/vmlinuz-%{KERNELRELEASE}
+cp /lib/modules/%{KERNELRELEASE}/System.map /boot/System.map-%{KERNELRELEASE}
+cp /lib/modules/%{KERNELRELEASE}/config /boot/config-%{KERNELRELEASE}
 fi
 
 %preun
@@ -100,7 +94,6 @@ fi
 %defattr (-, root, root)
 /lib/modules/%{KERNELRELEASE}
 %exclude /lib/modules/%{KERNELRELEASE}/build
-/boot/*
 
 %files headers
 %defattr (-, root, root)
-- 
2.43.0

Re: [PATCH v3] rpm-pkg: simplify installkernel %post

[Masahiro Yamada]

On Wed, Dec 20, 2023 at 12:57 AM Jose Ignacio Tornos Martinez
<jtornosm@redhat.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. Suitable manual actions are added as a default if tool is not
> present (unusual).
>
> Special installation case for discontinued architecture ia64 has been
> removed.


This code does not exist any more.

A patch applicable to linux-kbuild would be appreciated.





-- 
Best Regards
Masahiro Yamada

[PATCH v4] rpm-pkg: simplify installkernel %post

[Jose Ignacio Tornos Martinez]

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. Suitable manual actions are added as a default if tool is not
present (unusual).

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.
V2 -> V3:
- Follow the suggestions from Masahiro Yamada and change the installation
V3 -> V4:
- Make the patch applicable to linux-kbuild/for-next (ia64 support was
already removed).

 scripts/package/kernel.spec | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
index 89298983a169..17e7196c9be1 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,12 +70,12 @@ 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
+kernel-install add %{KERNELRELEASE} /lib/modules/%{KERNELRELEASE}/vmlinuz
+else
+cp /lib/modules/%{KERNELRELEASE}/vmlinuz /boot/vmlinuz-%{KERNELRELEASE}
+cp /lib/modules/%{KERNELRELEASE}/System.map /boot/System.map-%{KERNELRELEASE}
+cp /lib/modules/%{KERNELRELEASE}/config /boot/config-%{KERNELRELEASE}
 fi
 
 %preun
@@ -94,7 +94,6 @@ fi
 %defattr (-, root, root)
 /lib/modules/%{KERNELRELEASE}
 %exclude /lib/modules/%{KERNELRELEASE}/build
-/boot/*
 
 %files headers
 %defattr (-, root, root)
-- 
2.43.0

Re: [PATCH v4] rpm-pkg: simplify installkernel %post

[Nathan Chancellor]

On Tue, Dec 19, 2023 at 09:17:19PM +0100, Jose Ignacio Tornos Martinez 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. Suitable manual actions are added as a default if tool is not
> present (unusual).
> 
> cc: stable@vger.kernel.org
> Co-Developed-by: Davide Cavalca <dcavalca@meta.com>
> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>

Still works for me on Fedora 39, thanks for the fix!

Tested-by: Nathan Chancellor <nathan@kernel.org>

> ---
> V1 -> V2:
> - Complete to be backward compatible with the previous installkernel
> application.
> V2 -> V3:
> - Follow the suggestions from Masahiro Yamada and change the installation
> V3 -> V4:
> - Make the patch applicable to linux-kbuild/for-next (ia64 support was
> already removed).
> 
>  scripts/package/kernel.spec | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
> index 89298983a169..17e7196c9be1 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,12 +70,12 @@ 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
> +kernel-install add %{KERNELRELEASE} /lib/modules/%{KERNELRELEASE}/vmlinuz
> +else
> +cp /lib/modules/%{KERNELRELEASE}/vmlinuz /boot/vmlinuz-%{KERNELRELEASE}
> +cp /lib/modules/%{KERNELRELEASE}/System.map /boot/System.map-%{KERNELRELEASE}
> +cp /lib/modules/%{KERNELRELEASE}/config /boot/config-%{KERNELRELEASE}
>  fi
>  
>  %preun
> @@ -94,7 +94,6 @@ fi
>  %defattr (-, root, root)
>  /lib/modules/%{KERNELRELEASE}
>  %exclude /lib/modules/%{KERNELRELEASE}/build
> -/boot/*
>  
>  %files headers
>  %defattr (-, root, root)
> -- 
> 2.43.0
> 

Re: [PATCH v4] rpm-pkg: simplify installkernel %post

[Masahiro Yamada]

On Wed, Dec 20, 2023 at 5:17 AM Jose Ignacio Tornos Martinez
<jtornosm@redhat.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. Suitable manual actions are added as a default if tool is not
> present (unusual).


This paragraph should be reworded,
and the corresponding code should be fixed.


This patch works for fedora 38 and fedora 39,
but may break openSUSE tumbleweed, at least.



The kernel-install itself does not copy files,
but invoked scripts in /usr/lib/kernel/install.d/


In Fedora,

/usr/lib/kernel/install.d/20-grub.install

copies those files to /boot/.



In openSUSE, the 'udev' package provides
/usr/bin/kernel-install,
but /usr/lib/kernel/install.d/20-grub.install
is missing.


masahiro@ea071f1f0504:~> rpm -qpl udev-254.5-8.1.x86_64.rpm | grep kernel
/usr/bin/kernel-install
/usr/lib/kernel
/usr/lib/kernel/install.conf
/usr/lib/kernel/install.d
/usr/lib/kernel/install.d/50-depmod.install
/usr/lib/kernel/install.d/90-loaderentry.install
/usr/lib/kernel/install.d/90-uki-copy.install
/usr/lib/systemd/system/sockets.target.wants/systemd-udevd-kernel.socket
/usr/lib/systemd/system/systemd-udevd-kernel.socket
/usr/share/bash-completion/completions/kernel-install
/usr/share/man/man8/kernel-install.8.gz
/usr/share/man/man8/systemd-udevd-kernel.socket.8.gz
/usr/share/zsh/site-functions/_kernel-install



In openSUSE with the udev package installed,
none of vmlinuz, config, System.map is copied
to the /boot directory.




Applying the following on top should fix the regression,
although I did not test any other RPM-based distros.



diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
index afef3b0f6a3d..eb5cc440216b 100644
--- a/scripts/package/kernel.spec
+++ b/scripts/package/kernel.spec
@@ -71,12 +71,13 @@ rm -rf %{buildroot}

 %post
 if [ -x /usr/bin/kernel-install ]; then
-kernel-install add %{KERNELRELEASE} /lib/modules/%{KERNELRELEASE}/vmlinuz
-else
-cp /lib/modules/%{KERNELRELEASE}/vmlinuz /boot/vmlinuz-%{KERNELRELEASE}
-cp /lib/modules/%{KERNELRELEASE}/System.map /boot/System.map-%{KERNELRELEASE}
-cp /lib/modules/%{KERNELRELEASE}/config /boot/config-%{KERNELRELEASE}
+    /usr/bin/kernel-install add %{KERNELRELEASE}
/lib/modules/%{KERNELRELEASE}/vmlinuz
 fi
+for file in vmlinuz System.map config; do
+    if [ ! -e "/boot/${file}-%{KERNELRELEASE}" ]; then
+        cp "/lib/modules/%{KERNELRELEASE}/${file}"
"/boot/${file}-%{KERNELRELEASE}"
+    fi
+done

 %preun
 if [ -x /sbin/new-kernel-pkg ]; then












>
> 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.
> V2 -> V3:
> - Follow the suggestions from Masahiro Yamada and change the installation
> V3 -> V4:
> - Make the patch applicable to linux-kbuild/for-next (ia64 support was
> already removed).
>
>  scripts/package/kernel.spec | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
> index 89298983a169..17e7196c9be1 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,12 +70,12 @@ 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
> +kernel-install add %{KERNELRELEASE} /lib/modules/%{KERNELRELEASE}/vmlinuz
> +else
> +cp /lib/modules/%{KERNELRELEASE}/vmlinuz /boot/vmlinuz-%{KERNELRELEASE}
> +cp /lib/modules/%{KERNELRELEASE}/System.map /boot/System.map-%{KERNELRELEASE}
> +cp /lib/modules/%{KERNELRELEASE}/config /boot/config-%{KERNELRELEASE}
>  fi
>
>  %preun
> @@ -94,7 +94,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

Re: [PATCH v4] rpm-pkg: simplify installkernel %post

[Jose Ignacio Tornos Martinez]

Hello Masahiro,
 
Sorry for the delay in answering, I was on holiday.

Good catch
Ok, I will create a new patch including your suggestion and I will test it with Fedora, openSUSE and others if possible.

Thank you again for your help

Best regards
José Ignacio

[PATCH V5 1/2] rpm-pkg: simplify installkernel %post

[Jose Ignacio Tornos Martinez]

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@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.
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
+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
 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

Re: [PATCH V5 1/2] rpm-pkg: simplify installkernel %post

[Masahiro Yamada]

On Sun, Jan 14, 2024 at 5:07 PM Jose Ignacio Tornos Martinez
<jtornosm@redhat.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@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.
> 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

Re: [PATCH V5 1/2] rpm-pkg: simplify installkernel %post

[Jose Ignacio Tornos Martinez]

>> %post
>> ...
>> +if [ -e /boot/vmlinuz-%{KERNELRELEASE} ] && file -bL /boot/vmlinuz-%{KERNELRELEASE} | grep -q " #%{release} "; then
>> ...
>>
>>  %preun
...
>> +if [ -e /boot/vmlinuz-%{KERNELRELEASE} ] && file -bL /boot/vmlinuz-%{KERNELRELEASE} | grep -q " #%{release} "; then
> I do not understand why this is needed.
> Please explain.
Of course. 
Fisrt of all, I have seen (i.e. openSUSE Tumbleweed) that in the same way
that vmlinuz, System.map and config was not copied when the rpm was
installed (because of the reason that you commented with the missing
script), they were not removed when the rpm was removed, so I have added
the lines to remove in a similar way as you suggested for install. 
And I have seen as well (i.e. openSUSE Tumbleweed)) that if the a new rpm
is installed (same release but bigger build version to use default options
for the tool), vmlinuz, System.map and config are not copied from %post
because vmlinuz, System.map and config already exist and the situation is
not good, because /lib/modules/{KERNELRELEASE} is updated but the commented
files in /boot are not updated. That is the reason why I have tried to
identify when vmlinuz, System.map and config are not the good ones, to copy
too.
Besides, in the commented situation, the older rpm (same release but older
build version) is removed and with that, the new vmlinuz, System.map and
config are removed too. That is the reason that I have tried to identify
again the files, removing only the suitable vmlinuz, System.map and config
with the same release and build number requested.

> And, is the output of 'file' standardized?
With no more information, file is going to print the strings in the file,
that is, the information containig release, version, ... and here we can
find what we are interested in. So in some way depends on vmlinuz binary.

> 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.
You are right, again good catch.
I will try to think something for aarch64. Maybe something more general,
and  independent of the kernel binary name, is possible and valid for other
architectures, maybe with rpm command.
If nothing comes up, I will do only for x86.

> Unreadable.
> I suggested code with indentation and quotation,
> but you got rid of them.
I did not want to modify the style.
Ok, I will follow your suggestion, it's clearer to me too.

Thanks

Best regards
José Ignacio

Re: [PATCH V5 1/2] rpm-pkg: simplify installkernel %post

[Masahiro Yamada]

[-- Attachment #1: Type: text/plain, Size: 3647 bytes --]

On Thu, Jan 18, 2024 at 11:12 PM Jose Ignacio Tornos Martinez
<jtornosm@redhat.com> wrote:
>
> >> %post
> >> ...
> >> +if [ -e /boot/vmlinuz-%{KERNELRELEASE} ] && file -bL /boot/vmlinuz-%{KERNELRELEASE} | grep -q " #%{release} "; then
> >> ...
> >>
> >>  %preun
> ...
> >> +if [ -e /boot/vmlinuz-%{KERNELRELEASE} ] && file -bL /boot/vmlinuz-%{KERNELRELEASE} | grep -q " #%{release} "; then
> > I do not understand why this is needed.
> > Please explain.
> Of course.
> Fisrt of all, I have seen (i.e. openSUSE Tumbleweed) that in the same way
> that vmlinuz, System.map and config was not copied when the rpm was
> installed (because of the reason that you commented with the missing
> script), they were not removed when the rpm was removed, so I have added
> the lines to remove in a similar way as you suggested for install.


Here, you are wrong.

Those installed files should be removed by %ghost markers.
I already have a local patch to do this.
(see the attachment)


I just asked you to fix up the code as I suggested in v4.



> And I have seen as well (i.e. openSUSE Tumbleweed)) that if the a new rpm
> is installed (same release but bigger build version to use default options
> for the tool), vmlinuz, System.map and config are not copied from %post
> because vmlinuz, System.map and config already exist and the situation is
> not good, because /lib/modules/{KERNELRELEASE} is updated but the commented
> files in /boot are not updated. That is the reason why I have tried to
> identify when vmlinuz, System.map and config are not the good ones, to copy
> too.



For me (on Fedora 39 and openSUSE Tumbleweed), rpm fails due to file conflict.

vagrant@opensuse-tumbleweed20231218:~> sudo rpm -i
kernel-6.7.0_12924_g660a5f4a53e7-4.x86_64.rpm
file /lib/modules/6.7.0-12924-g660a5f4a53e7/vmlinuz from install of
kernel-6.7.0_12924_g660a5f4a53e7-4.x86_64 conflicts with file from
package kernel-6.7.0_12924_g660a5f4a53e7-3.x86_64

So, this does not happen.




> Besides, in the commented situation, the older rpm (same release but older
> build version) is removed and with that, the new vmlinuz, System.map and
> config are removed too. That is the reason that I have tried to identify
> again the files, removing only the suitable vmlinuz, System.map and config
> with the same release and build number requested.
>
> > And, is the output of 'file' standardized?
> With no more information, file is going to print the strings in the file,
> that is, the information containig release, version, ... and here we can
> find what we are interested in. So in some way depends on vmlinuz binary.
>
> > 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.
> You are right, again good catch.
> I will try to think something for aarch64. Maybe something more general,
> and  independent of the kernel binary name, is possible and valid for other
> architectures, maybe with rpm command.
> If nothing comes up, I will do only for x86.


No. Please do not.


>
> > Unreadable.
> > I suggested code with indentation and quotation,
> > but you got rid of them.
> I did not want to modify the style.
> Ok, I will follow your suggestion, it's clearer to me too.
>
> Thanks
>
> Best regards
> José Ignacio
>


-- 
Best Regards
Masahiro Yamada

[-- Attachment #2: 0001-kbuild-rpm-pkg-specify-more-files-as-ghost.patch --]
[-- Type: text/x-patch, Size: 3430 bytes --]

From ed1f0f3be8f4a679a078dad0b9acaf23b31447b2 Mon Sep 17 00:00:00 2001
From: Masahiro Yamada <masahiroy@kernel.org>
Date: Mon, 25 Dec 2023 22:58:17 +0900
Subject: [PATCH] kbuild: rpm-pkg: specify more files as %ghost

Installing the kernel package is fine, but when uninstalling it, the
following warnings are shown:

  warning: file modules.symbols.bin: remove failed: No such file or directory
  warning: file modules.symbols: remove failed: No such file or directory
  warning: file modules.softdep: remove failed: No such file or directory
  warning: file modules.devname: remove failed: No such file or directory
  warning: file modules.dep.bin: remove failed: No such file or directory
  warning: file modules.dep: remove failed: No such file or directory
  warning: file modules.builtin.bin: remove failed: No such file or directory
  warning: file modules.builtin.alias.bin: remove failed: No such file or directory
  warning: file modules.alias.bin: remove failed: No such file or directory
  warning: file modules.alias: remove failed: No such file or directory

The %preun scriptlet runs 'kernel-install remove', which in turn invokes
/usr/lib/kernel/install.d/50-depmod.install to remove those files before
the actual package removal.

Modern RPM-based distributions do not ship files generated by depmod.
Mark them as %ghost in order to exclude them from the package, but still
claim the ownership on them. Do likewise for the files copied to /boot.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
 scripts/package/kernel.spec | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
index 6a7625da7ea5..c72466a93843 100644
--- a/scripts/package/kernel.spec
+++ b/scripts/package/kernel.spec
@@ -66,6 +66,27 @@ ln -fns /usr/src/kernels/%{KERNELRELEASE} %{buildroot}/lib/modules/%{KERNELRELEA
 %{make} %{makeflags} run-command KBUILD_RUN_COMMAND='${srctree}/scripts/package/install-extmod-build %{buildroot}/usr/src/kernels/%{KERNELRELEASE}'
 %endif
 
+{
+    echo "%defattr (-, root, root)"
+
+    for f in System.map config kernel modules.builtin modules.builtin.modinfo \
+             modules.order vmlinuz; do
+        echo "/lib/modules/%{KERNELRELEASE}/${f}"
+    done
+
+    for x in alias alias.bin builtin.alias.bin builtin.bin dep dep.bin devname \
+             softdep symbols symbols.bin; do
+        echo "%ghost /lib/modules/%{KERNELRELEASE}/modules.${x}"
+    done
+
+    for f in System.map config initramfs vmlinuz; do
+        echo "%ghost /boot/${f}-%{KERNELRELEASE}"
+    done
+
+    echo "%exclude /lib/modules/%{KERNELRELEASE}/build"
+    echo "%exclude /kernel-files.list"
+} > %{buildroot}/kernel-files.list
+
 %clean
 rm -rf %{buildroot}
 
@@ -78,6 +99,9 @@ for file in vmlinuz System.map config; do
          cp "/lib/modules/%{KERNELRELEASE}/${file}" "/boot/${file}-%{KERNELRELEASE}"
     fi
 done
+if [ ! -e "/lib/modules/%{KERNELRELEASE}/modules.dep" ]; then
+    /usr/sbin/depmod %{KERNELRELEASE}
+fi
 
 %preun
 if [ -x /sbin/new-kernel-pkg ]; then
@@ -91,10 +115,7 @@ if [ -x /sbin/update-bootloader ]; then
 /sbin/update-bootloader --remove %{KERNELRELEASE}
 fi
 
-%files
-%defattr (-, root, root)
-/lib/modules/%{KERNELRELEASE}
-%exclude /lib/modules/%{KERNELRELEASE}/build
+%files -f %{buildroot}/kernel-files.list
 
 %files headers
 %defattr (-, root, root)
-- 
2.40.1

Re: [PATCH V5 1/2] rpm-pkg: simplify installkernel %post

[Jose Ignacio Tornos Martinez]

> Those installed files should be removed by %ghost markers.
> I already have a local patch to do this.
> (see the attachment)
I like the idea of your new patch, a lot of things can be fixed in that way.
Ok, I will remove the extra code to remove (%preun) in the patch.
Just a comment about your patch: for openSUSE /boot/initramfs-* files are
called /boot/initrd-* and maybe someone would not require it (i.e. embedded
systems). If it is created it is normally removed and it might not be
necessary (although I like your idea to control it).

> I just asked you to fix up the code as I suggested in v4.
Now I understand why no code was added in %preun.
Ok, your suggestion was very good, but let me try and explain better with
commands what I would like to fix after next point. When I said 'update'
wasn't clear, I think.
If it doesn't fit with your idea or global usage, I will include your
suggestion like it is.

> For me (on Fedora 39 and openSUSE Tumbleweed), rpm fails due to file conflict.
> 
> vagrant@opensuse-tumbleweed20231218:~> sudo rpm -i
> kernel-6.7.0_12924_g660a5f4a53e7-4.x86_64.rpm
> file /lib/modules/6.7.0-12924-g660a5f4a53e7/vmlinuz from install of
> kernel-6.7.0_12924_g660a5f4a53e7-4.x86_64 conflicts with file from
> package kernel-6.7.0_12924_g660a5f4a53e7-3.x86_64
> 
> So, this does not happen.
I was refering to the cases when zypper is used to install a new kernel with
the same release and different build number or when 'rpm -i --replacefiles' 
is used (in this case it would be necessary to remove the old kernel with
'rpm -e --justdb' too).
In this cases we only need the possibility of copying the files from the new
package and not only if they don't exist.
I have thought about an easy way (no extra or problematic command) and I think
I have it.
In addition to your suggestion (if the file does not exit in /boot), I will
just compare the file in /boot with the file in /lib/modules/%{KERNELRELEASE}
and if it is not the same, we allow copying:
%post
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

Let me try with a new patch to know your opinion.

Thanks

Best regards
José Ignacio

[PATCH] rpm-pkg: simplify installkernel %post

[Jose Ignacio Tornos Martinez]

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@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.
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
 
 %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
 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}
 fi
 
 %files
 %defattr (-, root, root)
 /lib/modules/%{KERNELRELEASE}
 %exclude /lib/modules/%{KERNELRELEASE}/build
-/boot/*
 
 %files headers
 %defattr (-, root, root)
-- 
2.43.0

Re: [PATCH] rpm-pkg: simplify installkernel %post

[Masahiro Yamada]

On Tue, Jan 23, 2024 at 3:23 AM Jose Ignacio Tornos Martinez
<jtornosm@redhat.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 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@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.
> 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

Re: [PATCH] rpm-pkg: simplify installkernel %post

[Jose Ignacio Tornos Martinez]

Ok, I will simplify the check to copy as you say (cpm --silent returns
error if file doesn't exist) and and I will limit the indentation to the
modifications.
If you are going to modify the rest, I think other style changes are not
necessary.

Thanks

Best regards
José Ignacio

[PATCH v7] rpm-pkg: simplify installkernel %post

[Jose Ignacio Tornos Martinez]

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@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.
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.
V6 -> V7
- Simplify check to copy (cpm --silent return error if file doesn't exist).
- Limit indientation to modifications.

 scripts/package/kernel.spec | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
index 89298983a169..f58726671fb3 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,13 +70,14 @@ 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 ! cmp --silent "/lib/modules/%{KERNELRELEASE}/${file}" "/boot/${file}-%{KERNELRELEASE}"; then
+		cp "/lib/modules/%{KERNELRELEASE}/${file}" "/boot/${file}-%{KERNELRELEASE}"
+	fi
+done
 
 %preun
 if [ -x /sbin/new-kernel-pkg ]; then
@@ -94,7 +95,6 @@ fi
 %defattr (-, root, root)
 /lib/modules/%{KERNELRELEASE}
 %exclude /lib/modules/%{KERNELRELEASE}/build
-/boot/*
 
 %files headers
 %defattr (-, root, root)
-- 
2.43.0

Re: [PATCH v7] rpm-pkg: simplify installkernel %post

[Masahiro Yamada]

On Mon, Jan 29, 2024 at 6:28 PM Jose Ignacio Tornos Martinez
<jtornosm@redhat.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 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@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.
> 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.
> V6 -> V7
> - Simplify check to copy (cpm --silent return error if file doesn't exist).
> - Limit indientation to modifications.


Applied to linux-kbuild/fixes. Thanks.


-- 
Best Regards
Masahiro Yamada

[PATCH V5 2/2] rpm-pkg: avoid install/remove the running kernel

[Jose Ignacio Tornos Martinez]

Normally, upper tools and scripts are checking if we want to modify the
running kernel but not always. Check install and remove operation that
affect the running kernel to avoid possible issues.

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@vger.kernel.org
Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
---
 scripts/package/kernel.spec | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
index 74542af8cbfe..216514cbf5f6 100644
--- a/scripts/package/kernel.spec
+++ b/scripts/package/kernel.spec
@@ -69,6 +69,12 @@ ln -fns /usr/src/kernels/%{KERNELRELEASE} %{buildroot}/lib/modules/%{KERNELRELEA
 %clean
 rm -rf %{buildroot}
 
+%pre
+if [ $(uname -r) == %{KERNELRELEASE} ];then
+echo "same kernel release is running" > /dev/stderr
+exit 1
+fi
+
 %post
 if [ -x /usr/bin/kernel-install ]; then
 /usr/bin/kernel-install add %{KERNELRELEASE} /lib/modules/%{KERNELRELEASE}/vmlinuz
@@ -85,6 +91,10 @@ fi
 done
 
 %preun
+if [ $(uname -r) == %{KERNELRELEASE} ];then
+echo "same kernel release is running" > /dev/stderr
+exit 1
+fi
 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
-- 
2.43.0

Re: [PATCH V5 2/2] rpm-pkg: avoid install/remove the running kernel

[Masahiro Yamada]

On Sun, Jan 14, 2024 at 5:07 PM Jose Ignacio Tornos Martinez
<jtornosm@redhat.com> wrote:
>
> Normally, upper tools and scripts are checking if we want to modify the
> running kernel but not always. Check install and remove operation that
> affect the running kernel to avoid possible issues.
>
> 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@vger.kernel.org
> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
> ---
>  scripts/package/kernel.spec | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
> index 74542af8cbfe..216514cbf5f6 100644
> --- a/scripts/package/kernel.spec
> +++ b/scripts/package/kernel.spec
> @@ -69,6 +69,12 @@ ln -fns /usr/src/kernels/%{KERNELRELEASE} %{buildroot}/lib/modules/%{KERNELRELEA
>  %clean
>  rm -rf %{buildroot}
>
> +%pre
> +if [ $(uname -r) == %{KERNELRELEASE} ];then
> +echo "same kernel release is running" > /dev/stderr


What is the problem with this?






> +exit 1
> +fi
> +
>  %post
>  if [ -x /usr/bin/kernel-install ]; then
>  /usr/bin/kernel-install add %{KERNELRELEASE} /lib/modules/%{KERNELRELEASE}/vmlinuz
> @@ -85,6 +91,10 @@ fi
>  done
>
>  %preun
> +if [ $(uname -r) == %{KERNELRELEASE} ];then
> +echo "same kernel release is running" > /dev/stderr
> +exit 1
> +fi
>  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
> --
> 2.43.0
>
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH V5 2/2] rpm-pkg: avoid install/remove the running kernel

[Jose Ignacio Tornos Martinez]

> What is the problem with this?
In my opinion, it is risky to remove the kernel that is running, that is
the reason why I am trying to protect this.
If you try to remove or update (and the running kernel is removed), if the
kernel and modules are already preloaded in memory, it could not happen
anything but some extra action could be necessary or automatically started,
and even the new kernel could not boot.
Fedora and others are taking this into account with upper tools and declare
the running kernel as protected avoinding this action. But others
(i.e. openSUSE Tumbleweed) allow this behavior.
It may only be a safety measure but it can also be beneficial to avoid
problems, just in case. 
Besides, in this way, all the tested distributions would have the same
behavior.

If it is ok, I will create a following version patch describing the problem
better and using indentation as you suggested for the other patch.

Thanks

Best regards
José Ignacio

Re: [PATCH V5 2/2] rpm-pkg: avoid install/remove the running kernel

[Masahiro Yamada]

On Thu, Jan 18, 2024 at 11:12 PM Jose Ignacio Tornos Martinez
<jtornosm@redhat.com> wrote:
>
> > What is the problem with this?
> In my opinion, it is risky to remove the kernel that is running, that is
> the reason why I am trying to protect this.
> If you try to remove or update (and the running kernel is removed), if the
> kernel and modules are already preloaded in memory, it could not happen
> anything but some extra action could be necessary or automatically started,
> and even the new kernel could not boot.
> Fedora and others are taking this into account with upper tools and declare
> the running kernel as protected avoinding this action. But others
> (i.e. openSUSE Tumbleweed) allow this behavior.



As I replied in 1/2, I see an error like this:


vagrant@opensuse-tumbleweed20231218:~> sudo rpm -i
kernel-6.7.0_12924_g660a5f4a53e7-4.x86_64.rpm
file /lib/modules/6.7.0-12924-g660a5f4a53e7/vmlinuz from install of
kernel-6.7.0_12924_g660a5f4a53e7-4.x86_64 conflicts with file from
package kernel-6.7.0_12924_g660a5f4a53e7-3.x86_64



You can proceed with 'rpm -i --force', but
that is the user's responsibility if something bad happens.







> It may only be a safety measure but it can also be beneficial to avoid
> problems, just in case.
> Besides, in this way, all the tested distributions would have the same
> behavior.
>
> If it is ok, I will create a following version patch describing the problem
> better and using indentation as you suggested for the other patch.


No, not OK.





>
> Thanks
>
> Best regards
> José Ignacio
>

--
Best Regards
Masahiro Yamada

Re: [PATCH V5 2/2] rpm-pkg: avoid install/remove the running kernel

[Jose Ignacio Tornos Martinez]

> You can proceed with 'rpm -i --force', but
> that is the user's responsibility if something bad happens.

> No, not OK.
Ok, understood and maybe it is interesting for the user.
Indeed, zypper tool from openSUSE can be configured to protect the running
kernel as dnf.
I drop this patch.

Thanks

Best regards
José Ignacio