This patch set adds support for a new Github-based workflow so it's easier to run the tests as early and as often as possible. Hopefully as a result, the bugs can be prevented or fixed early.
Four kinds of tests are triggered when there is a pull request created in a Github kexec-tools repo [1], - format check using shfmt - static analysis using shellcheck - ShellSpec unit tests (test cases in spec/) - integration tests ( tests cases in tests/)
Before enabling Github Action, I've gone through all the test failures most of which are shellcheck warnings to make all tests have green status [2]. Because there are so many test warnings from shellcheck, it makes it difficult to find new test failures. Despite a few false positives, most warnings are valuable and make the code more robust.
This patch set is also available for review on [1].
Why Github over Gilab? ======================
The biggest reason is Gitlab doesn't doesn't allow merge request from forked project to use a specific runner i.e. our own testing machine to run the tests [3] (I have to use a specific runner because Gitlab's shared runners are very slow. For example, it takes ~3 hours to finish current integration tests while it can be finished in ~8min using our own testing machine). A second reason if out of security concern. Currently for Gitlab specific runner, we need to run the Docker as root which makes the testing machine quite vulnerable.
[1] https://github.com/coiby/kexec-tools/pull/1 [2] https://github.com/coiby/kexec-tools/runs/7987323055?check_suite_focus=true [3] https://www.google.com/url?q=https://pagure.io/fedora-infrastructure/issue/1...
Coiby Xu (24): format code using shfmt fix shellcheck warnings for dracut-early-kdump-module-setup.sh fix shellcheck warnings for dracut-early-kdump.sh fix shellcheck errors of dracut-fadump-init-fadump.sh Disable shellcheck rule SC2154 for variables defined by dracut Eliminate shellcheck SC2129 warnings Eliminate shellcheck SC2086 warnings Eliminate shellcheck SC2029 warnings Eliminate shellcheck SC2034 warnings Eliminate shellcheck SC2181 warnings Eliminate shellcheck SC2154 warning Eliminate shellcheck SC2005 warning Eliminate shellcheck SC2046 warning Eliminate shellcheck SC1004 warning Eliminate shellcheck SC2295 warnings Eliminate shellcheck SC2295 warnings Eliminate shellcheck SC2013 warning Eliminate shellcheck SC2030 and SC2031 warnings Make kdump-logger.sh fully POSIX-compatible Eliminate shellcheck warnings in kdump-lib.sh Eliminate shellcheck warning in spec/kdumpctl_general_spec.sh tests: use guestmount to mount a qemu image when USE_GUESTMOUNT=1 tests: make QEMU instance timeout configurable tests: enable Github action
.github/workflows/main.yml | 53 ++++ dracut-early-kdump-module-setup.sh | 4 +- dracut-early-kdump.sh | 87 ++++--- dracut-fadump-init-fadump.sh | 6 +- dracut-fadump-module-setup.sh | 2 + dracut-kdump.sh | 27 +- dracut-module-setup.sh | 42 ++-- kdump-dep-generator.sh | 6 +- kdump-lib-initramfs.sh | 8 +- kdump-lib.sh | 34 +-- kdump-logger.sh | 388 ++++++++++++++++------------- kdump-migrate-action.sh | 5 +- kdump-restart.sh | 2 +- kdumpctl | 66 ++--- mkdumprd | 1 + mkfadumprd | 2 + spec/kdumpctl_general_spec.sh | 2 + tests/scripts/build-image.sh | 6 + tests/scripts/image-init-lib.sh | 94 +++++-- tests/scripts/test-lib.sh | 10 +- tools/run-integration-tests.sh | 29 +++ 21 files changed, 540 insertions(+), 334 deletions(-) create mode 100644 .github/workflows/main.yml create mode 100755 tools/run-integration-tests.sh
Signed-off-by: Coiby Xu coxu@redhat.com --- dracut-early-kdump.sh | 84 ++++---- dracut-fadump-init-fadump.sh | 2 +- kdump-dep-generator.sh | 6 +- kdump-lib.sh | 7 +- kdump-logger.sh | 378 +++++++++++++++++++---------------- kdump-migrate-action.sh | 2 +- kdump-restart.sh | 2 +- kdumpctl | 66 +++--- 8 files changed, 286 insertions(+), 261 deletions(-)
diff --git a/dracut-early-kdump.sh b/dracut-early-kdump.sh index 45ee6dc..eb2e886 100755 --- a/dracut-early-kdump.sh +++ b/dracut-early-kdump.sh @@ -16,69 +16,69 @@ EARLY_KEXEC_ARGS=""
# initiate the kdump logger if ! dlog_init; then - echo "failed to initiate the kdump logger." - exit 1 + echo "failed to initiate the kdump logger." + exit 1 fi
prepare_parameters() { - EARLY_KDUMP_CMDLINE=$(prepare_cmdline "${KDUMP_COMMANDLINE}" "${KDUMP_COMMANDLINE_REMOVE}" "${KDUMP_COMMANDLINE_APPEND}") - EARLY_KDUMP_KERNEL="/boot/kernel-earlykdump" - EARLY_KDUMP_INITRD="/boot/initramfs-earlykdump" + EARLY_KDUMP_CMDLINE=$(prepare_cmdline "${KDUMP_COMMANDLINE}" "${KDUMP_COMMANDLINE_REMOVE}" "${KDUMP_COMMANDLINE_APPEND}") + EARLY_KDUMP_KERNEL="/boot/kernel-earlykdump" + EARLY_KDUMP_INITRD="/boot/initramfs-earlykdump" }
early_kdump_load() { - if ! check_kdump_feasibility; then - return 1 - fi + if ! check_kdump_feasibility; then + return 1 + fi
- if is_fadump_capable; then - dwarn "WARNING: early kdump doesn't support fadump." - return 1 - fi + if is_fadump_capable; then + dwarn "WARNING: early kdump doesn't support fadump." + return 1 + fi
- if check_current_kdump_status; then - return 1 - fi + if check_current_kdump_status; then + return 1 + fi
- prepare_parameters + prepare_parameters
- EARLY_KEXEC_ARGS=$(prepare_kexec_args "${KEXEC_ARGS}") + EARLY_KEXEC_ARGS=$(prepare_kexec_args "${KEXEC_ARGS}")
- if is_secure_boot_enforced; then - dinfo "Secure Boot is enabled. Using kexec file based syscall." - EARLY_KEXEC_ARGS="$EARLY_KEXEC_ARGS -s" - fi + if is_secure_boot_enforced; then + dinfo "Secure Boot is enabled. Using kexec file based syscall." + EARLY_KEXEC_ARGS="$EARLY_KEXEC_ARGS -s" + fi
- # Here, only output the messages, but do not save these messages - # to a file because the target disk may not be mounted yet, the - # earlykdump is too early. - ddebug "earlykdump: $KEXEC ${EARLY_KEXEC_ARGS} $standard_kexec_args \ + # Here, only output the messages, but do not save these messages + # to a file because the target disk may not be mounted yet, the + # earlykdump is too early. + ddebug "earlykdump: $KEXEC ${EARLY_KEXEC_ARGS} $standard_kexec_args \ --command-line=$EARLY_KDUMP_CMDLINE --initrd=$EARLY_KDUMP_INITRD \ $EARLY_KDUMP_KERNEL"
- if $KEXEC $EARLY_KEXEC_ARGS $standard_kexec_args \ - --command-line="$EARLY_KDUMP_CMDLINE" \ - --initrd=$EARLY_KDUMP_INITRD $EARLY_KDUMP_KERNEL; then - dinfo "kexec: loaded early-kdump kernel" - return 0 - else - derror "kexec: failed to load early-kdump kernel" - return 1 - fi + if $KEXEC $EARLY_KEXEC_ARGS $standard_kexec_args \ + --command-line="$EARLY_KDUMP_CMDLINE" \ + --initrd=$EARLY_KDUMP_INITRD $EARLY_KDUMP_KERNEL; then + dinfo "kexec: loaded early-kdump kernel" + return 0 + else + derror "kexec: failed to load early-kdump kernel" + return 1 + fi }
set_early_kdump() { - if getargbool 0 rd.earlykdump; then - dinfo "early-kdump is enabled." - early_kdump_load - else - dinfo "early-kdump is disabled." - fi - - return 0 + if getargbool 0 rd.earlykdump; then + dinfo "early-kdump is enabled." + early_kdump_load + else + dinfo "early-kdump is disabled." + fi + + return 0 }
set_early_kdump diff --git a/dracut-fadump-init-fadump.sh b/dracut-fadump-init-fadump.sh index 94a3751..3e72c70 100755 --- a/dracut-fadump-init-fadump.sh +++ b/dracut-fadump-init-fadump.sh @@ -37,7 +37,7 @@ if [ -f /proc/device-tree/rtas/ibm,kernel-dump ] || [ -f /proc/device-tree/ibm,o case $mp in /oldroot/*) umount -d "$mp" && loop=1 ;; esac - done </proc/mounts + done < /proc/mounts done umount -d -l oldroot
diff --git a/kdump-dep-generator.sh b/kdump-dep-generator.sh index f48c8f6..0eb8482 100644 --- a/kdump-dep-generator.sh +++ b/kdump-dep-generator.sh @@ -11,13 +11,13 @@ dest_dir="/tmp"
if [ -n "$1" ]; then - dest_dir=$1 + dest_dir=$1 fi
systemd_dir=/usr/lib/systemd/system kdump_wants=$dest_dir/kdump.service.wants
if is_ssh_dump_target; then - mkdir -p $kdump_wants - ln -sf $systemd_dir/network-online.target $kdump_wants/ + mkdir -p $kdump_wants + ln -sf $systemd_dir/network-online.target $kdump_wants/ fi diff --git a/kdump-lib.sh b/kdump-lib.sh index f7b659e..e2fdbd2 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -451,8 +451,7 @@ is_wdt_active()
have_compression_in_dracut_args() { - [[ "$(kdump_get_conf_val dracut_args)" =~ \ - (^|[[:space:]])--(gzip|bzip2|lzma|xz|lzo|lz4|zstd|no-compress|compress)([[:space:]]|$) ]] + [[ "$(kdump_get_conf_val dracut_args)" =~ (^|[[:space:]])--(gzip|bzip2|lzma|xz|lzo|lz4|zstd|no-compress|compress)([[:space:]]|$) ]] }
# If "dracut_args" contains "--mount" information, use it @@ -829,7 +828,7 @@ PROC_IOMEM=/proc/iomem get_system_size() { sum=$(sed -n "s/\s*([0-9a-fA-F]+)-([0-9a-fA-F]+) : System RAM$/+ 0x\2 - 0x\1 + 1/p" $PROC_IOMEM) - echo $(( (sum) / 1024 / 1024 / 1024)) + echo $(((sum) / 1024 / 1024 / 1024)) }
# Return the recommended size for the reserved crashkernel memory @@ -927,7 +926,7 @@ get_luks_crypt_dev()
[[ -b /dev/block/$1 ]] || return 1
- _type=$(blkid -u filesystem,crypto -o export -- "/dev/block/$1" | \ + _type=$(blkid -u filesystem,crypto -o export -- "/dev/block/$1" | sed -n -E "s/^TYPE=(.*)$/\1/p") [[ $_type == "crypto_LUKS" ]] && echo "$1"
diff --git a/kdump-logger.sh b/kdump-logger.sh index 3fd433d..20ac86e 100755 --- a/kdump-logger.sh +++ b/kdump-logger.sh @@ -46,7 +46,7 @@ kdump_kmsgloglvl="" # be used in the first kernel because the dracut-lib.sh is invisible in # the first kernel. if [ -f /lib/dracut-lib.sh ]; then - . /lib/dracut-lib.sh + . /lib/dracut-lib.sh fi
# @brief Get the log level from kernel command line. @@ -55,14 +55,14 @@ fi # get_kdump_loglvl() { - [ -f /lib/dracut-lib.sh ] && kdump_sysloglvl=$(getarg rd.kdumploglvl) - [ -z "$kdump_sysloglvl" ] && return 1; + [ -f /lib/dracut-lib.sh ] && kdump_sysloglvl=$(getarg rd.kdumploglvl) + [ -z "$kdump_sysloglvl" ] && return 1
- if [ -f /lib/dracut-lib.sh ] && ! isdigit "$kdump_sysloglvl"; then - return 1 - fi + if [ -f /lib/dracut-lib.sh ] && ! isdigit "$kdump_sysloglvl"; then + return 1 + fi
- return 0 + return 0 }
# @brief Check the log level. @@ -71,105 +71,121 @@ get_kdump_loglvl() # check_loglvl() { - case "$1" in - 0|1|2|3|4) - return 0 - ;; - *) - return 1 - ;; - esac + case "$1" in + 0 | 1 | 2 | 3 | 4) + return 0 + ;; + *) + return 1 + ;; + esac }
# @brief Initializes Logger. # @retval 1 if something has gone wrong # @retval 0 on success. # -dlog_init() { - ret=0 - - if [ -s /proc/vmcore ];then - if ! get_kdump_loglvl; then - logger -t "kdump[$$]" -p warn -- "Kdump is using the default log level(3)." - kdump_sysloglvl=3 - fi - kdump_stdloglvl=0 - kdump_kmsgloglvl=0 - else - kdump_stdloglvl=$KDUMP_STDLOGLVL - kdump_sysloglvl=$KDUMP_SYSLOGLVL - kdump_kmsgloglvl=$KDUMP_KMSGLOGLVL - fi - - [ -z "$kdump_stdloglvl" ] && kdump_stdloglvl=3 - [ -z "$kdump_sysloglvl" ] && kdump_sysloglvl=0 - [ -z "$kdump_kmsgloglvl" ] && kdump_kmsgloglvl=0 - - for loglvl in "$kdump_stdloglvl" "$kdump_kmsgloglvl" "$kdump_sysloglvl"; do - if ! check_loglvl "$loglvl"; then - echo "Illegal log level: $kdump_stdloglvl $kdump_kmsgloglvl $kdump_sysloglvl" - return 1 - fi - done - - # Skip initialization if it's already done. - [ -n "$kdump_maxloglvl" ] && return 0 - - if [ "$UID" -ne 0 ]; then - kdump_kmsgloglvl=0 - kdump_sysloglvl=0 - fi - - if [ "$kdump_sysloglvl" -gt 0 ]; then - if [ -d /run/systemd/journal ] \ - && systemd-cat --version 1>/dev/null 2>&1 \ - && systemctl --quiet is-active systemd-journald.socket 1>/dev/null 2>&1; then - readonly _systemdcatfile="/var/tmp/systemd-cat" - mkfifo "$_systemdcatfile" 1>/dev/null 2>&1 - readonly _dlogfd=15 - systemd-cat -t 'kdump' --level-prefix=true <"$_systemdcatfile" & - exec 15>"$_systemdcatfile" - elif ! [ -S /dev/log ] && [ -w /dev/log ] || ! command -v logger >/dev/null; then - # We cannot log to syslog, so turn this facility off. - kdump_kmsgloglvl=$kdump_sysloglvl - kdump_sysloglvl=0 - ret=1 - errmsg="No '/dev/log' or 'logger' included for syslog logging" - fi - fi - - kdump_maxloglvl=0 - for _dlog_lvl in $kdump_stdloglvl $kdump_sysloglvl $kdump_kmsgloglvl; do - [ $_dlog_lvl -gt $kdump_maxloglvl ] && kdump_maxloglvl=$_dlog_lvl - done - readonly kdump_maxloglvl - export kdump_maxloglvl - - if [ $kdump_stdloglvl -lt 4 ] && [ $kdump_kmsgloglvl -lt 4 ] && [ $kdump_sysloglvl -lt 4 ]; then - unset ddebug - ddebug() { :; }; - fi - - if [ $kdump_stdloglvl -lt 3 ] && [ $kdump_kmsgloglvl -lt 3 ] && [ $kdump_sysloglvl -lt 3 ]; then - unset dinfo - dinfo() { :; }; - fi - - if [ $kdump_stdloglvl -lt 2 ] && [ $kdump_kmsgloglvl -lt 2 ] && [ $kdump_sysloglvl -lt 2 ]; then - unset dwarn - dwarn() { :; }; - unset dwarning - dwarning() { :; }; - fi - - if [ $kdump_stdloglvl -lt 1 ] && [ $kdump_kmsgloglvl -lt 1 ] && [ $kdump_sysloglvl -lt 1 ]; then - unset derror - derror() { :; }; - fi - - [ -n "$errmsg" ] && derror "$errmsg" - - return $ret +dlog_init() +{ + ret=0 + + if [ -s /proc/vmcore ]; then + if ! get_kdump_loglvl; then + logger -t "kdump[$$]" -p warn -- "Kdump is using the default log level(3)." + kdump_sysloglvl=3 + fi + kdump_stdloglvl=0 + kdump_kmsgloglvl=0 + else + kdump_stdloglvl=$KDUMP_STDLOGLVL + kdump_sysloglvl=$KDUMP_SYSLOGLVL + kdump_kmsgloglvl=$KDUMP_KMSGLOGLVL + fi + + [ -z "$kdump_stdloglvl" ] && kdump_stdloglvl=3 + [ -z "$kdump_sysloglvl" ] && kdump_sysloglvl=0 + [ -z "$kdump_kmsgloglvl" ] && kdump_kmsgloglvl=0 + + for loglvl in "$kdump_stdloglvl" "$kdump_kmsgloglvl" "$kdump_sysloglvl"; do + if ! check_loglvl "$loglvl"; then + echo "Illegal log level: $kdump_stdloglvl $kdump_kmsgloglvl $kdump_sysloglvl" + return 1 + fi + done + + # Skip initialization if it's already done. + [ -n "$kdump_maxloglvl" ] && return 0 + + if [ "$UID" -ne 0 ]; then + kdump_kmsgloglvl=0 + kdump_sysloglvl=0 + fi + + if [ "$kdump_sysloglvl" -gt 0 ]; then + if [ -d /run/systemd/journal ] && + systemd-cat --version 1> /dev/null 2>&1 && + systemctl --quiet is-active systemd-journald.socket 1> /dev/null 2>&1; then + readonly _systemdcatfile="/var/tmp/systemd-cat" + mkfifo "$_systemdcatfile" 1> /dev/null 2>&1 + readonly _dlogfd=15 + systemd-cat -t 'kdump' --level-prefix=true < "$_systemdcatfile" & + exec 15> "$_systemdcatfile" + elif ! [ -S /dev/log ] && [ -w /dev/log ] || ! command -v logger > /dev/null; then + # We cannot log to syslog, so turn this facility off. + kdump_kmsgloglvl=$kdump_sysloglvl + kdump_sysloglvl=0 + ret=1 + errmsg="No '/dev/log' or 'logger' included for syslog logging" + fi + fi + + kdump_maxloglvl=0 + for _dlog_lvl in $kdump_stdloglvl $kdump_sysloglvl $kdump_kmsgloglvl; do + [ $_dlog_lvl -gt $kdump_maxloglvl ] && kdump_maxloglvl=$_dlog_lvl + done + readonly kdump_maxloglvl + export kdump_maxloglvl + + if [ $kdump_stdloglvl -lt 4 ] && [ $kdump_kmsgloglvl -lt 4 ] && [ $kdump_sysloglvl -lt 4 ]; then + unset ddebug + ddebug() + { + : + } + fi + + if [ $kdump_stdloglvl -lt 3 ] && [ $kdump_kmsgloglvl -lt 3 ] && [ $kdump_sysloglvl -lt 3 ]; then + unset dinfo + dinfo() + { + : + } + fi + + if [ $kdump_stdloglvl -lt 2 ] && [ $kdump_kmsgloglvl -lt 2 ] && [ $kdump_sysloglvl -lt 2 ]; then + unset dwarn + dwarn() + { + : + } + unset dwarning + dwarning() + { + : + } + fi + + if [ $kdump_stdloglvl -lt 1 ] && [ $kdump_kmsgloglvl -lt 1 ] && [ $kdump_sysloglvl -lt 1 ]; then + unset derror + derror() + { + : + } + fi + + [ -n "$errmsg" ] && derror "$errmsg" + + return $ret }
## @brief Converts numeric level to logger priority defined by POSIX.2. @@ -178,14 +194,15 @@ dlog_init() { # @retval 1 if @a lvl is out of range. # @retval 0 if @a lvl is correct. # @result Echoes logger priority. -_lvl2syspri() { - case "$1" in - 1) echo error;; - 2) echo warning;; - 3) echo info;; - 4) echo debug;; - *) return 1;; - esac +_lvl2syspri() +{ + case "$1" in + 1) echo error ;; + 2) echo warning ;; + 3) echo info ;; + 4) echo debug ;; + *) return 1 ;; + esac }
## @brief Converts logger numeric level to syslog log level @@ -209,19 +226,20 @@ _lvl2syspri() { # </tt> # # @see /usr/include/sys/syslog.h -_dlvl2syslvl() { - case "$1" in - 1) set -- 3;; - 2) set -- 4;; - 3) set -- 6;; - 4) set -- 7;; - *) return 1;; - esac - - # The number is constructed by multiplying the facility by 8 and then - # adding the level. - # About The Syslog Protocol, please refer to the RFC5424 for more details. - echo $((24 + $1)) +_dlvl2syslvl() +{ + case "$1" in + 1) set -- 3 ;; + 2) set -- 4 ;; + 3) set -- 6 ;; + 4) set -- 7 ;; + *) return 1 ;; + esac + + # The number is constructed by multiplying the facility by 8 and then + # adding the level. + # About The Syslog Protocol, please refer to the RFC5424 for more details. + echo $((24 + $1)) }
## @brief Prints to stderr, to syslog and/or /dev/kmsg given message with @@ -249,19 +267,20 @@ _dlvl2syslvl() { # - @c WARN to @c warning # - @c INFO to @c info # - @c DEBUG to @c debug -_do_dlog() { - [ "$1" -le $kdump_stdloglvl ] && printf -- 'kdump: %s\n' "$2" >&2 - - if [ "$1" -le $kdump_sysloglvl ]; then - if [ "$_dlogfd" ]; then - printf -- "<%s>%s\n" "$(($(_dlvl2syslvl "$1") & 7))" "$2" 1>&$_dlogfd - else - logger -t "kdump[$$]" -p "$(_lvl2syspri "$1")" -- "$2" - fi - fi - - [ "$1" -le $kdump_kmsgloglvl ] && \ - echo "<$(_dlvl2syslvl "$1")>kdump[$$] $2" >/dev/kmsg +_do_dlog() +{ + [ "$1" -le $kdump_stdloglvl ] && printf -- 'kdump: %s\n' "$2" >&2 + + if [ "$1" -le $kdump_sysloglvl ]; then + if [ "$_dlogfd" ]; then + printf -- "<%s>%s\n" "$(($(_dlvl2syslvl "$1") & 7))" "$2" 1>&$_dlogfd + else + logger -t "kdump[$$]" -p "$(_lvl2syspri "$1")" -- "$2" + fi + fi + + [ "$1" -le $kdump_kmsgloglvl ] && + echo "<$(_dlvl2syslvl "$1")>kdump[$$] $2" > /dev/kmsg }
## @brief Internal helper function for _do_dlog() @@ -280,76 +299,83 @@ _do_dlog() { # This enables: # dwarn "This is a warning" # echo "This is a warning" | dwarn -dlog() { - [ -z "$kdump_maxloglvl" ] && return 0 - [ "$1" -le "$kdump_maxloglvl" ] || return 0 - - if [ $# -gt 1 ]; then - _dlog_lvl=$1; shift - _do_dlog "$_dlog_lvl" "$*" - else - while read -r line || [ -n "$line" ]; do - _do_dlog "$1" "$line" - done - fi +dlog() +{ + [ -z "$kdump_maxloglvl" ] && return 0 + [ "$1" -le "$kdump_maxloglvl" ] || return 0 + + if [ $# -gt 1 ]; then + _dlog_lvl=$1 + shift + _do_dlog "$_dlog_lvl" "$*" + else + while read -r line || [ -n "$line" ]; do + _do_dlog "$1" "$line" + done + fi }
## @brief Logs message at DEBUG level (4) # # @param msg Message. # @retval 0 It's always returned, even if logging failed. -ddebug() { - set +x - dlog 4 "$@" - if [ -n "$debug" ]; then - set -x - fi +ddebug() +{ + set +x + dlog 4 "$@" + if [ -n "$debug" ]; then + set -x + fi }
## @brief Logs message at INFO level (3) # # @param msg Message. # @retval 0 It's always returned, even if logging failed. -dinfo() { - set +x - dlog 3 "$@" - if [ -n "$debug" ]; then - set -x - fi +dinfo() +{ + set +x + dlog 3 "$@" + if [ -n "$debug" ]; then + set -x + fi }
## @brief Logs message at WARN level (2) # # @param msg Message. # @retval 0 It's always returned, even if logging failed. -dwarn() { - set +x - dlog 2 "$@" - if [ -n "$debug" ]; then - set -x - fi +dwarn() +{ + set +x + dlog 2 "$@" + if [ -n "$debug" ]; then + set -x + fi }
## @brief It's an alias to dwarn() function. # # @param msg Message. # @retval 0 It's always returned, even if logging failed. -dwarning() { - set +x - dwarn "$@" - if [ -n "$debug" ]; then - set -x - fi +dwarning() +{ + set +x + dwarn "$@" + if [ -n "$debug" ]; then + set -x + fi }
## @brief Logs message at ERROR level (1) # # @param msg Message. # @retval 0 It's always returned, even if logging failed. -derror() { - set +x - dlog 1 "$@" - if [ -n "$debug" ]; then - set -x - fi +derror() +{ + set +x + dlog 1 "$@" + if [ -n "$debug" ]; then + set -x + fi } diff --git a/kdump-migrate-action.sh b/kdump-migrate-action.sh index c516639..2fb8366 100755 --- a/kdump-migrate-action.sh +++ b/kdump-migrate-action.sh @@ -2,7 +2,7 @@
systemctl is-active kdump if [ $? -ne 0 ]; then - exit 0 + exit 0 fi
/usr/lib/kdump/kdump-restart.sh diff --git a/kdump-restart.sh b/kdump-restart.sh index a9ecfc1..2adc20b 100644 --- a/kdump-restart.sh +++ b/kdump-restart.sh @@ -1,7 +1,7 @@ #!/bin/bash export PATH="$PATH:/usr/bin:/usr/sbin"
-exec >>/var/log/kdump-migration.log 2>&1 +exec >> /var/log/kdump-migration.log 2>&1
echo "kdump: Partition Migration detected. Rebuilding initramfs image to reload." /usr/bin/kdumpctl rebuild diff --git a/kdumpctl b/kdumpctl index 126ecb9..49d834a 100755 --- a/kdumpctl +++ b/kdumpctl @@ -720,7 +720,7 @@ check_ssh_config()
[[ "${OPT[_fstype]}" == ssh ]] || return 0
- target=$(ssh -G "${OPT[_target]}" | sed -n -e "s/^hostname[[:space:]]+([^[:space:]]*).*$/\1/p") + target=$(ssh -G "${OPT[_target]}" | sed -n -e "s/^hostname[[:space:]]+([^[:space:]]*).*$/\1/p") [[ ${OPT[_target]} =~ .*@.* ]] || return 1 if [[ ${OPT[_target]#*@} != "$target" ]]; then derror "Invalid ssh destination ${OPT[_target]} provided." @@ -788,7 +788,7 @@ propagate_ssh_key()
parse_config || return 1
- if [[ ${OPT[_fstype]} != ssh ]] ; then + if [[ ${OPT[_fstype]} != ssh ]]; then derror "No ssh destination defined in $KDUMP_CONFIG_FILE." derror "Please verify that $KDUMP_CONFIG_FILE contains 'ssh <user>@<host>' and that it is properly formatted." exit 1 @@ -1471,30 +1471,30 @@ reset_crashkernel()
for _opt in "$@"; do case "$_opt" in - --fadump=*) - _val=${_opt#*=} - if _dump_mode=$(get_dump_mode_by_fadump_val $_val); then - _fadump_val=$_val - else - derror "failed to determine dump mode" - exit - fi - ;; - --kernel=*) - _val=${_opt#*=} - if ! _valid_grubby_kernel_path $_val; then - derror "Invalid $_opt, please specify a valid kernel path, ALL or DEFAULT" - exit - fi - _grubby_kernel_path=$_val - ;; - --reboot) - _reboot=yes - ;; - *) - derror "$_opt not recognized" - exit 1 - ;; + --fadump=*) + _val=${_opt#*=} + if _dump_mode=$(get_dump_mode_by_fadump_val $_val); then + _fadump_val=$_val + else + derror "failed to determine dump mode" + exit + fi + ;; + --kernel=*) + _val=${_opt#*=} + if ! _valid_grubby_kernel_path $_val; then + derror "Invalid $_opt, please specify a valid kernel path, ALL or DEFAULT" + exit + fi + _grubby_kernel_path=$_val + ;; + --reboot) + _reboot=yes + ;; + *) + derror "$_opt not recognized" + exit 1 + ;; esac done
@@ -1553,10 +1553,10 @@ reset_crashkernel() if [[ -z $_grubby_kernel_path ]]; then if [[ -z $KDUMP_KERNELVER ]] || ! _kernel_path=$(_find_kernel_path_by_release "$KDUMP_KERNELVER"); then - if ! _kernel_path=$(_get_current_running_kernel_path); then - derror "no running kernel found" - exit 1 - fi + if ! _kernel_path=$(_get_current_running_kernel_path); then + derror "no running kernel found" + exit 1 + fi fi _kernels=$_kernel_path else @@ -1618,9 +1618,9 @@ update_crashkernel_in_grub_etc_default_after_update() _new_default_crashkernel=${_crashkernel_vals[new_${_dump_mode}]}
if [[ $_crashkernel == auto ]] || - [[ $_crashkernel == "$_old_default_crashkernel" && - $_new_default_crashkernel != "$_old_default_crashkernel" ]]; then - _update_kernel_arg_in_grub_etc_default crashkernel "$_new_default_crashkernel" + [[ $_crashkernel == "$_old_default_crashkernel" && + $_new_default_crashkernel != "$_old_default_crashkernel" ]]; then + _update_kernel_arg_in_grub_etc_default crashkernel "$_new_default_crashkernel" fi }
Signed-off-by: Coiby Xu coxu@redhat.com --- dracut-early-kdump-module-setup.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/dracut-early-kdump-module-setup.sh b/dracut-early-kdump-module-setup.sh index 0451118..c03bdc8 100755 --- a/dracut-early-kdump-module-setup.sh +++ b/dracut-early-kdump-module-setup.sh @@ -23,7 +23,7 @@ prepare_kernel_initrd() {
prepare_kdump_bootinfo
- # $kernel is a variable from dracut + # shellcheck disable=SC2154 # $kernel is a variable from dracut if [[ $KDUMP_KERNELVER != "$kernel" ]]; then dwarn "Using kernel version '$KDUMP_KERNELVER' for early kdump," \ "but the initramfs is generated for kernel version '$kernel'" @@ -54,6 +54,7 @@ install() { inst_script "/lib/kdump/kdump-lib.sh" "/lib/kdump-lib.sh" inst_script "/lib/kdump/kdump-lib-initramfs.sh" "/lib/kdump/kdump-lib-initramfs.sh" inst_script "/lib/kdump/kdump-logger.sh" "/lib/kdump-logger.sh" + # shellcheck disable=SC2154 # $moddir is a variable from dracut inst_hook cmdline 00 "$moddir/early-kdump.sh" inst_binary "$KDUMP_KERNEL" inst_binary "$KDUMP_INITRD" @@ -61,5 +62,6 @@ install() { ln_r "$KDUMP_KERNEL" "/boot/kernel-earlykdump" ln_r "$KDUMP_INITRD" "/boot/initramfs-earlykdump"
+ # shellcheck disable=SC2154 # $initdir is a variable from dracut chmod -x "${initdir}/$KDUMP_KERNEL" }
As for EARLY_KDUMP_KERNELVER, it is no longer used so remove it.
Fixes: 8a476da ("earlykdump: generate symlink with stable name to kernel image and iniramfs") Signed-off-by: Coiby Xu coxu@redhat.com --- dracut-early-kdump.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/dracut-early-kdump.sh b/dracut-early-kdump.sh index eb2e886..f6db3ac 100755 --- a/dracut-early-kdump.sh +++ b/dracut-early-kdump.sh @@ -6,7 +6,6 @@ standard_kexec_args="-p" EARLY_KDUMP_INITRD="" EARLY_KDUMP_KERNEL="" EARLY_KDUMP_CMDLINE="" -EARLY_KDUMP_KERNELVER="" EARLY_KEXEC_ARGS=""
. /etc/sysconfig/kdump @@ -57,7 +56,7 @@ early_kdump_load() ddebug "earlykdump: $KEXEC ${EARLY_KEXEC_ARGS} $standard_kexec_args \ --command-line=$EARLY_KDUMP_CMDLINE --initrd=$EARLY_KDUMP_INITRD \ $EARLY_KDUMP_KERNEL" - + # shellcheck disable=SC2086 # $EARLY_KEXEC_ARGS depends on word splitting if $KEXEC $EARLY_KEXEC_ARGS $standard_kexec_args \ --command-line="$EARLY_KDUMP_CMDLINE" \ --initrd=$EARLY_KDUMP_INITRD $EARLY_KDUMP_KERNEL; then
As detected by shellcheck, In dracut-fadump-init-fadump.sh line 18: for FILE in $(ls -A /fadumproot/); do ^-------------------^ SC2045: Iterating over ls output is fragile. Use globs.
In dracut-fadump-init-fadump.sh line 19: mv /fadumproot/$FILE /newroot/ ^---^ SC2086: Double quote to prevent globbing and word splitting.
"ls -A" fails to address the case where a file name has spaces. Use "find -exec mv" to fix SC2045 and SC2086 as well.
Signed-off-by: Coiby Xu coxu@redhat.com --- dracut-fadump-init-fadump.sh | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/dracut-fadump-init-fadump.sh b/dracut-fadump-init-fadump.sh index 3e72c70..ba50595 100755 --- a/dracut-fadump-init-fadump.sh +++ b/dracut-fadump-init-fadump.sh @@ -15,9 +15,7 @@ if [ -f /proc/device-tree/rtas/ibm,kernel-dump ] || [ -f /proc/device-tree/ibm,o mount -t ramfs ramfs /newroot
if [ $ROOTFS_IS_RAMFS ]; then - for FILE in $(ls -A /fadumproot/); do - mv /fadumproot/$FILE /newroot/ - done + find /fadumproot/ -mindepth 1 -maxdepth 1 -exec mv {} /newroot/ ; exec switch_root /newroot /init else mkdir /newroot/sys /newroot/proc /newroot/dev /newroot/run /newroot/oldroot
uname initdir and etc. are from dracut. In these cases, SC2154 warnings are false positive for a dracut module.
Signed-off-by: Coiby Xu coxu@redhat.com --- dracut-fadump-module-setup.sh | 2 ++ dracut-module-setup.sh | 8 +++++++- 2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/dracut-fadump-module-setup.sh b/dracut-fadump-module-setup.sh index f062486..8afbfec 100644 --- a/dracut-fadump-module-setup.sh +++ b/dracut-fadump-module-setup.sh @@ -9,7 +9,9 @@ depends() { }
install() { + # shellcheck disable=SC2154 # $initdir is a dracut variable mv -f "$initdir/init" "$initdir/init.dracut" + # shellcheck disable=SC2154 # $moddir is a dracut variable inst_script "$moddir/init-fadump.sh" /init chmod a+x "$initdir/init"
diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh index 4c6096a..21c0e70 100755 --- a/dracut-module-setup.sh +++ b/dracut-module-setup.sh @@ -1,6 +1,7 @@ #!/bin/bash
kdump_module_init() { + # shellcheck disable=SC2154 # $initdir is a dracut variable if ! [[ -d "${initdir}/tmp" ]]; then mkdir -p "${initdir}/tmp" fi @@ -9,6 +10,7 @@ kdump_module_init() { }
check() { + # shellcheck disable=SC2154 # $debug is a dracut variable [[ $debug ]] && set -x #kdumpctl sets this explicitly if [[ -z $IN_KDUMP ]] || [[ ! -f /etc/kdump.conf ]]; then @@ -23,6 +25,7 @@ depends() { kdump_module_init
add_opt_module() { + # shellcheck disable=SC2154 # $omit_dracutmodules is a dracut variable [[ " $omit_dracutmodules " != *\ $1\ * ]] && _dep="$_dep $1" }
@@ -851,7 +854,7 @@ kdump_check_iscsi_targets() { done [[ -d iscsi_session ]] && kdump_setup_iscsi_device "$PWD" ) - + # shellcheck disable=SC2154 # $hostonly and $mount_needs are from dracut [[ $hostonly ]] || [[ $mount_needs ]] && { for_each_host_dev_and_slaves_all kdump_check_setup_iscsi } @@ -906,6 +909,7 @@ get_pcs_fence_kdump_nodes() { for node in ${nodelist}; do # convert $node from 'uname="nodeX"' to 'nodeX' eval "$node" + # shellcheck disable=SC2154 # $uname from dracut nodename="$uname" # Skip its own node name if is_localhost "$nodename"; then @@ -1039,6 +1043,7 @@ install() { kdump_install_random_seed fi dracut_install -o /etc/adjtime /etc/localtime + # shellcheck disable=SC2154 # $uname and ${initdir} from dracut inst "$moddir/monitor_dd_progress" "/kdumpscripts/monitor_dd_progress" chmod +x "${initdir}/kdumpscripts/monitor_dd_progress" inst "/bin/dd" "/bin/dd" @@ -1058,6 +1063,7 @@ install() { inst "/lib/kdump/kdump-lib-initramfs.sh" "/lib/kdump-lib-initramfs.sh" inst "/lib/kdump/kdump-logger.sh" "/lib/kdump-logger.sh" inst "$moddir/kdump.sh" "/usr/bin/kdump.sh" + # shellcheck disable=SC2154 # $systemdsystemunitdir from dracut inst "$moddir/kdump-capture.service" "$systemdsystemunitdir/kdump-capture.service" systemctl -q --root "$initdir" add-wants initrd.target kdump-capture.service # Replace existing emergency service and emergency target
Quoting the rationale of SC2129,
Rather than adding >> something after every single line, you can simply group the relevant commands and redirect the group. So the file has to be opened and closed only once and it means a performance gain.
Signed-off-by: Coiby Xu coxu@redhat.com --- dracut-module-setup.sh | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh index 21c0e70..b478e22 100755 --- a/dracut-module-setup.sh +++ b/dracut-module-setup.sh @@ -1008,10 +1008,12 @@ kdump_install_systemd_conf() { # unneccessary memory consumption and make console output more useful. # Only do so for non fadump image. mkdir -p "${initdir}/etc/systemd/journald.conf.d" - echo "[Journal]" > "${initdir}/etc/systemd/journald.conf.d/kdump.conf" - echo "Storage=volatile" >> "${initdir}/etc/systemd/journald.conf.d/kdump.conf" - echo "ReadKMsg=no" >> "${initdir}/etc/systemd/journald.conf.d/kdump.conf" - echo "ForwardToConsole=yes" >> "${initdir}/etc/systemd/journald.conf.d/kdump.conf" + { + echo "[Journal]" + echo "Storage=volatile" + echo "ReadKMsg=no" + echo "ForwardToConsole=yes" + } > "${initdir}/etc/systemd/journald.conf.d/kdump.conf" }
remove_cpu_online_rule() {
For the cases like passing ssh opts, we need to word-split ssh_opts (renamed from rename _ssh_opt to improve readability). So they are false positives. For other cases, quote them to avoid prevent globbing and word splitting.
Signed-off-by: Coiby Xu coxu@redhat.com --- dracut-kdump.sh | 27 +++++++++++++++++---------- kdump-dep-generator.sh | 4 ++-- kdump-lib-initramfs.sh | 5 +++-- 3 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/dracut-kdump.sh b/dracut-kdump.sh index f4456a1..148cad7 100755 --- a/dracut-kdump.sh +++ b/dracut-kdump.sh @@ -301,12 +301,12 @@ do_failure_action() do_final_action() { dinfo "Executing final action $FINAL_ACTION" - eval $FINAL_ACTION + eval "$FINAL_ACTION" }
do_dump() { - eval $DUMP_INSTRUCTION + eval "$DUMP_INSTRUCTION" _ret=$?
if [ $_ret -ne 0 ]; then @@ -387,7 +387,7 @@ dump_raw() dump_ssh() { _ret=0 - _ssh_opt="-i $1 -o BatchMode=yes -o StrictHostKeyChecking=yes" + _ssh_opts="-i $1 -o BatchMode=yes -o StrictHostKeyChecking=yes" _ssh_dir="$KDUMP_PATH/$HOST_IP-$DATEDIR" if is_ipv6_address "$2"; then _scp_address=${2%@*}@"[${2#*@}]" @@ -398,29 +398,33 @@ dump_ssh() dinfo "saving to $2:$_ssh_dir"
cat /var/lib/random-seed > /dev/urandom - ssh -q $_ssh_opt "$2" mkdir -p "$_ssh_dir" || return 1 + # shellcheck disable=SC2086 # need to word-split $_ssh_opts + ssh -q $_ssh_opts "$2" mkdir -p "$_ssh_dir" || return 1
- save_vmcore_dmesg_ssh "$DMESG_COLLECTOR" "$_ssh_dir" "$_ssh_opt" "$2" + save_vmcore_dmesg_ssh "$DMESG_COLLECTOR" "$_ssh_dir" "$_ssh_opts" "$2"
dinfo "saving vmcore"
KDUMP_LOG_DEST=$2:$_ssh_dir/ - KDUMP_LOG_OP="scp -q $_ssh_opt '$KDUMP_LOG_FILE' '$_scp_address:$_ssh_dir/'" + KDUMP_LOG_OP="scp -q $_ssh_opts '$KDUMP_LOG_FILE' '$_scp_address:$_ssh_dir/'"
- save_opalcore_ssh "$_ssh_dir" "$_ssh_opt" "$2" "$_scp_address" + save_opalcore_ssh "$_ssh_dir" "$_ssh_opts" "$2" "$_scp_address"
if [ "${CORE_COLLECTOR%%[[:blank:]]*}" = "scp" ]; then - scp -q $_ssh_opt /proc/vmcore "$_scp_address:$_ssh_dir/vmcore-incomplete" + # shellcheck disable=SC2086 # need to word-split $_ssh_opts + scp -q $_ssh_opts /proc/vmcore "$_scp_address:$_ssh_dir/vmcore-incomplete" _ret=$? _vmcore="vmcore" else - $CORE_COLLECTOR /proc/vmcore | ssh $_ssh_opt "$2" "umask 0077 && dd bs=512 of='$_ssh_dir/vmcore-incomplete'" + # shellcheck disable=SC2086 # need to word-split $_ssh_opts + $CORE_COLLECTOR /proc/vmcore | ssh $_ssh_opts "$2" "umask 0077 && dd bs=512 of='$_ssh_dir/vmcore-incomplete'" _ret=$? _vmcore="vmcore.flat" fi
if [ $_ret -eq 0 ]; then - ssh $_ssh_opt "$2" "mv '$_ssh_dir/vmcore-incomplete' '$_ssh_dir/$_vmcore'" + # shellcheck disable=SC2086 # need to word-split $_ssh_opts + ssh $_ssh_opts "$2" "mv '$_ssh_dir/vmcore-incomplete' '$_ssh_dir/$_vmcore'" _ret=$? if [ $_ret -ne 0 ]; then derror "moving vmcore failed, exitcode:$_ret" @@ -451,11 +455,13 @@ save_opalcore_ssh()
dinfo "saving opalcore:$OPALCORE to $3:$1"
+ # shellcheck disable=SC2086 # need to word-split $2 if ! scp $2 $OPALCORE "$4:$1/opalcore-incomplete"; then derror "saving opalcore failed" return 1 fi
+ # shellcheck disable=SC2086 # need to word-split $2 ssh $2 "$3" mv "$1/opalcore-incomplete" "$1/opalcore" dinfo "saving opalcore complete" return 0 @@ -468,6 +474,7 @@ save_opalcore_ssh() save_vmcore_dmesg_ssh() { dinfo "saving vmcore-dmesg.txt to $4:$2" + # shellcheck disable=SC2086 # need to word-split $3 if $1 /proc/vmcore | ssh $3 "$4" "umask 0077 && dd of='$2/vmcore-dmesg-incomplete.txt'"; then ssh -q $3 "$4" mv "$2/vmcore-dmesg-incomplete.txt" "$2/vmcore-dmesg.txt" dinfo "saving vmcore-dmesg.txt complete" diff --git a/kdump-dep-generator.sh b/kdump-dep-generator.sh index 0eb8482..655e7bf 100644 --- a/kdump-dep-generator.sh +++ b/kdump-dep-generator.sh @@ -18,6 +18,6 @@ systemd_dir=/usr/lib/systemd/system kdump_wants=$dest_dir/kdump.service.wants
if is_ssh_dump_target; then - mkdir -p $kdump_wants - ln -sf $systemd_dir/network-online.target $kdump_wants/ + mkdir -p "$kdump_wants" + ln -sf $systemd_dir/network-online.target "$kdump_wants"/ fi diff --git a/kdump-lib-initramfs.sh b/kdump-lib-initramfs.sh index 84e6bf7..5c052a2 100755 --- a/kdump-lib-initramfs.sh +++ b/kdump-lib-initramfs.sh @@ -36,6 +36,7 @@ is_mounted() # $2: mount source type # $3: mount source # $4: extra args +# shellcheck disable=SC2086 # $4 means extra args which nees to word-splitted get_mount_info() { __kdump_mnt=$(findmnt -k -n -r -o "$1" "--$2" "$3" $4) @@ -58,13 +59,13 @@ is_fs_type_nfs() # If $1 contains dracut_args "--mount", return <filesystem type> get_dracut_args_fstype() { - echo $1 | grep "--mount" | sed "s/.*--mount .(.*)/\1/" | cut -d' ' -f3 + echo "$1" | grep "--mount" | sed "s/.*--mount .(.*)/\1/" | cut -d' ' -f3 }
# If $1 contains dracut_args "--mount", return <device> get_dracut_args_target() { - echo $1 | grep "--mount" | sed "s/.*--mount .(.*)/\1/" | cut -d' ' -f1 + echo "$1" | grep "--mount" | sed "s/.*--mount .(.*)/\1/" | cut -d' ' -f1 }
get_save_path()
For those cases, we need to expand the variable so they are false positives.
Signed-off-by: Coiby Xu coxu@redhat.com --- dracut-kdump.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/dracut-kdump.sh b/dracut-kdump.sh index 148cad7..259a010 100755 --- a/dracut-kdump.sh +++ b/dracut-kdump.sh @@ -416,14 +416,14 @@ dump_ssh() _ret=$? _vmcore="vmcore" else - # shellcheck disable=SC2086 # need to word-split $_ssh_opts + # shellcheck disable=SC2086,SC2029 # need to word-split $_ssh_opts and expand $_ssh_dir $CORE_COLLECTOR /proc/vmcore | ssh $_ssh_opts "$2" "umask 0077 && dd bs=512 of='$_ssh_dir/vmcore-incomplete'" _ret=$? _vmcore="vmcore.flat" fi
if [ $_ret -eq 0 ]; then - # shellcheck disable=SC2086 # need to word-split $_ssh_opts + # shellcheck disable=SC2086,SC2029 # need to word-split $_ssh_opts and expand $_ssh_dir ssh $_ssh_opts "$2" "mv '$_ssh_dir/vmcore-incomplete' '$_ssh_dir/$_vmcore'" _ret=$? if [ $_ret -ne 0 ]; then @@ -461,7 +461,7 @@ save_opalcore_ssh() return 1 fi
- # shellcheck disable=SC2086 # need to word-split $2 + # shellcheck disable=SC2086,SC2029 # need to word-split $1 and expand $2 ssh $2 "$3" mv "$1/opalcore-incomplete" "$1/opalcore" dinfo "saving opalcore complete" return 0 @@ -474,7 +474,7 @@ save_opalcore_ssh() save_vmcore_dmesg_ssh() { dinfo "saving vmcore-dmesg.txt to $4:$2" - # shellcheck disable=SC2086 # need to word-split $3 + # shellcheck disable=SC2086,SC2029 # need to word-split $3 and expand $2 if $1 /proc/vmcore | ssh $3 "$4" "umask 0077 && dd of='$2/vmcore-dmesg-incomplete.txt'"; then ssh -q $3 "$4" mv "$2/vmcore-dmesg-incomplete.txt" "$2/vmcore-dmesg.txt" dinfo "saving vmcore-dmesg.txt complete"
DEFAULT_SSHKEY, and FENCE_KDUMP_SEND are used by whoever sources kdump-lib-initramfs.sh and KDUMP_INITRD is used by kdumpctl. So they are false positives.
FENCE_KDUMP_CONFIG_FILE is only used by kdumpctl and dracut-module-setup.sh both of which source kdump-lib.sh so define FENCE_KDUMP_CONFIG_FILE in kdump-lib.sh instead.
dracut_args is unused.
Signed-off-by: Coiby Xu coxu@redhat.com --- kdump-lib-initramfs.sh | 3 ++- kdump-lib.sh | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/kdump-lib-initramfs.sh b/kdump-lib-initramfs.sh index 5c052a2..2d37156 100755 --- a/kdump-lib-initramfs.sh +++ b/kdump-lib-initramfs.sh @@ -4,9 +4,10 @@ # not be the default shell. Any code added must be POSIX compliant.
DEFAULT_PATH="/var/crash/" +# shellcheck disable=SC2034 DEFAULT_SSHKEY="/root/.ssh/kdump_id_rsa" KDUMP_CONFIG_FILE="/etc/kdump.conf" -FENCE_KDUMP_CONFIG_FILE="/etc/sysconfig/fence_kdump" +# shellcheck disable=SC2034 FENCE_KDUMP_SEND="/usr/libexec/fence_kdump_send"
# Read kdump config in well formated style diff --git a/kdump-lib.sh b/kdump-lib.sh index e2fdbd2..ec71d77 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -8,6 +8,8 @@ else . /lib/kdump/kdump-lib-initramfs.sh fi
+# shellcheck disable=SC2034 +FENCE_KDUMP_CONFIG_FILE="/etc/sysconfig/fence_kdump" FADUMP_ENABLED_SYS_NODE="/sys/kernel/fadump_enabled"
is_fadump_capable() @@ -421,7 +423,7 @@ get_ifcfg_filename() # returns 1 otherwise is_dracut_mod_omitted() { - local dracut_args dracut_mod=$1 + local dracut_mod=$1
set -- $(kdump_get_conf_val dracut_args) while [ $# -gt 0 ]; do @@ -736,8 +738,10 @@ prepare_kdump_bootinfo() if [[ ! -w $KDUMP_BOOTDIR ]]; then var_target_initrd_dir="/var/lib/kdump" mkdir -p "$var_target_initrd_dir" + # shellcheck disable=SC2034 # KDUMP_INITRD is used by kdumpctl KDUMP_INITRD="$var_target_initrd_dir/$kdump_initrd_base" else + # shellcheck disable=SC2034 # KDUMP_INITRD is used by kdumpctl KDUMP_INITRD="$KDUMP_BOOTDIR/$kdump_initrd_base" fi }
Suppress SC2181 warning from kdump-lib.sh for the sake of readability.
For the other warning from kdump-migrate-action.sh, fix it following shellcheck's advice, In kdump-migrate-action.sh line 4: if [ $? -ne 0 ]; then ^-- SC2181 (style): Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?.
Signed-off-by: Coiby Xu coxu@redhat.com --- kdump-lib.sh | 1 + kdump-migrate-action.sh | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kdump-lib.sh b/kdump-lib.sh index ec71d77..43dbad5 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -1024,6 +1024,7 @@ get_kernel_size() try_decompress '(\265/\375' xxx unzstd "$img" "$tmp"
# Finally check for uncompressed images or objects: + # shellcheck disable=SC2181 # to improve readability [[ $? -eq 0 ]] && get_vmlinux_size "$tmp" && return 0
# Fallback to use iomem diff --git a/kdump-migrate-action.sh b/kdump-migrate-action.sh index 2fb8366..65e0757 100755 --- a/kdump-migrate-action.sh +++ b/kdump-migrate-action.sh @@ -1,7 +1,6 @@ #!/bin/sh
-systemctl is-active kdump -if [ $? -ne 0 ]; then +if ! systemctl is-active kdump; then exit 0 fi
It's a known issue [1] that shellcheck fails to detect a variables assigned and referenced in a trap.
[1] https://github.com/koalaman/shellcheck/issues/1299
Signed-off-by: Coiby Xu coxu@redhat.com --- mkdumprd | 1 + mkfadumprd | 2 ++ 2 files changed, 3 insertions(+)
diff --git a/mkdumprd b/mkdumprd index 3e250e0..bcbf47b 100644 --- a/mkdumprd +++ b/mkdumprd @@ -33,6 +33,7 @@ MKDUMPRD_TMPDIR="$(mktemp -d -t mkdumprd.XXXXXX)" [ -d "$MKDUMPRD_TMPDIR" ] || perror_exit "dracut: mktemp -p -d -t dracut.XXXXXX failed." MKDUMPRD_TMPMNT="$MKDUMPRD_TMPDIR/target"
+# shellcheck disable=SC2154 # known issue of shellcheck https://github.com/koalaman/shellcheck/issues/1299 trap ' ret=$?; is_mounted $MKDUMPRD_TMPMNT && umount -f $MKDUMPRD_TMPMNT; diff --git a/mkfadumprd b/mkfadumprd index 86dfcee..b67ee8d 100644 --- a/mkfadumprd +++ b/mkfadumprd @@ -19,6 +19,8 @@ fi
MKFADUMPRD_TMPDIR="$(mktemp -d -t mkfadumprd.XXXXXX)" [ -d "$MKFADUMPRD_TMPDIR" ] || perror_exit "mkfadumprd: mktemp -d -t mkfadumprd.XXXXXX failed." + +# shellcheck disable=SC2154 # known issue of shellcheck https://github.com/koalaman/shellcheck/issues/1299 trap ' ret=$?; [[ -d $MKFADUMPRD_TMPDIR ]] && rm --one-file-system -rf -- "$MKFADUMPRD_TMPDIR";
Shellcheck finds In kdump-lib.sh line 249: echo $(get_persistent_dev "$dev") ^--------------------------^ SC2046: Quote this to prevent word splitting. ^--------------------------^ SC2005: Useless echo? Instead of 'echo $(cmd)', just use 'cmd'.
Here echo is useless. By removing echo, we also get rid of SC2046.
Signed-off-by: Coiby Xu coxu@redhat.com --- kdump-lib.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kdump-lib.sh b/kdump-lib.sh index 43dbad5..873833b 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -247,7 +247,7 @@ kdump_get_persistent_dev() dev=$(blkid -L "${dev#LABEL=}") ;; esac - echo $(get_persistent_dev "$dev") + get_persistent_dev "$dev" }
is_ostree()
"set --" was designed for word splitting. This is a known issue of shellcheck https://github.com/koalaman/shellcheck/issues/2335.
Signed-off-by: Coiby Xu coxu@redhat.com --- kdump-lib.sh | 1 + 1 file changed, 1 insertion(+)
diff --git a/kdump-lib.sh b/kdump-lib.sh index 873833b..ec338b4 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -425,6 +425,7 @@ is_dracut_mod_omitted() { local dracut_mod=$1
+ # shellcheck disable=SC2046 # a known issue https://github.com/koalaman/shellcheck/issues/2335 set -- $(kdump_get_conf_val dracut_args) while [ $# -gt 0 ]; do case $1 in
Shellcheck complains that
In kdump-lib.sh line 528: awk ' \ ^-- SC1004: This backslash+linefeed is literal. Break outside single quotes if you just want to break the line. ...
There is no need to use backslash+linefeed for awk and we can include the linefeed without a backlash.
Signed-off-by: Coiby Xu coxu@redhat.com --- kdump-lib.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kdump-lib.sh b/kdump-lib.sh index ec338b4..c5bee02 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -527,10 +527,10 @@ remove_cmdline_param() # get_bootcpu_apicid() { - awk ' \ - BEGIN { CPU = "-1"; } \ - $1=="processor" && $2==":" { CPU = $NF; } \ - CPU=="0" && /^apicid/ { print $NF; } \ + awk ' + BEGIN { CPU = "-1"; } + $1=="processor" && $2==":" { CPU = $NF; } + CPU=="0" && /^apicid/ { print $NF; } ' \ /proc/cpuinfo }
Fix the following shellcheck SC2295 warnings, In kdump-lib.sh line 177: _path=${1#$_mnt} ^---^ SC2295 (info): Expansions inside ${..} need to be quoted separately, otherwise they match as patterns.
Did you mean: _path=${1#"$_mnt"}
In kdump-lib.sh line 194: _fsroot=${_src#${_src_nofsroot}[} ^--------------^ SC2295 (info): Expansions inside ${..} need to be quoted separately, otherwise they match as patterns.
Did you mean: _fsroot=${_src#"${_src_nofsroot}"[}
In kdump-lib.sh line 203: _fsroot=${_fsroot#$_subvol} ^------^ SC2295 (info): Expansions inside ${..} need to be quoted separately, otherwise they match as patterns.
Did you mean: _fsroot=${_fsroot#"$_subvol"}
Signed-off-by: Coiby Xu coxu@redhat.com --- kdump-lib.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kdump-lib.sh b/kdump-lib.sh index c5bee02..9a5320d 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -174,7 +174,7 @@ get_bind_mount_source() local _fsroot _src_nofsroot
_mnt=$(df "$1" | tail -1 | awk '{print $NF}') - _path=${1#$_mnt} + _path=${1#"$_mnt"}
_src=$(get_mount_info SOURCE target "$_mnt" -f) _opt=$(get_mount_info OPTIONS target "$_mnt" -f) @@ -191,7 +191,7 @@ get_bind_mount_source() echo "$_mnt$_path" && return fi
- _fsroot=${_src#${_src_nofsroot}[} + _fsroot=${_src#"${_src_nofsroot}"[} _fsroot=${_fsroot%]} _mnt=$(get_mount_info TARGET source "$_src_nofsroot" -f)
@@ -200,7 +200,7 @@ get_bind_mount_source() local _subvol _subvol=${_opt#*subvol=} _subvol=${_subvol%,*} - _fsroot=${_fsroot#$_subvol} + _fsroot=${_fsroot#"$_subvol"} fi echo "$_mnt$_fsroot$_path" }
KDUMP_*LOGLVL are defined in /etc/kdump/sysconfig. They are accessible to kdump-logger.sh because scripts like kdumpcl will first source /etc/kdump/sysconfig and then source kdump-logger.sh.
Signed-off-by: Coiby Xu coxu@redhat.com --- kdump-logger.sh | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/kdump-logger.sh b/kdump-logger.sh index 20ac86e..f9af003 100755 --- a/kdump-logger.sh +++ b/kdump-logger.sh @@ -97,8 +97,11 @@ dlog_init() kdump_stdloglvl=0 kdump_kmsgloglvl=0 else + # shellcheck disable=SC2153 # KDUMP_*LOGLVL is read from /etc/kdump/sysconfig by file that source kdump-logger.sh kdump_stdloglvl=$KDUMP_STDLOGLVL + # shellcheck disable=SC2153 kdump_sysloglvl=$KDUMP_SYSLOGLVL + # shellcheck disable=SC2153 kdump_kmsgloglvl=$KDUMP_KMSGLOGLVL fi
shellcheck complains that, In dracut-module-setup.sh line 382: for _dev in $(cat "/sys/class/net/$_netdev/bonding/slaves"); do ^-- SC2013 (info): To read lines rather than words, pipe/redirect to a 'while read' loop.
This is a false positive because in this case read words is exactly what's needed.
Signed-off-by: Coiby Xu coxu@redhat.com --- dracut-module-setup.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh index b478e22..f26f66f 100755 --- a/dracut-module-setup.sh +++ b/dracut-module-setup.sh @@ -379,7 +379,7 @@ kdump_setup_bond() { local _netdev="$1" local _conpath="$2" local _dev _mac _slaves _kdumpdev _bondoptions - for _dev in $(cat "/sys/class/net/$_netdev/bonding/slaves"); do + for _dev in $(< "/sys/class/net/$_netdev/bonding/slaves"); do _mac=$(kdump_get_perm_addr "$_dev") _kdumpdev=$(kdump_setup_ifname "$_dev") echo -n " ifname=$_kdumpdev:$_mac" >> "${initdir}/etc/cmdline.d/42bond.conf"
shellcheck makes the following complaints,
In dracut-module-setup.sh line 268: _target=$(echo "$_route" | awk '{print $1}') ^-----^ SC2030 (info): Modification of _target is local (to subshell caused by pipeline).
In dracut-module-setup.sh line 292: [[ $_target == 'default' ]] && continue ^------^ SC2031 (info): _target was modified in a subshell. That change might be lost.
In dracut-module-setup.sh line 300: _rule="rd.route=[$_target]:[$_nexthop]:$kdumpnic" ^------^ SC2031 (info): _target was modified in a subshell. That change might be lost.
In dracut-module-setup.sh line 302: _rule="rd.route=$_target:$_nexthop:$kdumpnic" ^------^ SC2031 (info): _target was modified in a subshell. That change might be lost.
because setting variables in a pipeline actually happens in a subshell [1]. In this case, shellcheck wrongly think _target from kdump_static_ip is going to be used by kdump_handle_mulitpath_route so they are false positives and this is a known issue [2]. Rewrite kdump_static_ip to read from process substitution instead of from pipeline to get rid of these warnings.
[1] https://github.com/koalaman/shellcheck/wiki/SC2031 [2] https://github.com/koalaman/shellcheck/issues/280
Signed-off-by: Coiby Xu coxu@redhat.com --- dracut-module-setup.sh | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh index f26f66f..85ec1aa 100755 --- a/dracut-module-setup.sh +++ b/dracut-module-setup.sh @@ -262,17 +262,17 @@ kdump_static_ip() { echo -n "${_srcaddr}::${_gateway}:${_netmask}::" fi
- /sbin/ip $_ipv6_flag route show | grep -v default \ - | grep ".*via.* $_netdev " | grep -v "^[[:space:]]*nexthop" \ - | while read -r _route; do - _target=$(echo "$_route" | awk '{print $1}') - _nexthop=$(echo "$_route" | awk '{print $3}') - if [[ "x" != "x"$_ipv6_flag ]]; then - _target="[$_target]" - _nexthop="[$_nexthop]" - fi - echo "rd.route=$_target:$_nexthop:$kdumpnic" - done >> "${initdir}/etc/cmdline.d/45route-static.conf" + while read -r _route; do + _target=$(echo "$_route" | awk '{print $1}') + _nexthop=$(echo "$_route" | awk '{print $3}') + if [[ "x" != "x"$_ipv6_flag ]]; then + _target="[$_target]" + _nexthop="[$_nexthop]" + fi + echo "rd.route=$_target:$_nexthop:$kdumpnic" + done >> "${initdir}/etc/cmdline.d/45route-static.conf" \ + < <(/sbin/ip $_ipv6_flag route show | grep -v default \ + | grep ".*via.* $_netdev " | grep -v "^[[:space:]]*nexthop")
kdump_handle_mulitpath_route "$_netdev" "$_srcaddr" "$kdumpnic" }
Fix two POSIX issues detected by shellcheck, In kdump-logger.sh line 122: if [ "$UID" -ne 0 ]; then ^--^ SC3028 (warning): In POSIX sh, UID is undefined.
In kdump-logger.sh line 135: exec 15> "$_systemdcatfile" ^--------------------^ SC3023 (warning): In POSIX sh, FDs outside 0-9 are undefined.
Signed-off-by: Coiby Xu coxu@redhat.com --- kdump-logger.sh | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/kdump-logger.sh b/kdump-logger.sh index f9af003..ee568f1 100755 --- a/kdump-logger.sh +++ b/kdump-logger.sh @@ -119,6 +119,7 @@ dlog_init() # Skip initialization if it's already done. [ -n "$kdump_maxloglvl" ] && return 0
+ [ -n "$BASH" ] || UID=$(id -u) if [ "$UID" -ne 0 ]; then kdump_kmsgloglvl=0 kdump_sysloglvl=0 @@ -130,9 +131,15 @@ dlog_init() systemctl --quiet is-active systemd-journald.socket 1> /dev/null 2>&1; then readonly _systemdcatfile="/var/tmp/systemd-cat" mkfifo "$_systemdcatfile" 1> /dev/null 2>&1 - readonly _dlogfd=15 systemd-cat -t 'kdump' --level-prefix=true < "$_systemdcatfile" & - exec 15> "$_systemdcatfile" + if [ -n "$BASH" ]; then + readonly _dlogfd=15 + # shellcheck disable=SC3023 # this if branch only run by bash + exec 15> "$_systemdcatfile" + else + readonly _dlogfd=9 + exec 9> "$_systemdcatfile" + fi elif ! [ -S /dev/log ] && [ -w /dev/log ] || ! command -v logger > /dev/null; then # We cannot log to syslog, so turn this facility off. kdump_kmsgloglvl=$kdump_sysloglvl
In kdump-lib.sh line 855: IFS=, read start start_unit end end_unit size <<< \ ^--^ SC2162 (info): read without -r will mangle backslashes.
In kdump-lib.sh line 877: kdump_get_arch_recommend_crashkernel() ^-- SC2120 (warning): kdump_get_arch_recommend_crashkernel references arguments, but none are ever passed.
In kdump-lib.sh line 920: _ck_cmdline=$(kdump_get_arch_recommend_crashkernel) ^-- SC2119 (info): Use kdump_get_arch_recommend_crashkernel "$@" if function's $1 should mean script's $1.
Signed-off-by: Coiby Xu coxu@redhat.com --- kdump-lib.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kdump-lib.sh b/kdump-lib.sh index 9a5320d..9af3204 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -852,7 +852,7 @@ get_recommend_size() while read -r -d , range; do # need to use non-default IFS as double spaces are used as a # single delimiter while commas aren't... - IFS=, read start start_unit end end_unit size <<< \ + IFS=, read -r start start_unit end end_unit size <<< \ "$(echo "$range" | sed -n "s/([0-9]+)([GT]?)-([0-9]*)([GT]?):([0-9]+[MG])/\1,\2,\3,\4,\5/p")"
# aka. 102400T @@ -874,6 +874,7 @@ get_recommend_size()
# get default crashkernel # $1 dump mode, if not specified, dump_mode will be judged by is_fadump_capable +# shellcheck disable=SC2120 # kdumpctl will call this func with an argument kdump_get_arch_recommend_crashkernel() { local _arch _ck_cmdline _dump_mode
shellcheck complains that, In spec/kdumpctl_general_spec.sh line 100: '' failure ^-- SC2286 (error): This empty string is interpreted as a command name. Double check syntax (or use 'true' as a no-op).
Eliminate this warning by disabling SC2286 file-wide as is the same with the many shellspec files e.g. https://github.com/shellspec/shellspec/blob/b8935ec266d2c5f7f364c6024730e4d6...
Signed-off-by: Coiby Xu coxu@redhat.com --- spec/kdumpctl_general_spec.sh | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh index 5a3ce1c..effc7b2 100644 --- a/spec/kdumpctl_general_spec.sh +++ b/spec/kdumpctl_general_spec.sh @@ -1,4 +1,6 @@ #!/bin/bash +# shellcheck disable=SC2286 + Describe 'kdumpctl' Include ./kdumpctl
Currently tests depends on the kernel nbd module to the test image. But the nbd module may not exist (e.g. Gitlab CI's shared runner doesn't provide it) or we simply don't have the root permission to use /dev/nbd (e.g. in a rootless container) for security concerns. To address these limitations, use libguestfs's guestmount/guestunmount to mount/unmount when USE_GUESTMOUNT=1. Another benefit of guestmount is it can handle the case where a qemu image has multiple partitions automatically.
Note 1. guestmount doesn't need root permission but dnf currently mandates sudo. So still use sudo to call guestmount to simply make dnf happy. pkcon doesn't require sudo but unfornately it doesn't support --installroot.
2. guestunmount is similar to an async command, so we need to implement our own logic to wait for unmounting to be truly finished.
Signed-off-by: Coiby Xu coxu@redhat.com --- tests/scripts/build-image.sh | 6 +++ tests/scripts/image-init-lib.sh | 94 +++++++++++++++++++++++++-------- 2 files changed, 77 insertions(+), 23 deletions(-)
diff --git a/tests/scripts/build-image.sh b/tests/scripts/build-image.sh index c196bfb..f8d8485 100755 --- a/tests/scripts/build-image.sh +++ b/tests/scripts/build-image.sh @@ -54,4 +54,10 @@ img_add_qemu_cmd() {
[ -e "$INST_SCRIPT" ] && source $INST_SCRIPT
+if [[ $USE_GUESTMOUNT ]]; then + # unmount the image before renaming it otherwise sync_guestunmount may falsely + # think that the image has been truely unmounted + sync_guestunmount $OUTPUT_IMAGE.building ${MNTS[$OUTPUT_IMAGE.building]} +fi + mv $OUTPUT_IMAGE.building $OUTPUT_IMAGE diff --git a/tests/scripts/image-init-lib.sh b/tests/scripts/image-init-lib.sh index 467f32f..39b20f0 100644 --- a/tests/scripts/image-init-lib.sh +++ b/tests/scripts/image-init-lib.sh @@ -20,20 +20,64 @@ is_mounted() findmnt -k -n $1 &>/dev/null }
-clean_up() +is_qemu_image_locked() { - for _mnt in ${MNTS[@]}; do - is_mounted $_mnt && $SUDO umount -f -R $_mnt + qemu-img info $1 2>&1 >/dev/null | grep -q "lock" +} + +# call guestmount and wait for image is truely umounted. +# +# guestumount simply asks qemu to quit. When guesumount returns, qemu may still +# in the quiting process. In this case, if we start a new qemu instance, it will +# complain with 'Failed to get shared "write" lock'. +sync_guestunmount() { + local i _image _mnt _wait_times + + DEFAULT_TIMES=100 + _image=$1 + _mnt=$2 + i=0 + + $SUDO LIBGUESTFS_BACKEND=direct guestunmount $_mnt + + if [[ $GUEST_UNMOUNT_WAIT_TIMES ]]; then + _wait_times=$GUEST_UNMOUNT_WAIT_TIMES + else + _wait_times=$DEFAULT_TIMES + fi + + # wait 10s at maximum for $_image to be available + while is_qemu_image_locked $_image && [[ $i -lt $_wait_times ]]; do + i=$[$i+1] + sleep 0.1 done
- for _dev in ${DEVS[@]}; do - [ ! -e "$_dev" ] && continue - [[ "$_dev" == "/dev/loop"* ]] && $SUDO losetup -d "$_dev" - [[ "$_dev" == "/dev/nbd"* ]] && $SUDO qemu-nbd --disconnect "$_dev" + if [[ $i == $_wait_times ]]; then + perror_exit "After 0.1*$_wait_times seconds, $_image is still locked. You may need to increase GUEST_UNMOUNT_WAIT_TIMES (default=$DEFAULT_TIMES)" + fi +} + +clean_up() +{ + for _image in ${!MNTS[@]}; do + _mnt=${MNTS[$_image]} + if [[ $USE_GUESTMOUNT ]] && is_mounted $_mnt; then + sync_guestunmount $_image $_mnt + else + is_mounted $_mnt && $SUDO umount -f -R $_mnt + fi + rm $_image.lock done
- [ -d "$TMPDIR" ] && $SUDO rm --one-file-system -rf -- "$TMPDIR"; + if [[ ! $USE_GUESTMOUNT ]]; then + for _dev in ${DEVS[@]}; do + [ ! -e "$_dev" ] && continue + [[ "$_dev" == "/dev/loop"* ]] && $SUDO losetup -d "$_dev" + [[ "$_dev" == "/dev/nbd"* ]] && $SUDO qemu-nbd --disconnect "$_dev" + done + fi
+ [ -d "$TMPDIR" ] && $SUDO rm --one-file-system -rf -- "$TMPDIR"; sync }
@@ -159,10 +203,11 @@ mount_image() { [ $? -ne 0 ] || [ -z "$dev" ] && perror_exit "failed to setup loop device"
elif fmt_is_qcow2 "$fmt"; then - prepare_nbd - - dev=$(mount_nbd $image) - [ $? -ne 0 ] || [ -z "$dev" ] perror_exit "failed to connect qemu to nbd device '$dev'" + if [[ ! $USE_GUESTMOUNT ]]; then + prepare_nbd + dev=$(mount_nbd $image) + [ $? -ne 0 ] || [ -z "$dev" ] perror_exit "failed to connect qemu to nbd device '$dev'" + fi else perror_exit "Unrecognized image format '$fmt'" fi @@ -172,16 +217,19 @@ mount_image() { [ $? -ne 0 ] || [ -z "$mnt" ] && perror_exit "failed to create tmp mount dir" MNTS[$image]="$mnt"
- mnt_dev=$(get_mountable_dev "$dev") - [ $? -ne 0 ] || [ -z "$mnt_dev" ] && perror_exit "failed to setup loop device" - - $SUDO mount $mnt_dev $mnt - [ $? -ne 0 ] && perror_exit "failed to mount device '$mnt_dev'" - boot=$(get_mount_boot "$dev") - if [[ -n "$boot" ]]; then - root=$(get_image_mount_root $image) - $SUDO mount $boot $root/boot - [ $? -ne 0 ] && perror_exit "failed to mount the bootable partition for device '$mnt_dev'" + if [[ $USE_GUESTMOUNT ]]; then + $SUDO LIBGUESTFS_BACKEND=direct guestmount -a $image -i $mnt + else + mnt_dev=$(get_mountable_dev "$dev") + [ $? -ne 0 ] || [ -z "$mnt_dev" ] && perror_exit "failed to setup loop device" + $SUDO mount $mnt_dev $mnt + [ $? -ne 0 ] && perror_exit "failed to mount device '$mnt_dev'" + boot=$(get_mount_boot "$dev") + if [[ -n "$boot" ]]; then + root=$(get_image_mount_root $image) + $SUDO mount $boot $root/boot + [ $? -ne 0 ] && perror_exit "failed to mount the bootable partition for device '$mnt_dev'" + fi fi }
@@ -218,7 +266,7 @@ inst_pkg_in_image() { # if [ "$distro" != "Fedora" ]; then # perror_exit "only Fedora image is supported" # fi - release=$(cat $root/etc/fedora-release | sed -n "s/.*[Rr]elease\s*([0-9]*).*/\1/p") + release=$(sudo cat $root/etc/fedora-release | sed -n "s/.*[Rr]elease\s*([0-9]*).*/\1/p") [ $? -ne 0 ] || [ -z "$release" ] && perror_exit "only Fedora image is supported"
$SUDO dnf --releasever=$release --installroot=$root install -y $@
Currently, a VM is allowed to run at hard-coded 10mins at maximum. In cases where the tests are running inside Github or Gitlab's CI testing machines, it takes much longer time. Make it configurable by the KUMP_TEST_QEMU_TIMEOUT environment variable.
Signed-off-by: Coiby Xu coxu@redhat.com --- tests/scripts/test-lib.sh | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/tests/scripts/test-lib.sh b/tests/scripts/test-lib.sh index 8b24b2a..667cce0 100644 --- a/tests/scripts/test-lib.sh +++ b/tests/scripts/test-lib.sh @@ -86,10 +86,16 @@ build_test_image() { }
run_test_sync() { - local qemu_cmd=$(get_test_qemu_cmd $1) + local qemu_cmd=$(get_test_qemu_cmd $1) _timeout + + if [[ $KUMP_TEST_QEMU_TIMEOUT ]]; then + _timeout=$KUMP_TEST_QEMU_TIMEOUT + else + _timeout=10m + fi
if [ -n "$qemu_cmd" ]; then - timeout --foreground 10m $BASEDIR/run-qemu $(get_test_qemu_cmd $1) + timeout --foreground $_timeout $BASEDIR/run-qemu $(get_test_qemu_cmd $1) else echo "error: test qemu command line is not configured" > /dev/stderr return 1
With this patch, four kinds of tests are triggered when there is a pull request created in a Github kexec-tools repo, - format check using shfmt - static analysis using shellcheck - ShellSpec unit tests (test cases in spec/) - integration tests ( tests cases in tests/)
The tests are run inside a docker image. This docker image has all the needed software installed including fedpkg, shellspec and etc. This Docker image also provides /usr/share/cloud_images/Fedora-Cloud-Base-35-1.2.x86_64.qcow2 to avoid repeatedly downloading the Fedora 35 cloud base image each time the tests are triggered.
Signed-off-by: Coiby Xu coxu@redhat.com --- .github/workflows/main.yml | 53 ++++++++++++++++++++++++++++++++++ tools/run-integration-tests.sh | 29 +++++++++++++++++++ 2 files changed, 82 insertions(+) create mode 100644 .github/workflows/main.yml create mode 100755 tools/run-integration-tests.sh
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml new file mode 100644 index 0000000..a62aa0e --- /dev/null +++ b/.github/workflows/main.yml @@ -0,0 +1,53 @@ +name: kexec-tools tests + +on: pull_request + +jobs: + format-check: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - run: wget https://github.com/mvdan/sh/releases/download/v3.4.3/shfmt_v3.4.3_linux_amd6... -O /usr/local/bin/shfmt && chmod +x /usr/local/bin/shfmt + - run: shfmt -d *.sh kdumpctl mk*dumprd + + static-analysis: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - run: curl -L -O https://github.com/koalaman/shellcheck/releases/download/v0.8.0/shellcheck-v... && tar -xJf shellcheck-v0.8.0.linux.x86_64.tar.xz + # Currently, for kexec-tools, there is need for shellcheck to require + # the sourced file to give correct warnings about the checked file + - run: shellcheck-v0.8.0/shellcheck --exclude=1090,1091 *.sh spec/*.sh kdumpctl mk*dumprd + + unit-tests: + runs-on: ubuntu-latest + container: docker.io/fedora:latest + steps: + - uses: actions/checkout@v2 + - run: sudo dnf install -y make dracut grubby hostname + - run: curl -L -O https://github.com/shellspec/shellspec/archive/latest.tar.gz && tar -xzf latest.tar.gz + - run: cd shellspec-latest && sudo make install + - run: shellspec + + integration-tests: + runs-on: self-hosted + timeout-minutes: 45 + concurrency: + group: ${{ github.workflow }}-${{ github.ref }}-${{ matrix.container }}-${{ matrix.test }} + cancel-in-progress: true + strategy: + matrix: + container: [ + "fedora:35", + ] + fail-fast: false + container: + image: ghcr.io/coiby/${{ matrix.container }} + options: "--privileged -v /dev:/dev -v /lib/modules:/lib/modules:ro" + steps: + - name: "Checkout Repository" + uses: actions/checkout@v2 + with: + fetch-depth: 0 + - name: "${{ matrix.container }} kdump tests" + run: bash ./tools/run-integration-tests.sh diff --git a/tools/run-integration-tests.sh b/tools/run-integration-tests.sh new file mode 100755 index 0000000..7883c3f --- /dev/null +++ b/tools/run-integration-tests.sh @@ -0,0 +1,29 @@ +#!/bin/bash +set -ex + +[[ -d ${0%/*} ]] && cd "${0%/*}"/../ + +source /etc/os-release + +cd tests + +# fedpkg fetch sources based on branch.f$VERSION_ID.remote +if ! git remote show | grep -q fedora_src; then + git remote add fedora_src https://src.fedoraproject.org/rpms/kexec-tools.git + git config --add branch.f$VERSION_ID.remote fedora_src +fi + +can_we_use_qemu_nbd() +{ + _tmp_img=/tmp/test.qcow2 + + (sudo -v && sudo modprobe nbd \ + && qemu-img create -fqcow2 $_tmp_img 10m \ + && qemu-nbd -c /dev/nbd0 $_tmp_img && sudo qemu-nbd -d /dev/nbd0 && rm $_tmp_img) &> /dev/null +} + +if ! can_we_use_qemu_nbd; then + USE_GUESTMOUNT=1 +fi + +KUMP_TEST_QEMU_TIMEOUT=20m USE_GUESTMOUNT=$USE_GUESTMOUNT BASE_IMAGE=/usr/share/cloud_images/Fedora-Cloud-Base-35-1.2.x86_64.qcow2 RELEASE=$VERSION_ID make test-run
-
Coiby Xu