One more nit...
On Mon, 25 Apr 2022 15:18:47 +0200
Philipp Rudo <prudo(a)redhat.com> wrote:
Hi Kairui,
great idea!
A few comments/questions below.
On Sat, 23 Apr 2022 22:49:52 +0800
Kairui Song <ryncsn(a)gmail.com> wrote:
> From: Kairui Song <kasong(a)tencent.com>
>
> These kdump.sysconfig.* files are almost identical with a bit different
> in several parameters, just use a simple script to generate them upon
> packaging. This should make it easier to maintain, updating a comment or
> param for certain arch can be done in one place.
>
> There are some minor differences with the generated version due to some
> arch's sysconfig is not up-to-dated, this actually fixed the issues,
> to check these differences:
>
> # for arch in aarch64 i386 ppc64 ppc64le s390x x86_64; do
> ./kdump-gen-sysconfig.sh kdump.sysconfig.template $arch >
kdump.sysconfig.$arch.new
> git checkout HEAD^ kdump.sysconfig.$arch &>/dev/null
> echo "Diff of arch $arch:"
> diff kdump.sysconfig.$arch kdump.sysconfig.$arch.new; echo ""
> done; git reset;
>
> Diff of arch aarch64:
> > #What is the images extension. Relocatable kernels don't have one
> > KDUMP_IMG_EXT=""
>
> Diff of arch i386:
>
> Diff of arch ppc64:
> 42,43d41
> < #Specify the action after failure
> <
>
> Diff of arch ppc64le:
> 42,43d41
> < #Specify the action after failure
> <
>
> Diff of arch s390x:
> 26,28d25
> < # Any additional /sbin/mkdumprd arguments required.
> < MKDUMPRD_ARGS=""
> <
>
> Diff of arch x86_64:
>
> Signed-off-by: Kairui Song <kasong(a)tencent.com>
> ---
> kdump-gen-sysconfig.sh | 51 ++++++++++++++++++
> kdump.sysconfig.aarch64 | 53 ------------------
> kdump.sysconfig.i386 | 56 -------------------
> kdump.sysconfig.ppc64 | 58 --------------------
> kdump.sysconfig.ppc64le | 58 --------------------
> kdump.sysconfig.s390x | 59 ---------------------
> kdump.sysconfig => kdump.sysconfig.template | 3 ++
> kdump.sysconfig.x86_64 | 56 -------------------
> kexec-tools.spec | 18 +++----
> 9 files changed, 60 insertions(+), 352 deletions(-)
> create mode 100755 kdump-gen-sysconfig.sh
> delete mode 100644 kdump.sysconfig.aarch64
> delete mode 100644 kdump.sysconfig.i386
> delete mode 100644 kdump.sysconfig.ppc64
> delete mode 100644 kdump.sysconfig.ppc64le
> delete mode 100644 kdump.sysconfig.s390x
> rename kdump.sysconfig => kdump.sysconfig.template (96%)
> delete mode 100644 kdump.sysconfig.x86_64
>
> diff --git a/kdump-gen-sysconfig.sh b/kdump-gen-sysconfig.sh
> new file mode 100755
> index 0000000..1eac80d
> --- /dev/null
> +++ b/kdump-gen-sysconfig.sh
> @@ -0,0 +1,51 @@
> +#!/bin/bash
> +
> +TEMPLATE=$1
> +ARCH=$2
> +SED_EXP=""
> +
> +update_param()
> +{
> + SED_EXP="${SED_EXP}s/^$1=.*$/$1=\"$2\"/;"
> +}
> +
> +case "$ARCH" in
> +aarch64)
> + update_param KEXEC_ARGS "-s"
> + update_param KDUMP_COMMANDLINE_APPEND \
> + "irqpoll nr_cpus=1 reset_devices cgroup_disable=memory udev.children-max=2
panic=10 swiotlb=noforce novmcoredd cma=0 hugetlb_cma=0"
hmm... in the end KDUMP_COMMANDLINE_APPEND is set for every (supported)
arch. So having a "default" set in the template currently doesn't make
much sense. Furthermore the command line has quite a few common entries
between the arches. So how about splitting update_param into two
functions param_add and param_remove that add remove the given params
from the default set in the template. With that I hope it will be easier
to see the differences between the arches.
BTW, is there a difference between nr_cpus=1 and maxcpus=1?
> + ;;
> +i386)
> + update_param KDUMP_COMMANDLINE_APPEND \
> + "irqpoll nr_cpus=1 reset_devices numa=off udev.children-max=2 panic=10
transparent_hugepage=never novmcoredd cma=0 hugetlb_cma=0"
> + ;;
> +ppc64)
> + update_param KDUMP_COMMANDLINE_REMOVE \
> + "hugepages hugepagesz slub_debug quiet log_buf_len swiotlb
hugetlb_cma"
> + update_param KDUMP_COMMANDLINE_APPEND \
> + "irqpoll maxcpus=1 noirqdistrib reset_devices cgroup_disable=memory numa=off
udev.children-max=2 ehea.use_mcs=0 panic=10 kvm_cma_resv_ratio=0
transparent_hugepage=never novmcoredd hugetlb_cma=0"
> + update_param KEXEC_ARGS "--dt-no-old-root"
> + ;;
> +ppc64le)
> + update_param KDUMP_COMMANDLINE_REMOVE \
> + "hugepages hugepagesz slub_debug quiet log_buf_len swiotlb
hugetlb_cma"
> + update_param KDUMP_COMMANDLINE_APPEND \
> + "irqpoll maxcpus=1 noirqdistrib reset_devices cgroup_disable=memory numa=off
udev.children-max=2 ehea.use_mcs=0 panic=10 kvm_cma_resv_ratio=0
transparent_hugepage=never novmcoredd hugetlb_cma=0"
> + update_param KEXEC_ARGS "--dt-no-old-root -s"
> + ;;
> +s390x)
> + update_param KDUMP_COMMANDLINE_REMOVE \
> + "hugepages hugepagesz slub_debug quiet log_buf_len swiotlb vmcp_cma cma
hugetlb_cma prot_virt"
> + update_param KDUMP_COMMANDLINE_APPEND \
> + "nr_cpus=1 cgroup_disable=memory numa=off udev.children-max=2 panic=10
transparent_hugepage=never novmcoredd vmcp_cma=0 cma=0 hugetlb_cma=0"
> + update_param KEXEC_ARGS "-s"
> + ;;
> +x86_64)
> + update_param KDUMP_COMMANDLINE_APPEND \
> + "irqpoll nr_cpus=1 reset_devices cgroup_disable=memory mce=off numa=off
udev.children-max=2 panic=10 acpi_no_memhotplug transparent_hugepage=never nokaslr
hest_disable novmcoredd cma=0 hugetlb_cma=0"
> + update_param KEXEC_ARGS \
> + "-s"
> + ;;
Add some error handling? I.e.
*)
echo "Unknown architecture $ARCH provided" >&2
exit 1
;;
Thanks
Philipp
> +esac
> +
> +sed "$SED_EXP" "$TEMPLATE"
... If SED_EXP is empty sed will fail with an error. So to be fully
save the last line should probably be changed to something like
if [[ -n $SED_EXP ]]; then
sed "$SED_EXP" "$TEMPLATE"
else
cat $TEMPLATE
fi