[PATCH] tune2fs: remove dire warning about check intervals

[PATCH] tune2fs: remove dire warning about check intervals

[Eric Sandeen]

Time & mount-count based checks have been off by default for quite some
time now, but the dire warning about disabling them remains in the
tune2fs manpage, which is confusing.  We did "strongly consider
the consequences" and disabled it by default, no need to scare the
user about it now.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/misc/tune2fs.8.in b/misc/tune2fs.8.in
index 5c885f9..a8cacc7 100644
--- a/misc/tune2fs.8.in
+++ b/misc/tune2fs.8.in
@@ -134,17 +134,6 @@ Staggering the mount-counts at which filesystems are forcibly
 checked will avoid all filesystems being checked at one time
 when using journaled filesystems.
 .sp
-You should strongly consider the consequences of disabling
-mount-count-dependent checking entirely.  Bad disk drives, cables,
-memory, and kernel bugs could all corrupt a filesystem without
-marking the filesystem dirty or in error.  If you are using
-journaling on your filesystem, your filesystem will
-.B never
-be marked dirty, so it will not normally be checked.  A
-filesystem error detected by the kernel will still force
-an fsck on the next reboot, but it may already be too late
-to prevent data loss at that point.
-.sp
 See also the
 .B \-i
 option for time-dependent checking.

Re: [PATCH] tune2fs: remove dire warning about check intervals

[Andreas Dilger]

[-- Attachment #1.1: Type: text/plain, Size: 2404 bytes --]

On Jul 18, 2017, at 3:10 PM, Eric Sandeen <sandeen@redhat.com> wrote:
> 
> Time & mount-count based checks have been off by default for quite some
> time now, but the dire warning about disabling them remains in the
> tune2fs manpage, which is confusing.  We did "strongly consider
> the consequences" and disabled it by default, no need to scare the
> user about it now.

Sigh, I still think this is going in the wrong direction.  I'm happily
running a weekly e2fsck on a snapshot of the filesystem, and then reset
the time and mount-count fields in the superblock with tune2fs.  That
way I never see any warnings, or have slow boots because of a scan, but
I'm also notified if there are ever problems on the filesystem (which
happens occasionally, since I'm sometimes running experimental code).

Since virtually everyone is using MD/LVM devices these days, I don't
think that is hard to do.  I offered up my "lvcheck" script a few times,
but nobody at RH or on the DM team seemed interested at the time...
I'd also be happy if there was some other similar mechanism included with
the distro to do periodic background checks of the filesystem, rather
than letting them find any problem at some random time.  This is pretty
standard for RAID systems, I think it makes sense for the filesystem too.

Cheers, Andreas

> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/misc/tune2fs.8.in b/misc/tune2fs.8.in
> index 5c885f9..a8cacc7 100644
> --- a/misc/tune2fs.8.in
> +++ b/misc/tune2fs.8.in
> @@ -134,17 +134,6 @@ Staggering the mount-counts at which filesystems are forcibly
> checked will avoid all filesystems being checked at one time
> when using journaled filesystems.
> .sp
> -You should strongly consider the consequences of disabling
> -mount-count-dependent checking entirely.  Bad disk drives, cables,
> -memory, and kernel bugs could all corrupt a filesystem without
> -marking the filesystem dirty or in error.  If you are using
> -journaling on your filesystem, your filesystem will
> -.B never
> -be marked dirty, so it will not normally be checked.  A
> -filesystem error detected by the kernel will still force
> -an fsck on the next reboot, but it may already be too late
> -to prevent data loss at that point.
> -.sp
> See also the
> .B \-i
> option for time-dependent checking.
> 


Cheers, Andreas





[-- Attachment #1.2: lvcheck --]
[-- Type: application/octet-stream, Size: 13643 bytes --]

#!/bin/bash
# lvcheck, version 1.3
# Maintainer: Andreas Dilger <adilger@dilger.ca>

# Other credits:
# Concept and original script by Theodore Tso <tytso@mit.edu>
# Ideas, initial XFS/JFS/ZFS support: Andreas Dilger <adilger@dilger.ca>
# Better XFS support: Eric Sandeen <sandeen@redhat.com>
# Updated and polished: Bryan Kadzban <bryan.kadzban@is-a-geek.net>
# on_ac_power is mostly from Debian's powermgmt-base package

# Released under the GNU General Public License, either version 2 or
# (at your option) any later version.

# Overview:
# Run this from cron periodically (e.g. link to /etc/cron.weekly/lvcheck).
# If the machine is on AC power, it will run the checks; otherwise they will
# all be skipped. If the script can't tell whether the machine is on AC power,
# it will use $AC_UNKNOWN in the configuration file /etc/lvcheck.conf to
# decide whether to continue with the checks, or abort.
#
# The script will then decide which logical volumes are active, and can
# therefore be checked via an LVM snapshot. Each of these LVs will be
# queried to find its last-check day, and if that was more than $INTERVAL
# days ago (where $INTERVAL is set in the configuration file as well), or
# if the last-check day can't be determined, then the script will take an
# LVM snapshot of that LV and run fsck on the snapshot. The snapshot will
# be set to use 1/500 the space of the source LV. After fsck finishes,
# the snapshot is destroyed. (Snapshots are checked serially.)
#
# Any LV that passes fsck should have its last-check time updated (in the
# real superblock, not the snapshot's superblock); any LV whose fsck fails
# will send an email notification to a configurable user ($EMAIL). This $EMAIL
# setting is optional, but its use is highly recommended, since if any LV
# fails, it will need to be checked manually, offline. Relevant messages are
# also sent to syslog.

# Set default values for configuration params. Changes to these values will be
# overwritten on an upgrade!  To change these values, edit /etc/lvcheck.conf.
EMAIL='root'
INTERVAL=30
AC_UNKNOWN="CONTINUE"
LOGDIR="/var/log/lvcheck"
MINSNAP=256
MINFREE=0
VERBOSE=:

usage() {
	cat <<- USAGE
usage: lvcheck [-hnv] [-e:email] [-i interval] [-l logdir]
               [-m minfree] [device ...]
Check integrity of filesystems using temporary snapshots of logical volumes.
Defaults can be set in /etc/lvcheck.conf, or overridden on command line.

	-e: email address to send error messages to (default $EMAIL)
	-h: print this help message
	-i: interval after which a check is done (default $INTERVAL days)
	-l: directory in which logs should be stored (default $LOGDIR)
	-m: minimum free space to leave in volume group (default $MINFREE MB)
	-n: do not actually perform a check, just print what would be done
	-v: run verbosely
	device: one or more LVs to check (default: all LVs with filesystems)
	USAGE
}

# pull in configuration -- overwrite the defaults above if the file exists
# in either the system-wide /etc/lvcheck.conf or /usr/local/etc/lvcheck.conf
[ -r /etc/lvcheck.conf ] && . /etc/lvcheck.conf
CHECKPATH=$(dirname "$0" | sed -e 's:/s*bin::')
[ -r $CHECKPATH/etc/lvcheck.conf ] && . $CHECKPATH/etc/lvcheck.conf

# command-line options override config file options
while getopts "e:hi:l:m:nv" opt $*; do
	case $opt in
		e) EMAIL=$OPTARG;;
		h) usage; exit 0;;
		i) INTERVAL=$OPTARG;;
		l) LOGDIR=$OPTARG;;
		m) MINFREE=$OPTARG;;
		n) NOCHECK="echo";;
		v) VERBOSE="echo";;
		\?) usage; exit 1;;
	esac
done
shift $((OPTIND - 1))

[ -d "$LOGDIR" ] || mkdir -p "$LOGDIR"
if [ ! -d "$LOGDIR" ]; then
	LOGDIR=${tmp:-/tmp}
	log err "$LOGDIR: no such directory, logging to $LOGDIR"
fi

# send $2 to syslog, with severity $1
# severities are emerg/alert/crit/err/warning/notice/info/debug
function log() {
	local sev="$1"
	local msg="$2"
	local arg=

	# log warning-or-higher messages to stderr as well
	case $sev in
	emerg|alert|crit|err|warning)
		arg=-s
		;;
	info|debug)
		$VERBOSE "$sev: $msg"
		;;
	*)
		echo "error: unknown log severity '$sev'"
		sev=warning
		;;
	esac

	[ "$NOCHECK" ] || logger -t lvcheck $arg -p user."$sev" -- "$msg"
}

# determine whether the machine is on AC power
function on_ac_power() {
	local any_known=no

	# try sysfs power class first
	if [ -d /sys/class/power_supply ]; then
		for psu in /sys/class/power_supply/*; do
			if [ -r "$psu/type" ]; then
		       		type=$(cat "$psu/type")

				# ignore batteries
				[ "$type" = "Battery" ] && continue

				online=$(cat "$psu/online")

				[ "$online" = 1 ] && return 0
				[ "$online" = 0 ] && any_known=yes
			fi
		done

		[ "$any_known" = "yes" ] && return 1
	fi

	# else fall back to AC adapters in /proc
	if [ -d /proc/acpi/ac_adapter ]; then
		for ac in /proc/acpi/ac_adapter/*; do
			if [ -r "$ac/state" ]; then
				grep -q on-line "$ac/state" && return 0
				grep -q off-line "$ac/state" && any_known=yes
			elif [ -r "$ac/status" ]; then
				grep -q on-line "$ac/status" && return 0
				grep -q off-line "$ac/status" && any_known=yes
			fi
		done

		[ "$any_known" = "yes" ] && return 1
	fi

	if [ "$AC_UNKNOWN" == "CONTINUE" ]; then
		return 0	# assume on AC power
	elif [ "$AC_UNKNOWN" == "ABORT" ]; then
		return 1	# assume on battery
	else
		log err "Invalid value for AC_UNKNOWN in the config file"
		exit 1
	fi
}

# attempt to force a check of $1 on the next reboot
function try_force_check() {
	local dev="$1"
	local fstype="$2"

	case "$fstype" in
	ext2|ext3|ext4)
		MAX=$(dumpe2fs -h "$dev" 2>&1|awk '/Check interval/ {print $3}')
		# If the user has time dependent checking disabled, we
		# have to re-enable it.  This shouldn't be harmful, since
		# this script should be run periodically before boot and
		# reset the last-checked time each error-free run.
		if [ $MAX -lt 1 ]; then
			$NOCHECK tune2fs -i $((INTERVAL*6))d "$dev" 2> /dev/null
		fi
		$NOCHECK tune2fs -T 19700201 "$dev"
		;;
	xfs)
		# XFS does not enforce check intervals; let email suffice.
		;;
	*)
		log warning "$dev: don't know how to force a check on $fstype."
		;;
	esac
}

# attempt to set the last-check time on $1 to now, and the mount count to 0.
function try_delay_checks() {
	local dev="$1"
	local fstype="$2"

	case "$fstype" in
	ext2|ext3|ext4)
		$NOCHECK tune2fs -C 0 -T now "$dev"
		;;
	xfs)
		# XFS does not enforce check intervals; nothing to delay
		;;
	*)
		log info "$dev: don't know how to delay check on $fstype."
		;;
	esac
}

# print the date that $1 was last checked, in a format that date(1) will
# accept, or "Unknown" if we don't know how to find that date.
function try_get_check_date() {
	local dev="$1"
	local fstype="$2"

	case "$fstype" in
	ext2|ext3|ext4)
		dumpe2fs -h "$dev" 2>&1 | grep 'Last checked:' |
			sed -e 's/Last checked:[[:space:]]*//'
		;;
	zfs)
		# scan: scrub repaired 0 in 0h1m with 0 errors on \
		#     Fri Sep 16 01:27:18 2011
		zpool status $(echo $dev | cut -d/ -f1) |
			sed -e 's/scan:.*errors on //p; d'
		;;
	*)
		# XFS does not save the last-checked date 
		# TODO: add support for various other FSes
		echo "Unknown"
		;;
	esac
}

