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

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: [email protected]
Co-Developed-by: Davide Cavalca <[email protected]>
Signed-off-by: Jose Ignacio Tornos Martinez <[email protected]>
---
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


2023-12-12 19:19:40

by Nathan Chancellor

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

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: [email protected]
> Co-Developed-by: Davide Cavalca <[email protected]>
> Signed-off-by: Jose Ignacio Tornos Martinez <[email protected]>

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 <[email protected]>

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

2023-12-18 18:27:40

by Masahiro Yamada

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

On Wed, Dec 13, 2023 at 2:10 AM Jose Ignacio Tornos Martinez
<[email protected]> 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: [email protected]
> Co-Developed-by: Davide Cavalca <[email protected]>
> Signed-off-by: Jose Ignacio Tornos Martinez <[email protected]>
> ---
> 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

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

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


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

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: [email protected]
Co-Developed-by: Davide Cavalca <[email protected]>
Signed-off-by: Jose Ignacio Tornos Martinez <[email protected]>
---
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


2023-12-19 16:52:32

by Masahiro Yamada

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

On Wed, Dec 20, 2023 at 12:57 AM Jose Ignacio Tornos Martinez
<[email protected]> 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

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

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: [email protected]
Co-Developed-by: Davide Cavalca <[email protected]>
Signed-off-by: Jose Ignacio Tornos Martinez <[email protected]>
---
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


2023-12-20 17:18:27

by Nathan Chancellor

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

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: [email protected]
> Co-Developed-by: Davide Cavalca <[email protected]>
> Signed-off-by: Jose Ignacio Tornos Martinez <[email protected]>

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

Tested-by: Nathan Chancellor <[email protected]>

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

2023-12-26 04:03:03

by Masahiro Yamada

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

On Wed, Dec 20, 2023 at 5:17 AM Jose Ignacio Tornos Martinez
<[email protected]> 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: [email protected]
> Co-Developed-by: Davide Cavalca <[email protected]>
> Signed-off-by: Jose Ignacio Tornos Martinez <[email protected]>
> ---
> 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

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

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


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

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: [email protected]
Co-Developed-by: Davide Cavalca <[email protected]>
Signed-off-by: Jose Ignacio Tornos Martinez <[email protected]>
---
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


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

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: [email protected]
Signed-off-by: Jose Ignacio Tornos Martinez <[email protected]>
---
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


2024-01-17 01:31:02

by Masahiro Yamada

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

On Sun, Jan 14, 2024 at 5:07 PM Jose Ignacio Tornos Martinez
<[email protected]> 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: [email protected]
> Co-Developed-by: Davide Cavalca <[email protected]>
> Signed-off-by: Jose Ignacio Tornos Martinez <[email protected]>
> ---
> 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

2024-01-17 01:32:51

by Masahiro Yamada

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

On Sun, Jan 14, 2024 at 5:07 PM Jose Ignacio Tornos Martinez
<[email protected]> 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: [email protected]
> Signed-off-by: Jose Ignacio Tornos Martinez <[email protected]>
> ---
> 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

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

>> %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


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

> 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


2024-01-21 17:33:43

by Masahiro Yamada

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

On Thu, Jan 18, 2024 at 11:12 PM Jose Ignacio Tornos Martinez
<[email protected]> 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


Attachments:
0001-kbuild-rpm-pkg-specify-more-files-as-ghost.patch (3.35 kB)

2024-01-21 17:34:38

by Masahiro Yamada

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

On Thu, Jan 18, 2024 at 11:12 PM Jose Ignacio Tornos Martinez
<[email protected]> 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

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

> 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


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

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: [email protected]
Co-Developed-by: Davide Cavalca <[email protected]>
Signed-off-by: Jose Ignacio Tornos Martinez <[email protected]>
---
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


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

> 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




2024-01-28 07:33:28

by Masahiro Yamada

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

On Tue, Jan 23, 2024 at 3:23 AM Jose Ignacio Tornos Martinez
<[email protected]> 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: [email protected]
> Co-Developed-by: Davide Cavalca <[email protected]>
> Signed-off-by: Jose Ignacio Tornos Martinez <[email protected]>
> ---
> 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

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

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


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

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: [email protected]
Co-Developed-by: Davide Cavalca <[email protected]>
Signed-off-by: Jose Ignacio Tornos Martinez <[email protected]>
---
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


2024-01-31 01:49:59

by Masahiro Yamada

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

On Mon, Jan 29, 2024 at 6:28 PM Jose Ignacio Tornos Martinez
<[email protected]> 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: [email protected]
> Co-Developed-by: Davide Cavalca <[email protected]>
> Signed-off-by: Jose Ignacio Tornos Martinez <[email protected]>
> ---
> 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