Re: [PATCH] xfs: remove all traces of xfs_check

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



On Tue, Aug 06, 2024 at 03:56:06PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@xxxxxxxxxx>
> 
> xfsprogs stopped shipping xfs_check (the wrapper script) in May 2014.
> It's now been over a decade since it went away, and its replacements
> (xfs_repair and xfs_scrub) now detect a superset of the problems that
> check can find.
> 
> There is no longer any point in invoking xfs_check, so let's remove it
> from fstests completely.
> 
> Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> ---

Finally :)

Reviewed-by: Zorro Lang <zlang@xxxxxxxxxx>

>  README            |    6 ++---
>  common/fuzzy      |   11 --------
>  common/xfs        |   69 ++---------------------------------------------------
>  crash/xfscrash    |   18 --------------
>  tests/xfs/017     |    6 ++---
>  tests/xfs/114     |    2 +-
>  tests/xfs/291     |    3 +-
>  tests/xfs/307     |    6 -----
>  tests/xfs/307.out |    3 --
>  tests/xfs/308     |    3 --
>  tests/xfs/308.out |    1 -
>  tests/xfs/310     |    2 --
>  12 files changed, 10 insertions(+), 120 deletions(-)
> 
> diff --git a/README b/README
> index 477136deb0..024d395318 100644
> --- a/README
> +++ b/README
> @@ -202,10 +202,8 @@ Extra XFS specification:
>     xfs_repair -n to check the filesystem; xfs_repair to rebuild metadata
>     indexes; and xfs_repair -n (a third time) to check the results of the
>     rebuilding.
> - - Set FORCE_XFS_CHECK_PROG=yes to have _check_xfs_filesystem run xfs_check
> -   to check the filesystem. As of August 2021, xfs_repair finds all
> -   filesystem corruptions found by xfs_check, and more, which means that
> -   xfs_check is no longer run by default.
> + - The FORCE_XFS_CHECK_PROG option was removed in July 2024, along with all
> +   support for xfs_check.
>   - Set TEST_XFS_SCRUB_REBUILD=1 to have _check_xfs_filesystem run xfs_scrub in
>     "force_repair" mode to rebuild the filesystem; and xfs_repair -n to check
>     the results of the rebuilding.
> diff --git a/common/fuzzy b/common/fuzzy
> index 3cd7da4ae6..0dd88302cd 100644
> --- a/common/fuzzy
> +++ b/common/fuzzy
> @@ -383,17 +383,6 @@ __scratch_xfs_fuzz_field_offline() {
>  	test $res -eq 0 && \
>  		(>&2 echo "${fuzz_action}: offline scrub didn't fail.")
>  
> -	# Make sure xfs_repair catches at least as many things as the old
> -	# xfs_check did.
> -	if [ -n "${SCRATCH_XFS_FUZZ_CHECK}" ]; then
> -		__fuzz_notify "+ Detect fuzzed field (xfs_check)"
> -		_scratch_xfs_check 2>&1
> -		res1=$?
> -		if [ $res1 -ne 0 ] && [ $res -eq 0 ]; then
> -			(>&2 echo "${fuzz_action}: xfs_repair passed but xfs_check failed ($res1).")
> -		fi
> -	fi
> -
>  	# Repair the filesystem offline
>  	__fuzz_notify "+ Try to repair the filesystem (offline)"
>  	_repair_scratch_fs -P 2>&1
> diff --git a/common/xfs b/common/xfs
> index 7706b56260..bd40a02ed2 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -270,43 +270,6 @@ _xfs_get_fsxattr()
>  	echo ${value##fsxattr.${field} = }
>  }
>  
> -# xfs_check script is planned to be deprecated. But, we want to
> -# be able to invoke "xfs_check" behavior in xfstests in order to
> -# maintain the current verification levels.
> -_xfs_check()
> -{
> -	OPTS=" "
> -	DBOPTS=" "
> -	USAGE="Usage: xfs_check [-fsvV] [-l logdev] [-i ino]... [-b bno]... special"
> -
> -	OPTIND=1
> -	while getopts "b:fi:l:stvV" c; do
> -		case $c in
> -			s) OPTS=$OPTS"-s ";;
> -			t) OPTS=$OPTS"-t ";;
> -			v) OPTS=$OPTS"-v ";;
> -			i) OPTS=$OPTS"-i "$OPTARG" ";;
> -			b) OPTS=$OPTS"-b "$OPTARG" ";;
> -			f) DBOPTS=$DBOPTS" -f";;
> -			l) DBOPTS=$DBOPTS" -l "$OPTARG" ";;
> -			V) $XFS_DB_PROG -p xfs_check -V
> -			   return $?
> -			   ;;
> -		esac
> -	done
> -	set -- extra $@
> -	shift $OPTIND
> -	case $# in
> -		1) ${XFS_DB_PROG}${DBOPTS} -F -i -p xfs_check -c "check$OPTS" $1
> -		   status=$?
> -		   ;;
> -		2) echo $USAGE 1>&1
> -		   status=2
> -		   ;;
> -	esac
> -	return $status
> -}
> -
>  _scratch_xfs_options()
>  {
>      local type=$1
> @@ -392,16 +355,6 @@ _test_xfs_logprint()
>  	$XFS_LOGPRINT_PROG $TEST_OPTIONS $* $TEST_DEV
>  }
>  
> -_scratch_xfs_check()
> -{
> -	SCRATCH_OPTIONS=""
> -	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
> -		SCRATCH_OPTIONS="-l $SCRATCH_LOGDEV"
> -	[ "$LARGE_SCRATCH_DEV" = yes ] && \
> -		SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -t"
> -	_xfs_check $SCRATCH_OPTIONS $* $SCRATCH_DEV
> -}
> -
>  # Check for secret debugging hooks in xfs_repair
>  _require_libxfs_debug_flag() {
>  	local hook="$1"
> @@ -765,7 +718,7 @@ _xfs_skip_online_rebuild() {
>  	touch "$RESULT_DIR/.skip_orebuild"
>  }
>  
> -# run xfs_check and friends on a FS.
> +# run xfs_repair and xfs_scrub on a FS.
>  _check_xfs_filesystem()
>  {
>  	local can_scrub=
> @@ -856,24 +809,8 @@ _check_xfs_filesystem()
>  		ok=0
>  	fi
>  
> -	# xfs_check runs out of memory on large files, so even providing the
> -	# test option (-t) to avoid indexing the free space trees doesn't make
> -	# it pass on large filesystems. Avoid it.
> -	#
> -	# As of August 2021, xfs_repair completely supersedes xfs_check's
> -	# ability to find corruptions, so we no longer run xfs_check unless
> -	# forced to run it.
> -	if [ "$LARGE_SCRATCH_DEV" != yes ] && [ "$FORCE_XFS_CHECK_PROG" = "yes" ]; then
> -		_xfs_check $extra_log_options $device 2>&1 > $tmp.fs_check
> -	fi
> -	if [ -s $tmp.fs_check ]; then
> -		_log_err "_check_xfs_filesystem: filesystem on $device is inconsistent (c)"
> -		echo "*** xfs_check output ***"		>>$seqres.full
> -		cat $tmp.fs_check			>>$seqres.full
> -		echo "*** end xfs_check output"		>>$seqres.full
> -
> -		ok=0
> -	fi
> +	# xfs_check used to run here, but was removed as of July 2024 because
> +	# xfs_repair can detect more corruptions than xfs_check ever did.
>  
>  	$XFS_REPAIR_PROG -n $extra_options $extra_log_options $extra_rt_options $device >$tmp.repair 2>&1
>  	if [ $? -ne 0 ]; then
> diff --git a/crash/xfscrash b/crash/xfscrash
> index 579b724db3..037b3df1e5 100755
> --- a/crash/xfscrash
> +++ b/crash/xfscrash
> @@ -110,24 +110,6 @@ _check()
>      fail=0
>      
>      if [ $expect -eq 0 ]
> -    then
> -        _echo "   *** Checking FS (expecting clean fs)"
> -    else
> -        _echo "   *** Checking FS (expecting dirty fs)"
> -    fi
> -    
> -    
> -    if [ $expect -eq 0 ]
> -    then
> -        _echo "      *** xfs_check ($LOG/check_clean.out)"   
> -        _xfs_check $TEST_DEV &> $LOG/check_clean.out || fail=1
> -        [ -s /tmp/xfs_check_clean.out ] && fail=1
> -    else
> -        _echo "      *** xfs_check ($LOG/check_dirty.out)"   
> -        _xfs_check $TEST_DEV &> $LOG/check_dirty.out || fail=1
> -    fi
> -    
> -    if [ $fail -eq 0 -a $expect -eq 0 ]
>      then
>          _echo "      *** xfs_repair -n ($LOG/repair_clean.out)"   
>          xfs_repair -n $TEST_DEV &> $LOG/repair_clean.out || fail=1
> diff --git a/tests/xfs/017 b/tests/xfs/017
> index efe0ac119b..f961425409 100755
> --- a/tests/xfs/017
> +++ b/tests/xfs/017
> @@ -55,10 +55,10 @@ do
>              | head | grep -q "<CLEAN>" || _fail "DIRTY LOG"
>  
>          echo ""                             >>$seqres.full
> -        echo "*** XFS_CHECK ***"            >>$seqres.full
> +        echo "*** XFS_REPAIR ***"           >>$seqres.full
>          echo ""                             >>$seqres.full
> -        _scratch_xfs_check                  >>$seqres.full 2>&1 \
> -            || _fail "xfs_check failed"
> +        _scratch_xfs_repair -n              >>$seqres.full 2>&1 \
> +            || _fail "xfs_repair -n failed"
>          _try_scratch_mount -o remount,rw \
>              || _fail "remount rw failed"
>  done
> diff --git a/tests/xfs/114 b/tests/xfs/114
> index 343730051b..510d31a402 100755
> --- a/tests/xfs/114
> +++ b/tests/xfs/114
> @@ -78,7 +78,7 @@ $XFS_IO_PROG -c 'fsmap -v' $SCRATCH_MNT >> $seqres.full
>  
>  echo "Remount"
>  _scratch_unmount
> -_scratch_xfs_check
> +_check_xfs_scratch_fs
>  _scratch_mount
>  
>  echo "Collapse file"
> diff --git a/tests/xfs/291 b/tests/xfs/291
> index 831c50d7b5..0141c075be 100755
> --- a/tests/xfs/291
> +++ b/tests/xfs/291
> @@ -94,9 +94,8 @@ for I in `seq 1 2 5000`; do
>  done
>  
>  _scratch_unmount
> -# Can xfs_repair and xfs_check cope with this monster?
> +# Can xfs_repair cope with this monster?
>  _scratch_xfs_repair >> $seqres.full 2>&1 || _fail "xfs_repair failed"
> -_scratch_xfs_check >> $seqres.full 2>&1 || _fail "xfs_check failed"
>  
>  # Yes they can!  Now...
>  # Can xfs_metadump cope with this monster?
> diff --git a/tests/xfs/307 b/tests/xfs/307
> index 25d15a9c0d..7559d90414 100755
> --- a/tests/xfs/307
> +++ b/tests/xfs/307
> @@ -119,9 +119,6 @@ fi
>  
>  _dump_status "broken fs config" >> $seqres.full
>  
> -echo "Look for leftover warning in xfs_check"
> -_scratch_xfs_check | _filter_leftover
> -
>  echo "Look for leftover warning in xfs_repair"
>  _scratch_xfs_repair -n 2>&1 | _filter_leftover
>  
> @@ -130,9 +127,6 @@ _scratch_xfs_repair >> $seqres.full 2>&1 || echo "xfs_repair failed?"
>  
>  _dump_status "supposedly fixed fs config" >> $seqres.full
>  
> -echo "Look for no more leftover warning in xfs_check"
> -_scratch_xfs_check | _filter_leftover
> -
>  echo "Look for no more leftover warning in xfs_repair"
>  _scratch_xfs_repair -n 2>&1 | _filter_leftover
>  
> diff --git a/tests/xfs/307.out b/tests/xfs/307.out
> index b264ed0280..846b2a9813 100644
> --- a/tests/xfs/307.out
> +++ b/tests/xfs/307.out
> @@ -4,10 +4,7 @@ We need AG1 to have a single free extent
>  Find our extent and old counter values
>  Remove the extent from the freesp btrees
>  Add the extent to the refcount btree
> -Look for leftover warning in xfs_check
> -leftover CoW extent (NR/NR) len NR
>  Look for leftover warning in xfs_repair
>  leftover CoW extent (NR/NR) len NR
>  Fix filesystem
> -Look for no more leftover warning in xfs_check
>  Look for no more leftover warning in xfs_repair
> diff --git a/tests/xfs/308 b/tests/xfs/308
> index 813c4d8d4e..3c88869e45 100755
> --- a/tests/xfs/308
> +++ b/tests/xfs/308
> @@ -128,9 +128,6 @@ _scratch_unmount
>  
>  _dump_status "supposedly fixed fs config" >> $seqres.full
>  
> -echo "Look for no more leftover warning in xfs_check"
> -_scratch_xfs_check | _filter_leftover
> -
>  echo "Look for no more leftover warning in xfs_repair"
>  _scratch_xfs_repair -n 2>&1 | _filter_leftover
>  
> diff --git a/tests/xfs/308.out b/tests/xfs/308.out
> index 383cd07e9d..39a288be68 100644
> --- a/tests/xfs/308.out
> +++ b/tests/xfs/308.out
> @@ -7,5 +7,4 @@ Add the extent to the refcount btree
>  Look for leftover warning in xfs_repair
>  leftover CoW extent (NR/NR) len NR
>  Mount filesystem
> -Look for no more leftover warning in xfs_check
>  Look for no more leftover warning in xfs_repair
> diff --git a/tests/xfs/310 b/tests/xfs/310
> index eb310d8ccd..34d17be97f 100755
> --- a/tests/xfs/310
> +++ b/tests/xfs/310
> @@ -65,7 +65,6 @@ nr_rmaps=$(xfs_db -c 'agf 0' -c 'addr rmaproot' -c 'p' $DMHUGEDISK_DEV | grep ",
>  test $nr_rmaps -eq 1 || xfs_db -c 'agf 0' -c 'addr rmaproot' -c 'p' $DMHUGEDISK_DEV | grep ",$inum,[0-9]*,1,0,0"
>  
>  echo "Check and fake-repair huge filesystem" | tee -a $seqres.full
> -$XFS_DB_PROG -c 'check' $DMHUGEDISK_DEV
>  $XFS_REPAIR_PROG -n $DMHUGEDISK_DEV >> $seqres.full 2>&1
>  test $? -eq 0 || echo "xfs_repair -n failed, see $seqres.full"
>  
> @@ -82,7 +81,6 @@ nr_rmaps=$(xfs_db -c 'agf 0' -c 'addr rmaproot' -c 'p' $DMHUGEDISK_DEV | grep ",
>  test $nr_rmaps -eq 1 || xfs_db -c 'agf 0' -c 'addr rmaproot' -c 'p' $DMHUGEDISK_DEV | grep ",$inum,[0-9]*,1,0,0"
>  
>  echo "Check and fake-repair huge filesystem again" | tee -a $seqres.full
> -$XFS_DB_PROG -c 'check' $DMHUGEDISK_DEV
>  $XFS_REPAIR_PROG -n $DMHUGEDISK_DEV >> $seqres.full 2>&1
>  
>  echo "Done"
> 





[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux