Hi Coiby,
On Fri, 1 Mar 2024 18:00:31 +0800 Coiby Xu coxu@redhat.com wrote:
This patch upstreams the to-be-split-out kdump-utils to https://github.com/rhkdump/kdump-utils. And it also simplify the .spec file by putting the installation logic into a Makefile.
Can you please explain to which repo this patch shall be added. I found that the github repo already has this patch applied but it is still missing in the fedora repo. I'm a little bit confused because I would expect that when you make the github repo the upstream for the fedora repo you also remove all the scripts/configs from the fedora repo as they are now fetched from github.
Furthermore, I noticed that while you add the new kdump-utils.spec you don't remove the corresponding lines from the kexec-tools.spec. Won't that cause a conflict as kdump-utils and kexec-tools try to add the same files?
Finally, I noticed that the github repo still contains the kexec-tools.spec and backported patches. Can't they be removed when the repo is only meant to be upstream for our fedora specific tooling?
Cc: Philipp Rudo prudo@redhat.com Cc: Carl George carl@redhat.com Signed-off-by: Coiby Xu coxu@redhat.com
COPYING | 339 +++++++++++++++++++++++++++++++++++++++++++++++ Makefile | 85 ++++++++++++ kdump-utils.spec | 133 +++++++++++++++++++ 3 files changed, 557 insertions(+) create mode 100644 COPYING create mode 100644 Makefile create mode 100644 kdump-utils.spec
diff --git a/COPYING b/COPYING new file mode 100644 index 00000000..d159169d --- /dev/null +++ b/COPYING @@ -0,0 +1,339 @@
GNU GENERAL PUBLIC LICENSEVersion 2, June 1991
The spec file says
+License: GPL-2.0-only AND LGPL-2.1-or-later
Do we need to ship the LGPL 2.1 license as well? If not, could we drop the license altogether?
[...]
diff --git a/Makefile b/Makefile new file mode 100644 index 00000000..ce05afa7 --- /dev/null +++ b/Makefile @@ -0,0 +1,85 @@ +prefix ?= /usr +libdir ?= ${prefix}/lib +datadir ?= ${prefix}/share +pkglibdir ?= ${libdir}/kdump +sysconfdir ?= /etc +bindir ?= ${prefix}/bin +sbindir ?= ${prefix}/sbin +mandir ?= ${prefix}/share/man +localstatedir ?= /var +sharedstatedir ?= /var/lib +udevrulesdir ?= ${libdir}/udev/rules.d +systemdsystemunitdir ?= ${libdir}/systemd/system/ +ARCH ?= $(shell uname -m) +dracutmoddir = $(DESTDIR)${libdir}/dracut/modules.d +kdumpbasemoddir = $(dracutmoddir)/99kdumpbase
+dracut-modules:
- mkdir -p $(dracutmoddir)
- mkdir -p -m755 $(kdumpbasemoddir)
- install -m 755 dracut-kdump.sh $(kdumpbasemoddir)/kdump.sh
- install -m 755 dracut-module-setup.sh $(kdumpbasemoddir)/module-setup.sh
- install -m 755 dracut-monitor_dd_progress $(kdumpbasemoddir)/monitor_dd_progress.sh
- install -m 644 dracut-kdump-emergency.service $(kdumpbasemoddir)/kdump-emergency.service
- install -m 644 dracut-kdump-capture.service $(kdumpbasemoddir)/kdump-capture.service
- install -m 644 dracut-kdump-emergency.target $(kdumpbasemoddir)/kdump-emergency.target
^^^^^ Unnecessary tab
- mkdir -p -m755 $(dracutmoddir)/99earlykdump
- install -m 755 dracut-early-kdump.sh $(dracutmoddir)/99earlykdump/kdump.sh
- install -m 755 dracut-early-kdump-module-setup.sh $(dracutmoddir)/99earlykdump/kdump-module-setup.sh
^^^^^ Unnecessary tab
+ifneq (,$(filter $(ARCH),ppc64 ppc64le))
I find the use of ifneq pretty confusing when you want to include the lines when ARCH matches one of the given ones. I'm afraid this can lead to bugs like the one below that easily could be avoided. Personally I would use
ifeq ($(filter ppcle64 ppc64, $(ARCH)), $(ARCH))
- mkdir -p -m755 $(dracutmoddir)/99zz-fadumpinit
- install -m 755 dracut-fadump-init-fadump.sh $(dracutmoddir)/99zz-fadumpinit/init-fadump.sh
- install -m 755 dracut-fadump-module-setup.sh $(dracutmoddir)/99zz-fadumpinit/module-setup.sh
+endif
+kdump-conf: gen-kdump-conf.sh
- ./gen-kdump-conf.sh $(ARCH) > kdump.conf
+kdump-sysconfig: gen-kdump-sysconfig.sh
- ./gen-kdump-sysconfig.sh $(ARCH) > kdump.sysconfig
+manpages:
- install -D -m 644 mkdumprd.8 kdumpctl.8 -t $(DESTDIR)$(mandir)/man8
- install -D -m 644 kdump.conf.5 $(DESTDIR)$(mandir)/man5/kdump.conf.5
+install: dracut-modules kdump-conf kdump-sysconfig manpages
- mkdir -p $(DESTDIR)$(pkglibdir)
- mkdir -p -m755 $(DESTDIR)$(sysconfdir)/kdump/pre.d
- mkdir -p -m755 $(DESTDIR)$(sysconfdir)/kdump/post.d
- mkdir -p -m755 $(DESTDIR)$(localstatedir)/crash
- mkdir -p -m755 $(DESTDIR)$(udevrulesdir)
- mkdir -p -m755 $(DESTDIR)$(sharedstatedir)/kdump
^^^^^ Unnecessary tab
- install -D -m 755 kdumpctl $(DESTDIR)$(bindir)/kdumpctl
- install -D -m 755 mkdumprd $(DESTDIR)$(sbindir)/mkdumprd
^ trailing space
- install -D -m 644 kdump.conf $(DESTDIR)$(sysconfdir)
- install -D -m 644 kdump.sysconfig $(DESTDIR)$(sysconfdir)/sysconfig/kdump
- install -D -m 755 kdump-lib.sh kdump-lib-initramfs.sh kdump-logger.sh -t $(DESTDIR)$(pkglibdir)
+ifneq (,$(filter $(ARCH),ppc64 ppc64le))
- install -m 755 mkfadumprd $(DESTDIR)$(sbindir)
- install -m 755 kdump-migrate-action.sh kdump-restart.sh -t $(DESTDIR)$(pkglibdir)
+endif
+ifneq ($(ARCH),s390x)
- install -m 755 kdump-udev-throttler $(DESTDIR)$(udevrulesdir)/../kdump-udev-throttler
+endif
+ifneq (,$(filter $(ARCH),s390x ppc64 ppc64le))
^^^^^ ifeq? 98-kexec.rules should be included for all arches except the given ones, shouldn't it.
In general, why don't you use a else-if here instead, i.e.
ifeq ($(ARCH), s390x) # For s390x the ELF header is created in the kdump kernel and therefore kexec # udev rules are not required else ifeq ($(filter ppcle64 ppc64, $(ARCH)), $(ARCH)) install -m 644 98-kexec.rules.ppc64 $(DESTDIR)$(udevrulesdir)/98-kexec.rules install -m 755 -D %{SOURCE37} $(DESTDIR)$(libdir)/kernel/install.d/60-fadump.install else install -m 644 98-kexec.rules $(DESTDIR)$(udevrulesdir)/98-kexec.rules endif
Personally I find this much better readable.
- # For s390x the ELF header is created in the kdump kernel and therefore kexec
- # udev rules are not required
- install -m 644 98-kexec.rules $(DESTDIR)$(udevrulesdir)/98-kexec.rules
+endif
+ifneq (,$(filter $(ARCH),ppc64 ppc64le))
- install -m 644 98-kexec.rules.ppc64 $(DESTDIR)$(udevrulesdir)/98-kexec.rules
- install -m 755 -D %{SOURCE37} $(DESTDIR)$(libdir)/kernel/install.d/60-fadump.install
+endif
- install -D -m 644 kdump.service $(DESTDIR)$(systemdsystemunitdir)/kdump.service
- install -m 755 -D kdump-dep-generator.sh $(DESTDIR)$(libdir)/systemd/system-generators/kdump-dep-generator.sh
- install -m 755 -D 60-kdump.install $(DESTDIR)$(libdir)/kernel/install.d/60-kdump.install
- install -m 755 -D 92-crashkernel.install $(DESTDIR)$(libdir)/kernel/install.d/92-crashkernel.install
diff --git a/kdump-utils.spec b/kdump-utils.spec new file mode 100644 index 00000000..c479376b --- /dev/null +++ b/kdump-utils.spec @@ -0,0 +1,133 @@ +Name: kdump-utils +Version: 1.0.42 +Release: 1%{?dist} +Summary: Kernel crash dump collection utilities
+License: GPL-2.0-only AND LGPL-2.1-or-later +URL: https://github.com/rhkdump/kdump-utils +Source0: https://github.com/rhkdump/kdump-utils/archive/v%%7Bversion%7D/%%7Bname%7D-%...
+%ifarch ppc64 ppc64le +Requires(post): servicelog +Recommends: keyutils +%endif +Requires(pre): coreutils +Requires(pre): sed +Requires: kexec-tools >= 2.0.28-5 +Requires: makedumpfile +Requires: dracut >= 058 +Requires: dracut-network >= 058 +Requires: dracut-squash >= 058 +Requires: ethtool +Requires: util-linux +# Needed for UKI support +Recommends: binutils +Recommends: grubby +Recommends: hostname +BuildRequires: systemd-rpm-macros
BuildRequires: make?
With the Makefile added
+%ifnarch s390x +Requires: systemd-udev%{?_isa} +%endif +%description +kdump-utils is reponsible for collecting the crash kernel dump. It builds and +loads the kdump initramfs so when a kernel crashes, the system will boot the +kdump kernel and initramfs to save the colletecd crash kernel dump to specified +target.
+%prep +%autosetup
+%install
Unnecessary newline
Thanks Philipp
+%make_install
+%post +# don't try to systemctl preset the kdump service for old kexec-tools +# +# when the old kexec-tools gets removed, this trigger will be excuted to +# create a file. So later the posttrans scriptlet will know there is no need to +# systemctl preset the kdump service. +# This solution can be dropped in F41 when we assume no users will use old +# version of kexec-tools. +%define kexec_tools_no_preset %{_localstatedir}/lib/rpm-state/kexec-tools.no-preset +%triggerun -- kexec-tools +touch %{kexec_tools_no_preset}
+touch /etc/kdump.conf
+%ifarch ppc64 ppc64le +servicelog_notify --remove --command=/usr/lib/kdump/kdump-migrate-action.sh 2>/dev/null +servicelog_notify --add --command=/usr/lib/kdump/kdump-migrate-action.sh --match='refcode="#MIGRATE" and serviceable=0' --type=EVENT --method=pairs_stdin >/dev/null +%endif
+%postun +%systemd_postun_with_restart kdump.service
+%preun +%ifarch ppc64 ppc64le +servicelog_notify --remove --command=/usr/lib/kdump/kdump-migrate-action.sh >/dev/null +%endif +%systemd_preun kdump.service
+%posttrans +# don't try to systemctl preset the kdump service for old kexec-tools +if [[ -f %{kexec_tools_no_preset} ]]; then
- # this if branch can be removed in F41 when we assume no users will use the old kexec-tools
- rm %{kexec_tools_no_preset}
+else
- # Initial installation
- %systemd_post kdump.service
+fi +# Try to reset kernel crashkernel value to new default value or set up +# crasherkernel value for new install +# +# Note +# 1. Skip ostree systems as they are not supported. +# 2. For Fedora 36 and RHEL9, "[ $1 == 1 ]" in posttrans scriptlet means both install and upgrade; +# For Fedora > 36, "[ $1 == 1 ]" only means install and "[ $1 == 2 ]" means upgrade +if [ ! -f /run/ostree-booted ] && [ $1 == 1 -o $1 == 2 ]; then
- kdumpctl _reset-crashkernel-after-update
- :
+fi
+%files +%ifarch ppc64 ppc64le +%{_sbindir}/mkfadumprd +%{_prefix}/lib/kernel/install.d/60-fadump.install +%endif +%{_sbindir}/mkdumprd +%{_bindir}/* +%{_prefix}/lib/kdump +%config(noreplace,missingok) %{_sysconfdir}/sysconfig/kdump +%config(noreplace,missingok) %verify(not mtime) %{_sysconfdir}/kdump.conf +%ifnarch s390x +%{_udevrulesdir} +%{_udevrulesdir}/../kdump-udev-throttler +%endif +%{_prefix}/lib/dracut/modules.d/* +%dir %{_localstatedir}/crash +%dir %{_sysconfdir}/kdump +%dir %{_sysconfdir}/kdump/pre.d +%dir %{_sysconfdir}/kdump/post.d +%dir %{_sharedstatedir}/kdump +%{_mandir}/man8/kdumpctl.8* +%{_mandir}/man8/mkdumprd.8* +%{_mandir}/man5/kdump.conf.5* +%{_unitdir}/kdump.service +%{_prefix}/lib/systemd/system-generators/kdump-dep-generator.sh +%{_prefix}/lib/kernel/install.d/60-kdump.install +%{_prefix}/lib/kernel/install.d/92-crashkernel.install +%license COPYING +%doc kexec-kdump-howto.txt +%doc early-kdump-howto.txt +%doc fadump-howto.txt +%doc kdump-in-cluster-environment.txt +%doc live-image-kdump-howto.txt +%doc crashkernel-howto.txt
+%changelog +* Tue Oct 24 2023 Coiby coxu@redhat.com - 1.0.42-1 +- split from kexec-tools