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

[Dave Young]

Hi,
On 03/27/18 at 08:19am, 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
>
> kdump-lib.sh | 210 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  kdumpctl     | 213 ++---------------------------------------------------------
>  2 files changed, 216 insertions(+), 207 deletions(-)
>
> diff --git a/kdump-lib.sh b/kdump-lib.sh
> index d981c4f..de292a4 100755
> --- a/kdump-lib.sh
> +++ b/kdump-lib.sh
> @@ -485,3 +485,213 @@ 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
>
> +}
>
> +#
> +# This function appends argument "$2=$3" to string ($1) if not already present.
> +#

Since this function moved to lib file, we should add comment here
about what param it expects from the caller exlain about what is
$1, $2 and $3,

ditto for other similar functions added in the patch..
> +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")); }'`
>
> +}
>
> +# 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
>
> +}
>
> +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 ]
>            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.
>    [ -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
>    echo $kdump_bootdir
>
> +}
>
> +# This function performs a series of edits on the command line.
> +# Store the final result in global $KDUMP_COMMANDLINE.
> +prepare_cmdline()
> +{
>    local kdump_commandline=$1
>    local kdump_commandline_remove=$2
>    local kdump_commandline_append=$3

If above 3 variables are const, then just directly use them,
no need extra variables here.
>    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
>    echo ${cmdline}
>
> +}
> diff --git a/kdumpctl b/kdumpctl
> index 4280e7e..6131e58 100755
> --- a/kdumpctl
> +++ b/kdumpctl
> @@ -21,6 +21,9 @@ image_time=0
>
> [[ $dracutbasedir ]] || dracutbasedir=/usr/lib/dracut
>  . $dracutbasedir/dracut-functions.sh
> +if [ -f /etc/sysconfig/kdump ]; then
>    . /etc/sysconfig/kdump
>
> +fi
>  . /lib/kdump/kdump-lib.sh

does kdump-lib.sh still depends on sysconfig setup?
If those functions accepts passed in arguments then it should
be fine to source sysconfig later like before.
> standard_kexec_args="-p"
> @@ -28,10 +31,6 @@ standard_kexec_args="-p"
>  # Some default values in case /etc/sysconfig/kdump doesn't include
>  KDUMP_COMMANDLINE_REMOVE="hugepages hugepagesz slub_debug"
>
> -if [ -f /etc/sysconfig/kdump ]; then
> . /etc/sysconfig/kdump
>
> -fi
>
> single_instance_lock()
>  {
>    local rc timeout=5
> @@ -62,93 +61,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 +273,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 +493,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 +583,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 +716,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 +825,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`
> -- 
> 2.9.5
> _______________________________________________
> kexec mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]

Thanks
Dave