From 756e698650789107566cf9aef045d2bddc57b60e Mon Sep 17 00:00:00 2001 From: "Guilherme G. Piccoli" Date: Thu, 19 Jan 2023 11:17:52 -0300 Subject: [PATCH] save-dumps: Improve pstore/kdump loop file reading on log saving MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There is a bunch of improvements done here in the file collecting loops during the log saving routine; special thanks to Clayton Craft (craftyguy) for the suggestions that led to most of this refactor. (a) We were using the LOOP_CNT variable, and it proved to to be unnecessary; so removed it. (b) Overall we were looping on a log counter and running "find/grep" at each iteration. Hereby we reworked this logic to loop on top of the file entries directly, improving both performance and (likely) the code readability. (c) Fixed some comments and a shellcheck complain for not guarding LOGS_FOUND on quotes ¬¬ With that refactor, we fixed a "counting" issue: if there are 2+ pstore logs (a console dump plus oops/panic dump), despite we only save the dmesg from panic, it was saved as log#1 instead of log#0. Really minor, but...fixed now. Also, fixed here another relevant issue: we were always removing the saved logs, even in the case of the zip compression fail; now we bail-out if the compressed blob wasn't created, preserving the logs collected. P.S. Worth to mention the use of here-strings in the code to make global variables modified inside the loop available after the loop - very interesting/useful trick but bash restricted, unfortunately. Signed-off-by: Guilherme G. Piccoli --- save-dumps.sh.in | 101 +++++++++++++++++++++-------------------------- 1 file changed, 46 insertions(+), 55 deletions(-) diff --git a/save-dumps.sh.in b/save-dumps.sh.in index 68a61bc..84f2082 100644 --- a/save-dumps.sh.in +++ b/save-dumps.sh.in @@ -7,90 +7,80 @@ if [ ! -d "${MAIN_FOLDER}" ]; then exit 1 fi -LOGS_FOUND=0 -KDUMP_TMP_FOLDER="${MAIN_FOLDER}/.tmp" - # Use UTC timezone to match kdump collection CURRENT_TSTAMP=$(date -u +"%Y%m%d%H%M") +KDUMP_TMP_FOLDER="${MAIN_FOLDER}/.tmp" -# By default, pstore is mounted in this location; if it isn't, we bail-out. +# By default, pstore is mounted in this location; if it isn't, we move on. # Notice we currently only support the logs generated by the ramoops backend. -PSTORE_CNT=$(find /sys/fs/pstore/* 2>/dev/null | grep -c ramoops) -if [ "${PSTORE_CNT}" -ne 0 ]; then +LOGS_FOUND=0 +PSTORE_FOLDER="${KDUMP_TMP_FOLDER}/pstore" - PSTORE_FOLDER="${KDUMP_TMP_FOLDER}/pstore" - mkdir -p "${PSTORE_FOLDER}" +while IFS= read -r log +do + if [[ "${log}" == *"dmesg-ramoops"* ]]; then + SAVED_FILE="${PSTORE_FOLDER}/dmesg-pstore.${CURRENT_TSTAMP}-${LOGS_FOUND}" + mkdir -p "${PSTORE_FOLDER}" - LOOP_CNT=0 - while [ "${PSTORE_CNT}" -gt 0 ]; do - PSTORE_FILE="$(find /sys/fs/pstore/* | grep dmesg-ramoops | sort | head -n1)" - SAVED_FILE="${PSTORE_FOLDER}/dmesg-pstore.${CURRENT_TSTAMP}-${LOOP_CNT}" - - if [ -e "${PSTORE_FILE}" ]; then - cat "${PSTORE_FILE}" > "${SAVED_FILE}" - sync "${SAVED_FILE}" - rm -f "${PSTORE_FILE}" - fi - - PSTORE_CNT=$((PSTORE_CNT - 1)) - LOOP_CNT=$((LOOP_CNT + 1)) - done - LOGS_FOUND=${LOOP_CNT} + cat "${log}" > "${SAVED_FILE}" + sync "${SAVED_FILE}" + rm -f "${log}" + LOGS_FOUND=$((LOGS_FOUND + 1)) + fi +done <<< "$(find /sys/fs/pstore/ -type f 2>/dev/null)" +if [ "${LOGS_FOUND}" -gt 0 ]; then # Logs should live on <...>/.tmp folder, due to the zip compression. mv "${PSTORE_FOLDER}"/* "${KDUMP_TMP_FOLDER}/" 2>/dev/null rm -rf "${PSTORE_FOLDER}" fi -# Now, we proceed the same way if there are kdump data. +# Now, proceed the same way if there are kdump data. +CRASHES_FOUND=0 KDUMP_CRASH_FOLDER="${MAIN_FOLDER}/crash" -KDUMP_CNT=$(find "${KDUMP_CRASH_FOLDER}"/* -type d 2>/dev/null | wc -l) -if [ "${KDUMP_CNT}" -ne 0 ]; then +while IFS= read -r crash +do + # When collecting the vmcore/dmesg during kdump, folder is + # saved with its name == the timestamp of the collection. + CRASH_TSTAMP=$(basename "${crash}") + if [[ ! "${CRASH_TSTAMP}" =~ ^[0-9]{12}$ ]]; then + continue + fi + KD_FOLDER="${KDUMP_TMP_FOLDER}/kdump" mkdir -p "${KD_FOLDER}" - LOOP_CNT=0 - while [ "${KDUMP_CNT}" -gt 0 ]; do - CRASH_CURRENT=$(find "${KDUMP_CRASH_FOLDER}"/* -type d 2>/dev/null | head -n1) + if [ -s "${crash}/dmesg.txt" ]; then + SAVED_FILE="${KD_FOLDER}/dmesg-kdump.${CRASH_TSTAMP}" + mv "${crash}/dmesg.txt" "${SAVED_FILE}" + sync "${SAVED_FILE}" + CRASHES_FOUND=$((CRASHES_FOUND + 1)) + fi - # When collecting the vmcore/dmesg during kdump, folder is - # saved with its name == the timestamp of the collection. - CRASH_TSTAMP=$(basename "${CRASH_CURRENT}") + # We won't pack vmcores in the zip blob, but let's save + # it in case it was collected as well. + if [ -s "${crash}/vmcore.compressed" ]; then + SAVED_FILE="${KDUMP_CRASH_FOLDER}/vmcore.${CRASH_TSTAMP}" + mv "${crash}/vmcore.compressed" "${SAVED_FILE}" + sync "${SAVED_FILE}" + fi - if [ -s "${CRASH_CURRENT}/dmesg.txt" ]; then - SAVED_FILE="${KD_FOLDER}/dmesg-kdump.${CRASH_TSTAMP}" - mv "${CRASH_CURRENT}/dmesg.txt" "${SAVED_FILE}" - sync "${SAVED_FILE}" - - fi - - # We won't pack vmcores in the zip blob, but let's save - # it in case it was collected as well. - if [ -s "${CRASH_CURRENT}/vmcore.compressed" ]; then - SAVED_FILE="${KDUMP_CRASH_FOLDER}/vmcore.${CRASH_TSTAMP}" - mv "${CRASH_CURRENT}/vmcore.compressed" "${SAVED_FILE}" - sync "${SAVED_FILE}" - - fi - - rm -rf "${CRASH_CURRENT}" - KDUMP_CNT=$((KDUMP_CNT - 1)) - LOOP_CNT=$((LOOP_CNT + 1)) - done - - LOGS_FOUND=$((LOGS_FOUND + LOOP_CNT)) + rm -rf "${crash}" +done <<< "$(find "${KDUMP_CRASH_FOLDER}"/ -mindepth 1 -type d 2>/dev/null)" +if [ "${CRASHES_FOUND}" -gt 0 ]; then # Logs should live on .tmp folder, due to the zip compression. mv "${KD_FOLDER}"/* "${KDUMP_TMP_FOLDER}/" 2>/dev/null rm -rf "${KD_FOLDER}" + LOGS_FOUND=$((LOGS_FOUND + CRASHES_FOUND)) fi # If we have pstore and/or kdump logs, let's process them... LOGS_FOLDER="${MAIN_FOLDER}/logs" -if [ ${LOGS_FOUND} -ne 0 ]; then +if [ "${LOGS_FOUND}" -ne 0 ]; then mkdir -p "${LOGS_FOLDER}" # First we collect some more info, like DMI data, os-release, etc; @@ -114,6 +104,7 @@ if [ ${LOGS_FOUND} -ne 0 ]; then if [ ! -s "${LOG_FNAME}" ]; then logger "kdump: couldn't create the compressed log archive" logger "kdump: check folder \"${KDUMP_TMP_FOLDER}\" for logs" + exit 0 else logger "kdump: logs saved in \"${LOGS_FOLDER}\"" fi