[PATCH 0/2] e2scrub: fix some problems

[PATCH 0/2] e2scrub: fix some problems

[Darrick J. Wong]

Hi all,

Fix a couple of problems that have been reported against e2scrub_all.
The first fixes a complaint about a large increase in boot time due to
automatic reaping of e2scrub snapshots.  The second eliminates some
broken hackery around loop iteration in bash.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

[PATCH 1/2] e2scrub_all: don't even reap if the config file doesn't allow it

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Dave Chinner complains that the automated on-boot e2scrub reaping takes
a long time (because the lvs command can take a while to run) even
though the automated e2scrub is disabled via e2scrub.conf on his
systems.

We still need the reaping service to kill off stale e2scrub snapshots
after a crash, but it's unnecessary to annoy everyone with slow bootup.
Because we can look for the e2scrub snapshots in /dev/mapper, let's
skip reaping if periodic e2scrub is disabled unless we find evidence of
e2scrub snapshots in /dev.

Reported-by: Dave Chinner <david@fromorbit.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 scrub/e2scrub_all.in |   15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)


diff --git a/scrub/e2scrub_all.in b/scrub/e2scrub_all.in
index 1418a229..72e66ff6 100644
--- a/scrub/e2scrub_all.in
+++ b/scrub/e2scrub_all.in
@@ -80,9 +80,18 @@ while getopts "nrAV" opt; do
 done
 shift "$((OPTIND - 1))"
 
-if [ -n "${SERVICE_MODE}" -a "${reap}" -ne 1 -a "${periodic_e2scrub}" -ne 1 ]
-then
-    exitcode 0
+# If we're in service mode and the service is not enabled via config file...
+if [ -n "${SERVICE_MODE}" -a "${periodic_e2scrub}" -ne 1 ]; then
+	# ...don't start e2scrub processes.
+	if [ "${reap}" -eq 0 ]; then
+		exitcode 0
+	fi
+
+	# ...and if we don't see any leftover e2scrub snapshots, don't
+	# run the reaping process either, because lvs can be slow.
+	if ! readlink -q -s -e /dev/mapper/*.e2scrub* > /dev/null; then
+		exitcode 0
+	fi
 fi
 
 # close file descriptor 3 (from cron) since it causes lvm to kvetch

[PATCH 2/2] e2scrub_all: fix broken stdin redirection

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

gregor herrmann reports that the weekly e2scrub cronjob emits these
errors:

/sbin/e2scrub_all: line 173: /proc/8234/fd/pipe:[90083173]: No such file or directory

The root cause of this is that the ls_targets stdout is piped to stdin
to the entire ls_targets loop body to prevent the loop body from reading
the loop iteration items.  Remove all the broken hackery by reading the
target list into a bash array and iterating the bash array.

Addresses-Debian-Bug: #944033

Reported-by: gregor herrmann <gregoa@debian.org>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 scrub/e2scrub_all.in |   14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)


diff --git a/scrub/e2scrub_all.in b/scrub/e2scrub_all.in
index 72e66ff6..f0336711 100644
--- a/scrub/e2scrub_all.in
+++ b/scrub/e2scrub_all.in
@@ -101,6 +101,12 @@ exec 3<&-
 # indicating success to avoid spamming the sysadmin with fail messages
 # when e2scrub_all is run out of cron or a systemd timer.
 
+if ! type mapfile >& /dev/null ; then
+    test -n "${SERVICE_MODE}" && exitcode 0
+    echo "e2scrub_all: can't find mapfile --- is bash 4.xx installed?"
+    exitcode 1
+fi
+
 if ! type lsblk >& /dev/null ; then
     test -n "${SERVICE_MODE}" && exitcode 0
     echo "e2scrub_all: can't find lsblk --- is util-linux installed?"
@@ -165,13 +171,13 @@ escape_path_for_systemd() {
 }
 
 # Scrub any mounted fs on lvm by creating a snapshot and fscking that.
-stdin="$(realpath /dev/stdin)"
-ls_targets | while read tgt; do
+mapfile -t targets < <(ls_targets)
+for tgt in "${targets[@]}"; do
 	# If we're not reaping and systemd is present, try invoking the
 	# systemd service.
 	if [ "${reap}" -ne 1 ] && type systemctl > /dev/null 2>&1; then
 		tgt_esc="$(escape_path_for_systemd "${tgt}")"
-		${DBG} systemctl start "e2scrub@${tgt_esc}" 2> /dev/null < "${stdin}"
+		${DBG} systemctl start "e2scrub@${tgt_esc}" 2> /dev/null
 		res=$?
 		if [ "${res}" -eq 0 ] || [ "${res}" -eq 1 ]; then
 			continue;
@@ -179,7 +185,7 @@ ls_targets | while read tgt; do
 	fi
 
 	# Otherwise use direct invocation
-	${DBG} "@root_sbindir@/e2scrub" ${scrub_args} "${tgt}" < "${stdin}"
+	${DBG} "@root_sbindir@/e2scrub" ${scrub_args} "${tgt}"
 done
 
 exitcode 0

Re: [PATCH 0/2] e2scrub: fix some problems

[Theodore Y. Ts'o]

On Mon, Nov 04, 2019 at 05:54:08PM -0800, Darrick J. Wong wrote:
> Hi all,
> 
> Fix a couple of problems that have been reported against e2scrub_all.
> The first fixes a complaint about a large increase in boot time due to
> automatic reaping of e2scrub snapshots.  The second eliminates some
> broken hackery around loop iteration in bash.
> 
> This is an extraordinary way to destroy everything.  Enjoy!
> Comments and questions are, as always, welcome.

Thanks, look good.  I've applied this to the e2fsprogs maint branch.

	     	    	 	      - Ted