# do any extra checks for filesystem type $2, on device $1
function should_still_check() {
	local dev="$1"
	local fstype="$2"

	case "$fstype" in
	ext*)	;;
	jbd*)
		log debug "skip $dev: is an external journal"
		return 1
		;;
	swap)
		log debug "skip $dev: is a swap device"
		return 1
		;;
	zfs)
		log debug "skip $dev: is a ZFS device"
		return 1
		;;
	*)
		log warning "skip $dev: can't check $fstype passively: assume OK"
		;;
	esac

	return 0
}

# check the FS on $1 passively, saving output to $3.
function perform_check() {
	local dev="$1"
	local fstype="$2"
	local errlog="$3"
	local cons=""

	case "$fstype" in
	ext2|ext3|ext4)
		if dumpe2fs -h "$dev" 2>&1| grep -q "Journal device"; then
			log debug "$dev: removing external journal"
			$NOCHECK tune2fs -U time $dev 2>&1 | tail -n +2
			$NOCHECK tune2fs -O has_journal $dev 2>&1 | tail -n +2
		fi

		tty -s && cons="-C 0"
		# first clear the orphaned-inode list, to avoid unnecessary FS
		# changes in the next step (which would cause an "error" exit
		# from e2fsck). -C 0 is present for cases where the script is
		# run interactively (logsave -s strips out the progress bar).
		# ignore the return status of this e2fsck, as it doesn't matter.
		log info "$dev: cleaning orphan inode list"
		$NOCHECK nice logsave -as "$errlog" e2fsck -p $cons "$dev"

		# then do the real check; -y is here to give more info on any
		# errors that may be present on the FS, in the log file. the
		# snapshot is writable, so it shouldn't break anything if
		# e2fsck changes it.
		log info "$dev: running full fsck"
		$NOCHECK nice logsave -as "$errlog" e2fsck -fyvtt $cons "$dev"
		return $?
		;;
	jfs)
		log info "$dev: running full fsck"
		$NOCHECK nice logsave -as "$errlog" fsck.jfs -fn "$dev"
		return $?
		;;
	reiserfs)
		log info "$dev: running full fsck"
		echo Yes | $NOCHECK nice logsave -as "$errlog" \
			fsck.reiserfs --check "$dev"
		# apparently can't fail? let's hope not...
		return $?
		;;
	xfs)
		log info "$dev: running full fsck"
		$NOCHECK nice logsave -as "$errlog" xfs_repair -n "$dev"
		return $?
		;;
	zfs)
		#log info "$dev: starting full scrub"
		#$NOCHECK zpool scrub $(echo $dev | cut -d/ -f1)
		#return $?
		;;
	esac
}

