| Message ID | 20231212171044.1108464-1-jtornosm@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] rpm-pkg: simplify installkernel %post | expand |
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 >
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
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
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
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(+)