save-dumps: Improve pstore/kdump loop file reading on log saving

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 <gpiccoli@igalia.com>

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)
+LOGS_FOUND=0
-if [ "${PSTORE_CNT}" -ne 0 ]; then
+PSTORE_FOLDER="${KDUMP_TMP_FOLDER}/pstore"
 
-	PSTORE_FOLDER="${KDUMP_TMP_FOLDER}/pstore"
+while IFS= read -r log
-	mkdir -p "${PSTORE_FOLDER}"
+do
+	if [[ "${log}" == *"dmesg-ramoops"* ]]; then
+		SAVED_FILE="${PSTORE_FOLDER}/dmesg-pstore.${CURRENT_TSTAMP}-${LOGS_FOUND}"
+		mkdir -p "${PSTORE_FOLDER}"
 
-	LOOP_CNT=0
+		cat "${log}" > "${SAVED_FILE}"
-	while [ "${PSTORE_CNT}" -gt 0 ]; do
+		sync "${SAVED_FILE}"
-		PSTORE_FILE="$(find /sys/fs/pstore/* | grep dmesg-ramoops | sort | head -n1)"
+		rm -f "${log}"
-		SAVED_FILE="${PSTORE_FOLDER}/dmesg-pstore.${CURRENT_TSTAMP}-${LOOP_CNT}"
+		LOGS_FOUND=$((LOGS_FOUND + 1))
-
+	fi
-		if [ -e "${PSTORE_FILE}" ]; then
+done <<< "$(find /sys/fs/pstore/ -type f 2>/dev/null)"
-			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}
 
+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)
+while IFS= read -r crash
-if [ "${KDUMP_CNT}" -ne 0 ]; then
+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
+	if [ -s "${crash}/dmesg.txt" ]; then
-	while [ "${KDUMP_CNT}" -gt 0 ]; do
+		SAVED_FILE="${KD_FOLDER}/dmesg-kdump.${CRASH_TSTAMP}"
-		CRASH_CURRENT=$(find "${KDUMP_CRASH_FOLDER}"/* -type d 2>/dev/null | head -n1)
+		mv "${crash}/dmesg.txt" "${SAVED_FILE}"
+		sync "${SAVED_FILE}"
+		CRASHES_FOUND=$((CRASHES_FOUND + 1))
+	fi
 
-		#  When collecting the vmcore/dmesg during kdump, folder is
+	#  We won't pack vmcores in the zip blob, but let's save
-		#  saved with its name == the timestamp of the collection.
+	#  it in case it was collected as well.
-		CRASH_TSTAMP=$(basename "${CRASH_CURRENT}")
+	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
+	rm -rf "${crash}"
-			SAVED_FILE="${KD_FOLDER}/dmesg-kdump.${CRASH_TSTAMP}"
+done <<< "$(find "${KDUMP_CRASH_FOLDER}"/ -mindepth 1 -type d 2>/dev/null)"
-			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))
 
+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