# do everything needed to check and reset dates and counters on /dev/$1/$2.
function check_fs() {
	local vg="$1"
	local lv="$2"
	local fstype="$3"
	local snapsize="$4" # in units of MB

	local lvdev="/dev/$vg/$lv"
	local errlog="$LOGDIR/$vg-$lv-$(date +%Y%m%d)"
	local snaplvbase="$lv-lvcheck-temp"

	# try to remove old lvcheck snapshots, and skip in any case
	if [[ "$lv" =~ "lvcheck-temp" ]]; then
		# Assume script won't run more than one at a time?
		log warning "stale $lv: trying to remove old snapshot."

		$NOCHECK lvremove -f "$lv" ||
			log err "error $lv: could not delete"

		return 0
	fi

	# see whether FS needs any extra checks that might disqualify it
	should_still_check "$lvdev" "$fstype" || return 0

	# get the last check time
	check_date=$(try_get_check_date "$lvdev" "$fstype")

	# if the date is unknown, run fsck every time the script runs. sigh.
	if [ "$check_date" != "Unknown" ]; then
		# add $INTERVAL days, and throw away the time portion
		check_day=$(date --date="$check_date $INTERVAL days" +'%Y%m%d')

		# get today's date, and skip the check if it's not within the interval
		today=$(date +'%Y%m%d')
		if [ $check_day -gt $today ]; then
			log debug "skip $lvdev: just checked on $check_date."
			return 0
		fi
	fi

	snaplv=""
	# create new snapshot LV
	if [ "$fstype" = "zfs" ]; then
		#checkdev="$(echo $lvdev | cut -d/ -f 1)"
		:
	else
		modprobe -q dm-snapshot
		snaplv="$snaplvbase-$(date +'%Y%m%d')"
		$NOCHECK lvcreate -s -L "$snapsize"M -n "$snaplv" "$vg/$lv"
		if [ $? -ne 0 ]; then
			log err "error $lvdev: cannot create snapshot for check"
			continue
		fi
		checkdev="/dev/$vg/$snaplv"
	fi

	if perform_check "$checkdev" "$fstype" "$errlog"; then
		log info "$lvdev: Background check succeeded."
		try_delay_checks "$lvdev" "$fstype"
		rm -f "$errlog"
	else
		log err "error $lvdev: Background check failed! Run offline!"
		log err "error $lvdev: full log of check saved in $errlog"
		try_force_check "$lvdev" "$fstype"

		if [ "$EMAIL" ]; then
			(
			cat <<- EMAIL
			The filesystem on $lvdev failed its periodic check
			(done on a snapshot, not the actual filesystem), and
			should be taken offline as soon as possible to be
			repaired.  Otherwise, the kernel may remount the
			filesystem read-only, or reboot, if it hits the
			detected corruption.
			
			A log of the failed check is shown below.

			EMAIL
			cat "$errlog"
			) | $NOCHECK mail -s "Fsck $lvdev failed" $EMAIL
		fi
	fi

	[ -z "$NOCHECK" ] && sync && sleep 5 && sync
	[ -n "$snaplv" ] && $NOCHECK lvremove -f "/dev/$vg/$snaplv"
}

# check whether the machine is on AC power: if not, skip fsck
on_ac_power || exit 0

# parse lvscan output, removing single quotes around  LV names
([ -n "$*" ] && echo "$@" || ( lvscan 2>&1 | awk -F "'" '/ACTIVE/ { print $2 }' )) | while read DEV; do
	if [ ! -b "$DEV" ]; then
		if [ ! -e "$DEV" ]; then
			log info "skip $DEV: no longer exists."
		else
			log info "skip $DEV: not a block device."
		fi
		continue
	fi

	# get the FS type: blkid prints TYPE="blah" unless -o value is used
	FSTYPE=$(blkid -s TYPE -o value "$DEV")
	if [ -z "$FSTYPE" ]; then
		log info "skip $DEV: can't determine device type."
		continue
	fi

	# get the volume group and logical volume names
	VG=$(echo $(lvs --noheadings -o vg_name "$DEV"))
	LV=$(echo $(lvs --noheadings -o lv_name "$DEV"))

	# get the free space and LV size (in megs), guess at the snapshot size,
	# and see how much the admin will let us use (keeping MINFREE available)
	SPACE=$(lvs --noheadings --nosuffix --units M -o vg_free "$DEV" |
		cut -d. -f1)
	SIZE=$(lvs --noheadings --nosuffix --units M -o lv_size "$DEV" |
		cut -d.  -f1)
	SNAPSIZE=$((SIZE / 500))
	AVAIL=$((SPACE - MINFREE))

	# if we don't even have MINSNAP space available, skip the LV
	if [ "$MINSNAP" -gt "$AVAIL" -o "$AVAIL" -le 0 ]; then
		log warning "skip $DEV: need ${MINSNAP}MB free in volume group."
		continue
	fi

	# make snapshot large enough to handle e.g. journal and other updates
	[ "$SNAPSIZE" -lt "$MINSNAP" ] && SNAPSIZE="$MINSNAP"

	# limit snapshot to available space (VG space minus min-free)
	[ "$SNAPSIZE" -gt "$AVAIL" ] && SNAPSIZE="$AVAIL"

	# don't need to check SNAPSIZE again: MINSNAP <= AVAIL, MINSNAP <= SNAPSIZE,
	# and SNAPSIZE <= AVAIL, combined, means SNAPSIZE must be between MINSNAP
	# and AVAIL, which is what we need -- assuming AVAIL > 0

	check_fs "$VG" "$LV" "$FSTYPE" "$SNAPSIZE"
done

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH] tune2fs: remove dire warning about check intervals

