Re: [PATCH 1/2 V5] move some common functions from kdumpctl to kdump-lib.sh

[lijiang]

在 2018年05月04日 04:18, Kazuhito Hagio 写道:
> Hi Lianbo,
>
> Thank you for the update!
> but I noticed that the early kdump initramfs with this patchset
> doesn't have several commands which are used in kdump-lib.sh.
> I'm sorry I could not comment about it at once..
>
> I think that we can replace them with commands it already has
> (except for hexdump), but it might be safer to install them,
> because of code consistency and the less amount of changes.
>
> On 5/1/2018 4:34 AM, Lianbo Jiang wrote:
> > we move some common functions from kdumpctl to kdump-lib.sh, the
> > functions could be used in other modules, such as early kdump.
> > It has no bad effect.
> > Signed-off-by: Lianbo Jiang
> >
> > Some changes based the patch v3&v4:
> > -1.kdump-lib.sh
> > -add comment for some functions.
> > -modify check_boot_dir() function.
> >
> > kdump-lib.sh | 217 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  kdumpctl     | 206 +-------------------------------------------------------
> >  2 files changed, 220 insertions(+), 203 deletions(-)
> >
> > diff --git a/kdump-lib.sh b/kdump-lib.sh
> > index d981c4f..544a6ae 100755
> > --- a/kdump-lib.sh
> > +++ b/kdump-lib.sh
> > @@ -485,3 +485,220 @@ get_dracut_args_target()
> >  {
> >      echo $1 | grep "--mount" | sed "s/.*--mount .(.*)/\1/" | cut -d' ' -f1
> >  }
> >
> > +check_crash_mem_reserved()
> > +{
> >    local mem_reserved
> >    mem_reserved=$(cat /sys/kernel/kexec_crash_size)
> >    if [ $mem_reserved -eq 0 ]; then
> >            echo "No memory reserved for crash kernel"
> >            return 1
> >    fi
> >    return 0
> >
> > +}
> >
> > +check_kdump_feasibility()
> > +{
> >    if [ ! -e /sys/kernel/kexec_crash_loaded ]; then
> >            echo "Kdump is not supported on this kernel"
> >            return 1
> >    fi
> >    check_crash_mem_reserved
> >    return $?
> >
> > +}
> >
> > +check_current_kdump_status()
> > +{
> >    if [ ! -f /sys/kernel/kexec_crash_loaded ];then
> >            echo "Perhaps CONFIG_CRASH_DUMP is not enabled in kernel"
> >            return 1
> >    fi
> >    rc=`cat /sys/kernel/kexec_crash_loaded`
> >    if [ $rc == 1 ]; then
> >            return 0
> >    else
> >            return 1
> >    fi
> >
> > +}
> >
> > +# remove_cmdline_param <kernel cmdline> <param1> [<param2>] ... [<paramN>]
> > +# Remove a list of kernel parameters from a given kernel cmdline and print the result.
> > +# For each "arg" in the removing params list, "arg" and "arg=xxx" will be removed if exists.
> > +remove_cmdline_param()
> > +{
> >    local cmdline=$1
> >    shift
> >    for arg in $@; do
> >            cmdline=`echo $cmdline | \
> >                     sed -e "s/\b$arg=[^ ]*//g" \
> >                         -e "s/^$arg\b//g" \
> >                         -e "s/[[:space:]]$arg\b//g" \
> >                         -e "s/\s\+/ /g"`
> >    done
> >    echo $cmdline
> >
> > +}
> >
> > +#
> > +# This function returns the "apicid" of the boot
> > +# cpu (cpu 0) if present.
> > +#
> > +get_bootcpu_apicid()
> > +{
> > awk '                                                       \
> >    BEGIN { CPU = "-1"; }                                   \
> >    $1=="processor" && $2==":"      { CPU = $NF; }          \
> >    CPU=="0" && /^apicid/           { print $NF; }          \
> >    '                                                       \
> >    /proc/cpuinfo
> >
> > +}
> >
> > +#
> > +# append_cmdline <kernel cmdline> <parameter name> <parameter value>
> > +# This function appends argument "$2=$3" to string ($1) if not already present.
> > +#
> > +append_cmdline()
> > +{
> >    local cmdline=$1
> >    local newstr=${cmdline/$2/""}
> >    # unchanged str implies argument wasn't there
> >    if [ "$cmdline" == "$newstr" ]; then
> >            cmdline="${cmdline} ${2}=${3}"
> >    fi
> >    echo $cmdline
> >
> > +}
> >
> > +# This function check iomem and determines if we have more than
> > +# 4GB of ram available. Returns 1 if we do, 0 if we dont
> > +need_64bit_headers()
> > +{
> >    return `tail -n 1 /proc/iomem | awk '{ split ($1, r, "-"); \
> >    print (strtonum("0x" r[2]) > strtonum("0xffffffff")); }'`
>
> since it doesn't have "tail", I saw this on an i686 system:
>
> dracut-cmdline[173]: /lib/kdump-lib.sh: line 593: tail: command not found
> > +}
> >
> > +# Check if secure boot is being enforced.
> > +#
> > +# Per Peter Jones, we need check efivar SecureBoot-$(the UUID) and
> > +# SetupMode-$(the UUID), they are both 5 bytes binary data. The first four
> > +# bytes are the attributes associated with the variable and can safely be
> > +# ignored, the last bytes are one-byte true-or-false variables. If SecureBoot
> > +# is 1 and SetupMode is 0, then secure boot is being enforced.
> > +#
> > +# Assume efivars is mounted at /sys/firmware/efi/efivars.
> > +is_secure_boot_enforced()
> > +{
> >    local secure_boot_file setup_mode_file
> >    local secure_boot_byte setup_mode_byte
> >    secure_boot_file=$(find /sys/firmware/efi/efivars -name SecureBoot-* 2>/dev/null)
> >    setup_mode_file=$(find /sys/firmware/efi/efivars -name SetupMode-* 2>/dev/null)
>
> same as above, it doesn't have "find".
> >    if [ -f "$secure_boot_file" ] && [ -f "$setup_mode_file" ]; then
> >            secure_boot_byte=$(hexdump -v -e '/1 "%d\ "' $secure_boot_file|cut -d' ' -f 5)
> >            setup_mode_byte=$(hexdump -v -e '/1 "%d\ "' $setup_mode_file|cut -d' ' -f 5)
>
> same as above, it doesn't have "hexdump" and "cut".
> >            if [ "$secure_boot_byte" = "1" ] && [ "$setup_mode_byte" = "0" ]; then
> >                    return 0
> >            fi
> >    fi
> >    return 1
> >
> > +}
> >
> > +#
> > +# prepare_kexec_args <kexec args>
> > +# This function prepares kexec argument.
> > +#
> > +prepare_kexec_args()
> > +{
> >    local kexec_args=$1
> >    local found_elf_args
> >    ARCH=`uname -m`
> >    if [ "$ARCH" == "i686" -o "$ARCH" == "i386" ]
> >    then
> >    need_64bit_headers
> >    if [ $? == 1 ]
>
> lost an indent here?
>
> BTW, without this patch, the kdump-lib.sh mostly has 4 spaces
> for an indent.  I think it would be better to have the same style
> in a file if possible.
> >            then
> >                    found_elf_args=`echo $kexec_args | grep elf32-core-headers`
> >                    if [ -n "$found_elf_args" ]
> >                    then
> >                            echo -n "Warning: elf32-core-headers overrides correct elf64 setting"
> >                            echo
> >                    else
> >                            kexec_args="$kexec_args --elf64-core-headers"
> >                    fi
> >            else
> >                    found_elf_args=`echo $kexec_args | grep elf64-core-headers`
> >                    if [ -z "$found_elf_args" ]
> >                    then
> >                            kexec_args="$kexec_args --elf32-core-headers"
> >                    fi
> >            fi
> >    fi
> >    echo $kexec_args
> >
> > +}
> >
> > +check_boot_dir()
> > +{
> >    local kdump_bootdir=$1
> >    #If user specify a boot dir for kdump kernel, let's use it. Otherwise
> >    #check whether it's a atomic host. If yes parse the subdirectory under
> >    #/boot; If not just find it under /boot.
> >    if [ -n "$kdump_bootdir" ]; then
> >            echo "$kdump_bootdir"
> >            return
> >    fi
> >    if ! is_atomic || [ "$(uname -m)" = "s390x" ]; then
> >            kdump_bootdir="/boot"
> >    else
> >            eval $(cat /proc/cmdline| grep "BOOT_IMAGE" | cut -d' ' -f1)
> >            kdump_bootdir="/boot"$(dirname $BOOT_IMAGE)
>
> also here, it doesn't have "cut" and "dirname".
>
> As far as I've reviewed this, the above commands are missing
> in the initramfs on my test systems.

Thanks, Kazu
I am glad that you help me review this patch. 
I will check the code style and fix the problem that some commands can not be found.

Thanks
Lianbo
> Thanks,
> Kazu
> >    fi
> >    echo $kdump_bootdir
> >
> > +}
> >
> > +#
> > +# prepare_cmdline <commandline> <commandline remove> <commandline append>
> > +# This function performs a series of edits on the command line.
> > +# Store the final result in global $KDUMP_COMMANDLINE.
> > +prepare_cmdline()
> > +{
> >    local cmdline id
> >    if [ -z "$1" ]; then
> >            cmdline=$(cat /proc/cmdline)
> >    else
> >            cmdline="$1"
> >    fi
> >    # These params should always be removed
> >    cmdline=$(remove_cmdline_param "$cmdline" crashkernel panic_on_warn)
> >    # These params can be removed configurably
> >    cmdline=$(remove_cmdline_param "$cmdline" "$2")
> >    # Always remove "root=X", as we now explicitly generate all kinds
> >    # of dump target mount information including root fs.
> >    #
> >    # We do this before KDUMP_COMMANDLINE_APPEND, if one really cares
> >    # about it(e.g. for debug purpose), then can pass "root=X" using
> >    # KDUMP_COMMANDLINE_APPEND.
> >    cmdline=$(remove_cmdline_param "$cmdline" root)
> >    # With the help of "--hostonly-cmdline", we can avoid some interitage.
> >    cmdline=$(remove_cmdline_param "$cmdline" rd.lvm.lv rd.luks.uuid rd.dm.uuid rd.md.uuid fcoe)
> >    cmdline="${cmdline} $3"
> >    id=$(get_bootcpu_apicid)
> >    if [ ! -z ${id} ] ; then
> >            cmdline=$(append_cmdline "${cmdline}" disable_cpu_apicid ${id})
> >    fi
> >    echo ${cmdline}
> >
> > +}
> > diff --git a/kdumpctl b/kdumpctl
> > index 4280e7e..afbe96e 100755
> > --- a/kdumpctl
> > +++ b/kdumpctl
> > @@ -62,93 +62,6 @@ determine_dump_mode()
> >    fi
> >  }
> >
> > -# remove_cmdline_param <kernel cmdline> <param1> [<param2>] ... [<paramN>]
> > -# Remove a list of kernel parameters from a given kernel cmdline and print the result.
> > -# For each "arg" in the removing params list, "arg" and "arg=xxx" will be removed if exists.
> > -remove_cmdline_param()
> > -{
> > local cmdline=$1
> > shift
> > for arg in $@; do
> > cmdline=`echo $cmdline | \
> > 	 sed -e "s/\b$arg=[^ ]*//g" \
> > 	     -e "s/^$arg\b//g" \
> > 	     -e "s/[[:space:]]$arg\b//g" \
> > 	     -e "s/\s\+/ /g"`
> > done
> > echo $cmdline
> >
> > -}
> >
> > -#
> > -# This function returns the "apicid" of the boot
> > -# cpu (cpu 0) if present.
> > -#
> > -get_bootcpu_apicid()
> > -{
> > awk '							\
> > BEGIN { CPU = "-1"; }					\
> > $1=="processor" && $2==":"	{ CPU = $NF; }		\
> > CPU=="0" && /^apicid/		{ print $NF; }		\
> > '							\
> > /proc/cpuinfo
> >
> > -}
> >
> > -#
> > -# This function appends argument "$2=$3" to string ($1) if not already present.
> > -#
> > -append_cmdline()
> > -{
> > local cmdline=$1
> > local newstr=${cmdline/$2/""}
> > # unchanged str implies argument wasn't there
> > if [ "$cmdline" == "$newstr" ]; then
> > cmdline="${cmdline} ${2}=${3}"
> > fi
> > echo $cmdline
> >
> > -}
> >
> > -# This function performs a series of edits on the command line.
> > -# Store the final result in global $KDUMP_COMMANDLINE.
> > -prepare_cmdline()
> > -{
> > local cmdline id
> > if [ -z "$KDUMP_COMMANDLINE" ]; then
> > cmdline=$(cat /proc/cmdline)
> > else
> > cmdline=${KDUMP_COMMANDLINE}
> > fi
> > # These params should always be removed
> > cmdline=$(remove_cmdline_param "$cmdline" crashkernel panic_on_warn)
> > # These params can be removed configurably
> > cmdline=$(remove_cmdline_param "$cmdline" ${KDUMP_COMMANDLINE_REMOVE})
> > # Always remove "root=X", as we now explicitly generate all kinds
> > # of dump target mount information including root fs.
> > #
> > # We do this before KDUMP_COMMANDLINE_APPEND, if one really cares
> > # about it(e.g. for debug purpose), then can pass "root=X" using
> > # KDUMP_COMMANDLINE_APPEND.
> > cmdline=$(remove_cmdline_param "$cmdline" root)
> > # With the help of "--hostonly-cmdline", we can avoid some interitage.
> > cmdline=$(remove_cmdline_param "$cmdline" rd.lvm.lv rd.luks.uuid rd.dm.uuid rd.md.uuid fcoe)
> > cmdline="${cmdline} ${KDUMP_COMMANDLINE_APPEND}"
> > id=$(get_bootcpu_apicid)
> > if [ ! -z ${id} ] ; then
> > cmdline=$(append_cmdline "${cmdline}" disable_cpu_apicid ${id})
> > fi
> > KDUMP_COMMANDLINE=$cmdline
> >
> > -}
> >
> > save_core()
> >  {
> >    coredir="/var/crash/`date +"%Y-%m-%d-%H:%M"`"
> > @@ -361,21 +274,6 @@ get_pcs_cluster_modified_files()
> >    echo $modified_files
> >  }
> >
> > -check_boot_dir()
> > -{
> > #If user specify a boot dir for kdump kernel, let's use it. Otherwise
> > #check whether it's a atomic host. If yes parse the subdirectory under
> > #/boot; If not just find it under /boot.
> > [ -n "$KDUMP_BOOTDIR" ] && return
> > if ! is_atomic || [ "$(uname -m)" = "s390x" ]; then
> > KDUMP_BOOTDIR="/boot"
> > else
> > eval $(cat /proc/cmdline| grep "BOOT_IMAGE" | cut -d' ' -f1)
> > KDUMP_BOOTDIR="/boot"$(dirname $BOOT_IMAGE)
> > fi
> >
> > -}
> >
> > setup_initrd()
> >  {
> >    DEFAULT_INITRD="${KDUMP_BOOTDIR}/initramfs-`uname -r`.img"
> > @@ -596,7 +494,7 @@ check_rebuild()
> >    local _force_no_rebuild force_no_rebuild="0"
> >    local ret system_modified="0"
> > check_boot_dir
> >
> > KDUMP_BOOTDIR=$(check_boot_dir "${KDUMP_BOOTDIR}")
> >
> > if [ -z "$KDUMP_KERNELVER" ]; then
> > kdump_kver=`uname -r`
> >
> > @@ -686,44 +584,13 @@ check_rebuild()
> >    return $?
> >  }
> >
> > -# This function check iomem and determines if we have more than
> > -# 4GB of ram available. Returns 1 if we do, 0 if we dont
> > -need_64bit_headers()
> > -{
> > return `tail -n 1 /proc/iomem | awk '{ split ($1, r, "-"); \
> > print (strtonum("0x" r[2]) > strtonum("0xffffffff")); }'`
> >
> > -}
> >
> > # Load the kdump kernel specified in /etc/sysconfig/kdump
> >  # If none is specified, try to load a kdump kernel with the same version
> >  # as the currently running kernel.
> >  load_kdump()
> >  {
> > ARCH=`uname -m`
> > if [ "$ARCH" == "i686" -o "$ARCH" == "i386" ]
> > then
> > need_64bit_headers
> > if [ $? == 1 ]
> > then
> > 	FOUND_ELF_ARGS=`echo $KEXEC_ARGS | grep elf32-core-headers`
> > 	if [ -n "$FOUND_ELF_ARGS" ]
> > 	then
> > 		echo -n "Warning: elf32-core-headers overrides correct elf64 setting"
> > 		echo
> > 	else	
> > 		KEXEC_ARGS="$KEXEC_ARGS --elf64-core-headers"
> > 	fi
> > else
> > 	FOUND_ELF_ARGS=`echo $KEXEC_ARGS | grep elf64-core-headers`
> > 	if [ -z "$FOUND_ELF_ARGS" ]
> > 	then
> > 		KEXEC_ARGS="$KEXEC_ARGS --elf32-core-headers"
> > 	fi
> > fi
> > fi
> > prepare_cmdline
> >
> > KEXEC_ARGS=$(prepare_kexec_args "${KEXEC_ARGS}")
> >
> > KDUMP_COMMANDLINE=$(prepare_cmdline "${KDUMP_COMMANDLINE}" "${KDUMP_COMMANDLINE_REMOVE}" "${KDUMP_COMMANDLINE_APPEND}")
> >
> > # For secureboot enabled machines, use new kexec file based syscall.
> > # Old syscall will always fail as it does not have capability to
> >
> > @@ -850,21 +717,6 @@ check_current_fadump_status()
> >    return 1
> >  }
> >
> > -check_current_kdump_status()
> > -{
> > if [ ! -f /sys/kernel/kexec_crash_loaded ];then
> > echo "Perhaps CONFIG_CRASH_DUMP is not enabled in kernel"
> > return 1
> > fi
> > rc=`cat /sys/kernel/kexec_crash_loaded`
> > if [ $rc == 1 ]; then
> > return 0
> > else
> > return 1
> > fi
> >
> > -}
> >
> > check_current_status()
> >  {
> >    if [ $DEFAULT_DUMP_MODE == "fadump" ]; then
> > @@ -974,58 +826,6 @@ selinux_relabel()
> >    done
> >  }
> >
> > -# Check if secure boot is being enforced.
> > -#
> > -# Per Peter Jones, we need check efivar SecureBoot-$(the UUID) and
> > -# SetupMode-$(the UUID), they are both 5 bytes binary data. The first four
> > -# bytes are the attributes associated with the variable and can safely be
> > -# ignored, the last bytes are one-byte true-or-false variables. If SecureBoot
> > -# is 1 and SetupMode is 0, then secure boot is being enforced.
> > -#
> > -# Assume efivars is mounted at /sys/firmware/efi/efivars.
> > -is_secure_boot_enforced()
> > -{
> > local secure_boot_file setup_mode_file
> > local secure_boot_byte setup_mode_byte
> > secure_boot_file=$(find /sys/firmware/efi/efivars -name SecureBoot-* 2>/dev/null)
> > setup_mode_file=$(find /sys/firmware/efi/efivars -name SetupMode-* 2>/dev/null)
> > if [ -f "$secure_boot_file" ] && [ -f "$setup_mode_file" ]; then
> > secure_boot_byte=$(hexdump -v -e '/1 "%d\ "' $secure_boot_file|cut -d' ' -f 5)
> > setup_mode_byte=$(hexdump -v -e '/1 "%d\ "' $setup_mode_file|cut -d' ' -f 5)
> > if [ "$secure_boot_byte" = "1" ] && [ "$setup_mode_byte" = "0" ]; then
> > 	return 0
> > fi
> > fi
> > return 1
> >
> > -}
> >
> > -check_crash_mem_reserved()
> > -{
> > local mem_reserved
> > mem_reserved=$(cat /sys/kernel/kexec_crash_size)
> > if [ $mem_reserved -eq 0 ]; then
> > echo "No memory reserved for crash kernel"
> > return 1
> > fi
> > return 0
> >
> > -}
> >
> > -check_kdump_feasibility()
> > -{
> > if [ ! -e /sys/kernel/kexec_crash_loaded ]; then
> > echo "Kdump is not supported on this kernel"
> > return 1
> > fi
> > check_crash_mem_reserved
> > return $?
> >
> > -}
> >
> > check_fence_kdump_config()
> >  {
> >    local hostname=`hostname`