Philipp Rudo <prudo(a)redhat.com> 于2022年4月25日周一 21:32写道:
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
Hmm, it seems `sed "" <file>` will not raise an error but simple cat
the file, I tested with GNU sed 4.8 and BSD sed, both works.
Did you test it? Maybe I can add a comment here if it actually works,
no need to add extra if/cat.