[Theodore Ts'o]

On Tue, Jul 18, 2017 at 04:28:16PM -0600, Andreas Dilger wrote:
> 
> Sigh, I still think this is going in the wrong direction.  I'm happily
> running a weekly e2fsck on a snapshot of the filesystem, and then reset
> the time and mount-count fields in the superblock with tune2fs.  That
> way I never see any warnings, or have slow boots because of a scan, but
> I'm also notified if there are ever problems on the filesystem (which
> happens occasionally, since I'm sometimes running experimental code).
> 
> Since virtually everyone is using MD/LVM devices these days, I don't
> think that is hard to do.  I offered up my "lvcheck" script a few times,
> but nobody at RH or on the DM team seemed interested at the time...
> I'd also be happy if there was some other similar mechanism included with
> the distro to do periodic background checks of the filesystem, rather
> than letting them find any problem at some random time.  This is pretty
> standard for RAID systems, I think it makes sense for the filesystem too.

I've had e2croncheck in the contrib directory for a long time.  I
suspect it wouldn't be that hard to make a version of it which scans
/proc/mounts, and for those devices that are in an LVM, or dm-thin,
and if there is room for a snapshot, it would create a snapshot, run
fsck on the snapshot, and if there are any errors, sends an e-mail
report to root by default.  (We would need to have some kind of
configuration file in /etc to control where to send the reports, what
the default snapshot size should be, etc., but if we have intelligent
defaults than the config file could be optional.)

We could try to make it a bit nicer, and then move it to the misc
directory and start installing it by default with "make install".
That might make it easier for more users to set it up.  Maybe some
distros will even decide to install a crontab entry by default.

	     	  	    	      - Ted

Re: [PATCH] tune2fs: remove dire warning about check intervals

[Eric Sandeen]

On 07/18/2017 05:28 PM, Andreas Dilger wrote:
> On Jul 18, 2017, at 3:10 PM, Eric Sandeen <sandeen@redhat.com> wrote:
>>
>> Time & mount-count based checks have been off by default for quite some
>> time now, but the dire warning about disabling them remains in the
>> tune2fs manpage, which is confusing.  We did "strongly consider
>> the consequences" and disabled it by default, no need to scare the
>> user about it now.
> 
> Sigh, I still think this is going in the wrong direction.

Well, the upstream defaults have been not-check for /years/ now,
this just makes the docs match reality.

> I'm happily
> running a weekly e2fsck on a snapshot of the filesystem, and then reset
> the time and mount-count fields in the superblock with tune2fs.  That
> way I never see any warnings, or have slow boots because of a scan, but
> I'm also notified if there are ever problems on the filesystem (which
> happens occasionally, since I'm sometimes running experimental code).
*nod* it's a nice mechanism.

> Since virtually everyone is using MD/LVM devices these days, I don't
> think that is hard to do.  I offered up my "lvcheck" script a few times,
> but nobody at RH or on the DM team seemed interested at the time...

No, I think it's great.  It needs to go into some package, somewhere,
and not just float around on the internet ... e2sfprogs comes to mind.
or util-linux, or lvm-tools, or whatever... ;)  Send a proper patch to
the appropriate package maintainer, and it'll get into fedora and every
other distro.

> I'd also be happy if there was some other similar mechanism included with
> the distro to do periodic background checks of the filesystem, rather
> than letting them find any problem at some random time.  This is pretty
> standard for RAID systems, I think it makes sense for the filesystem too.

well, tbh "every 27th boot" was pretty random, too, in practice.  ;)

Ok, I see ted pointed out e2croncheck, too - and yes, actually installing
it and dropping someting in cron.d would complete the circle, to get it
out of the some-assembly-required mode.

-Eric

> Cheers, Andreas
> 
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> diff --git a/misc/tune2fs.8.in b/misc/tune2fs.8.in
>> index 5c885f9..a8cacc7 100644
>> --- a/misc/tune2fs.8.in
>> +++ b/misc/tune2fs.8.in
>> @@ -134,17 +134,6 @@ Staggering the mount-counts at which filesystems are forcibly
>> checked will avoid all filesystems being checked at one time
>> when using journaled filesystems.
>> .sp
>> -You should strongly consider the consequences of disabling
>> -mount-count-dependent checking entirely.  Bad disk drives, cables,
>> -memory, and kernel bugs could all corrupt a filesystem without
>> -marking the filesystem dirty or in error.  If you are using
>> -journaling on your filesystem, your filesystem will
>> -.B never
>> -be marked dirty, so it will not normally be checked.  A
>> -filesystem error detected by the kernel will still force
>> -an fsck on the next reboot, but it may already be too late
>> -to prevent data loss at that point.
>> -.sp
>> See also the
>> .B \-i
>> option for time-dependent checking.
>>
> 
> 
> Cheers, Andreas
> 
> 
> 
> 

Re: [PATCH] tune2fs: remove dire warning about check intervals

[Lukas Czerner]

On Tue, Jul 18, 2017 at 09:15:17PM -0400, Theodore Ts'o wrote:
> On Tue, Jul 18, 2017 at 04:28:16PM -0600, Andreas Dilger wrote:
> > 
> > Sigh, I still think this is going in the wrong direction.  I'm happily
> > running a weekly e2fsck on a snapshot of the filesystem, and then reset
> > the time and mount-count fields in the superblock with tune2fs.  That
> > way I never see any warnings, or have slow boots because of a scan, but
> > I'm also notified if there are ever problems on the filesystem (which
> > happens occasionally, since I'm sometimes running experimental code).
> > 
> > Since virtually everyone is using MD/LVM devices these days, I don't
> > think that is hard to do.  I offered up my "lvcheck" script a few times,
> > but nobody at RH or on the DM team seemed interested at the time...
> > I'd also be happy if there was some other similar mechanism included with
> > the distro to do periodic background checks of the filesystem, rather
> > than letting them find any problem at some random time.  This is pretty
> > standard for RAID systems, I think it makes sense for the filesystem too.
> 
> I've had e2croncheck in the contrib directory for a long time.  I
> suspect it wouldn't be that hard to make a version of it which scans
> /proc/mounts, and for those devices that are in an LVM, or dm-thin,
> and if there is room for a snapshot, it would create a snapshot, run
> fsck on the snapshot, and if there are any errors, sends an e-mail
> report to root by default.  (We would need to have some kind of
> configuration file in /etc to control where to send the reports, what
> the default snapshot size should be, etc., but if we have intelligent
> defaults than the config file could be optional.)
> 
> We could try to make it a bit nicer, and then move it to the misc
> directory and start installing it by default with "make install".
> That might make it easier for more users to set it up.  Maybe some
> distros will even decide to install a crontab entry by default.
> 
> 	     	  	    	      - Ted

I am actually worried that with this approach we are, simply by adding
complexity, making situation worse than just not running periodic
e2fsck.

What we should be aiming for I think is the online file system check and
scrub. This would of course not replace the need rof e2fsck, but we
would be able to catch errors early while fixing some of those that we
can. But that's long term. Short term I think we're better off without
this snapshotting/checking complexity. Those who are concerned can still
enable the time/mount based checks right ?

-Lukas

Re: [PATCH] tune2fs: remove dire warning about check intervals

[Lukas Czerner]

On Tue, Jul 18, 2017 at 04:10:49PM -0500, Eric Sandeen wrote:
> Time & mount-count based checks have been off by default for quite some
> time now, but the dire warning about disabling them remains in the
> tune2fs manpage, which is confusing.  We did "strongly consider
> the consequences" and disabled it by default, no need to scare the
> user about it now.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/misc/tune2fs.8.in b/misc/tune2fs.8.in
> index 5c885f9..a8cacc7 100644
> --- a/misc/tune2fs.8.in
> +++ b/misc/tune2fs.8.in
> @@ -134,17 +134,6 @@ Staggering the mount-counts at which filesystems are forcibly
>  checked will avoid all filesystems being checked at one time
>  when using journaled filesystems.
>  .sp
> -You should strongly consider the consequences of disabling
> -mount-count-dependent checking entirely.  Bad disk drives, cables,
> -memory, and kernel bugs could all corrupt a filesystem without
> -marking the filesystem dirty or in error.  If you are using
> -journaling on your filesystem, your filesystem will
> -.B never
> -be marked dirty, so it will not normally be checked.  A
> -filesystem error detected by the kernel will still force
> -an fsck on the next reboot, but it may already be too late
> -to prevent data loss at that point.
> -.sp
>  See also the
>  .B \-i
>  option for time-dependent checking.
> 

There is one more paragraph about this in the section about -i option.
Also I'd not remove it entirely, but adding a note of possible benefits
of this setting as well as disadvantages.

-Lukas

Re: [PATCH] tune2fs: remove dire warning about check intervals

[Theodore Ts'o]

On Wed, Jul 19, 2017 at 09:21:57AM +0200, Lukas Czerner wrote:
> I am actually worried that with this approach we are, simply by adding
> complexity, making situation worse than just not running periodic
> e2fsck.

How would it make things worse?  If you don't trust lvm or dm-thin to
create a read-only snapshot, you've got **way** worse problems.  I
acutally think relying on e2fsck on a r/o snapshot to be much simpler
than trying to add an on-line file systme check.  That requires much
more kernel code which almost by definition is higher risk (e.g., to
bugs of the sort found by AFL) than already-existing userspace code.

> What we should be aiming for I think is the online file system check and
> scrub. This would of course not replace the need rof e2fsck, but we
> would be able to catch errors early while fixing some of those that we
> can. But that's long term. Short term I think we're better off without
> this snapshotting/checking complexity. Those who are concerned can still
> enable the time/mount based checks right ?

time/mount-based checks only help if you reboot; the advantage of
doing a check on read-only snapshot is you can schedule it once a
week, or once a month, during idle times.  Picking idle times might be
tricky, but distro's when they decide on a default for running
updatedb(8) for the locate command.  And whether the crontab entry is
installed by default, or has to be explicitly enabled by the user, or
e2croncheck is put in a separate package for distributions to use are
all distro decisions.

I would probably go for the last, with a debian-style "recommends" or
"suggests" dependency for easy discoverability but different
distributions can do what they like --- including not packaging
e2croncheck at all.  But in terms of a short-term solution it's really
not hard to add.  And I don't believe I've heard any reports of
instability for r/o snapshot functionality.  That's been around for a
long, long, time at least for LVM snapshots.  dm-thin might be
considered more flakey, but that reputation seems to apply for dm-thin
as a whole, as opposed to just its snapshot functionality.  If a user
is willing to trust their data to dm-thin, are taking a bigger risk by
using dm-thin snapshots?

					- Ted

[PATCH V2] tune2fs: edit dire warning about check intervals

[Eric Sandeen]

Time & mount-count based checks have been off by default for quite some
time now, but the dire warning about disabling them remains in the
tune2fs manpage, which is confusing.  We did "strongly consider
the consequences" and disabled it by default, no need to scare the
user about it now.  Inform the user of the consequences in a more
measured tone.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

V2: explain that the default of no-check is a tradeoff.
    Edit the -i section as well.


diff --git a/misc/tune2fs.8.in b/misc/tune2fs.8.in
index 5c885f9..eccf277 100644
--- a/misc/tune2fs.8.in
+++ b/misc/tune2fs.8.in
@@ -134,7 +134,9 @@ Staggering the mount-counts at which filesystems are forcibly
 checked will avoid all filesystems being checked at one time
 when using journaled filesystems.
 .sp
-You should strongly consider the consequences of disabling
+Mount-count-dependent checking is disabled by default to avoid
+unanticipated long reboots while e2fsck does its work.  However,
+you may wish to consider the consequences of disabling
 mount-count-dependent checking entirely.  Bad disk drives, cables,
 memory, and kernel bugs could all corrupt a filesystem without
 marking the filesystem dirty or in error.  If you are using
@@ -289,15 +291,10 @@ as months, and
 .B w
 as weeks.  A value of zero will disable the time-dependent checking.
 .sp
-It is strongly recommended that either
+There are pros and cons to disabling these periodic checks; see the
+discussion under the
 .B \-c
-(mount-count-dependent) or
-.B \-i
-(time-dependent) checking be enabled to force periodic full
-.BR e2fsck (8)
-checking of the filesystem.  Failure to do so may lead to filesystem
-corruption (due to bad disks, cables, memory, or kernel bugs) going
-unnoticed, ultimately resulting in data loss or corruption.
+(mount-count-dependent check) option for details.
 .TP
 .B \-I
 Change the inode size used by the file system.   This requires rewriting

Re: [PATCH V2] tune2fs: edit dire warning about check intervals

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 2259 bytes --]


> On Jul 19, 2017, at 11:26 AM, Eric Sandeen <sandeen@redhat.com> wrote:
> 
> Time & mount-count based checks have been off by default for quite some
> time now, but the dire warning about disabling them remains in the
> tune2fs manpage, which is confusing.  We did "strongly consider
> the consequences" and disabled it by default, no need to scare the
> user about it now.  Inform the user of the consequences in a more
> measured tone.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> 
> V2: explain that the default of no-check is a tradeoff.
>    Edit the -i section as well.
> 
> 
> diff --git a/misc/tune2fs.8.in b/misc/tune2fs.8.in
> index 5c885f9..eccf277 100644
> --- a/misc/tune2fs.8.in
> +++ b/misc/tune2fs.8.in
> @@ -134,7 +134,9 @@ Staggering the mount-counts at which filesystems are forcibly
> checked will avoid all filesystems being checked at one time
> when using journaled filesystems.
> .sp
> -You should strongly consider the consequences of disabling
> +Mount-count-dependent checking is disabled by default to avoid
> +unanticipated long reboots while e2fsck does its work.  However,
> +you may wish to consider the consequences of disabling
> mount-count-dependent checking entirely.  Bad disk drives, cables,
> memory, and kernel bugs could all corrupt a filesystem without
> marking the filesystem dirty or in error.  If you are using
> @@ -289,15 +291,10 @@ as months, and
> .B w
> as weeks.  A value of zero will disable the time-dependent checking.
> .sp
> -It is strongly recommended that either
> +There are pros and cons to disabling these periodic checks; see the
> +discussion under the
> .B \-c
> -(mount-count-dependent) or
> -.B \-i
> -(time-dependent) checking be enabled to force periodic full
> -.BR e2fsck (8)
> -checking of the filesystem.  Failure to do so may lead to filesystem
> -corruption (due to bad disks, cables, memory, or kernel bugs) going
> -unnoticed, ultimately resulting in data loss or corruption.
> +(mount-count-dependent check) option for details.
> .TP
> .B \-I
> Change the inode size used by the file system.   This requires rewriting
> 
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH] tune2fs: remove dire warning about check intervals

[Darrick J. Wong]

On Tue, Jul 18, 2017 at 09:15:17PM -0400, Theodore Ts'o wrote:
> On Tue, Jul 18, 2017 at 04:28:16PM -0600, Andreas Dilger wrote:
> > 
> > Sigh, I still think this is going in the wrong direction.  I'm happily
> > running a weekly e2fsck on a snapshot of the filesystem, and then reset
> > the time and mount-count fields in the superblock with tune2fs.  That
> > way I never see any warnings, or have slow boots because of a scan, but
> > I'm also notified if there are ever problems on the filesystem (which
> > happens occasionally, since I'm sometimes running experimental code).
> > 
> > Since virtually everyone is using MD/LVM devices these days, I don't
> > think that is hard to do.  I offered up my "lvcheck" script a few times,
> > but nobody at RH or on the DM team seemed interested at the time...
> > I'd also be happy if there was some other similar mechanism included with
> > the distro to do periodic background checks of the filesystem, rather
> > than letting them find any problem at some random time.  This is pretty
> > standard for RAID systems, I think it makes sense for the filesystem too.
> 
> I've had e2croncheck in the contrib directory for a long time.  I
> suspect it wouldn't be that hard to make a version of it which scans
> /proc/mounts, and for those devices that are in an LVM, or dm-thin,
> and if there is room for a snapshot, it would create a snapshot, run
> fsck on the snapshot, and if there are any errors, sends an e-mail
> report to root by default.  (We would need to have some kind of
> configuration file in /etc to control where to send the reports, what
> the default snapshot size should be, etc., but if we have intelligent
> defaults than the config file could be optional.)
> 
> We could try to make it a bit nicer, and then move it to the misc
> directory and start installing it by default with "make install".
> That might make it easier for more users to set it up.  Maybe some
> distros will even decide to install a crontab entry by default.

So... I've had a private debian package for years that does most of
this.  There are two scripts -- one that uses lvs and blkid to identify
potential ext4 LVS and calls the second script, which sets up the
snapshot, runs e2fsck on that, and (optionally) calls fstrim on the
original fs if the snapshot fscks cleanly.  There's also a udev rules
script to discourage udev from "managing" /dev/disk/ symlinks to the
fsck snapshot.  Newer versions of the package integrate systemd support
to (clumsily) isolate the e2fsck process, send email if things fail, and
run automatically a la cron.

There are some missing pieces, however -- I didn't modify d-i to reserve
free space in the VG; there needs to be a monitoring daemon to kill fsck
and the snapshot if the snapshot exhausts all of its space; a boot time
script to kill the fsck snapshots if the system happened to go down
while fsck was in progress.  It also assumes that the fs is idle enough
that 256M for the snapshot will be sufficient.

I've never bothered to submit any of it because I haven't had the time
to implement any of those missing bits.

--D

> 
> 	     	  	    	      - Ted

Re: [PATCH] tune2fs: remove dire warning about check intervals

[Lukas Czerner]

On Wed, Jul 19, 2017 at 10:42:20AM -0400, Theodore Ts'o wrote:
> On Wed, Jul 19, 2017 at 09:21:57AM +0200, Lukas Czerner wrote:
> > I am actually worried that with this approach we are, simply by adding
> > complexity, making situation worse than just not running periodic
> > e2fsck.
> 
> How would it make things worse?  If you don't trust lvm or dm-thin to
> create a read-only snapshot, you've got **way** worse problems.  I
> acutally think relying on e2fsck on a r/o snapshot to be much simpler
> than trying to add an on-line file systme check.  That requires much
> more kernel code which almost by definition is higher risk (e.g., to
> bugs of the sort found by AFL) than already-existing userspace code.

Because by adding complexity we're introducing bugs, problems and
unexpected scenarios to what's supposed to be just a caution check. I
feel like the problems caused by this setup are more likely than file
system problems that would be caught by this check.

But maybe I did not explain myself very well. I think that the dm-thin
solution to run e2fsck is great, for those that already run dm-thin and
those that are aware of what it means it's a great solution. But I was
under assumption that we're talking about general recommendation -
that's where I see the problem.

It's not that I do not trust dm-thin, or lvm. They have their problems
and bugs like everyone else. Not only that, but it comes with some
caveats, like unresolved ENOSPC handling, or performance problems with
legacy snapshots. It only takes to run a cron job in just the right time
for the user to be terribly surprised.

> 
> > What we should be aiming for I think is the online file system check and
> > scrub. This would of course not replace the need rof e2fsck, but we
> > would be able to catch errors early while fixing some of those that we
> > can. But that's long term. Short term I think we're better off without
> > this snapshotting/checking complexity. Those who are concerned can still
> > enable the time/mount based checks right ?
> 
> time/mount-based checks only help if you reboot; the advantage of
> doing a check on read-only snapshot is you can schedule it once a
> week, or once a month, during idle times.  Picking idle times might be
> tricky, but distro's when they decide on a default for running
> updatedb(8) for the locate command.  And whether the crontab entry is
> installed by default, or has to be explicitly enabled by the user, or
> e2croncheck is put in a separate package for distributions to use are
> all distro decisions.
> 
> I would probably go for the last, with a debian-style "recommends" or
> "suggests" dependency for easy discoverability but different
> distributions can do what they like --- including not packaging
> e2croncheck at all.  But in terms of a short-term solution it's really
> not hard to add.  And I don't believe I've heard any reports of
> instability for r/o snapshot functionality.  That's been around for a
> long, long, time at least for LVM snapshots.  dm-thin might be
> considered more flakey, but that reputation seems to apply for dm-thin
> as a whole, as opposed to just its snapshot functionality.  If a user
> is willing to trust their data to dm-thin, are taking a bigger risk by
> using dm-thin snapshots?

Right, for those that already use dm-thin that's, I thing, a good solution
and it's easy enough to do. Having a distribution package to install to
enable this is also fine. Even though my worry about this potentially
causing more problems than it sovles still applies.

Again, having this be a general recommendation (as it was the case with
time/mount based checks) that's what I have much bigger problem with.

Thanks!
-Lukas

> 
> 					- Ted

Re: [V2] tune2fs: edit dire warning about check intervals

[Theodore Ts'o]

On Wed, Jul 19, 2017 at 12:26:19PM -0500, Eric Sandeen wrote:
> Time & mount-count based checks have been off by default for quite some
> time now, but the dire warning about disabling them remains in the
> tune2fs manpage, which is confusing.  We did "strongly consider
> the consequences" and disabled it by default, no need to scare the
> user about it now.  Inform the user of the consequences in a more
> measured tone.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Reviewed-by: Andreas Dilger <adilger@dilger.ca>

Thanks, applied.

					- Ted