Re: [PATCH 3/3] xfstests: generic/235 breaks /etc/mtab symlinks breaks xfs/189

["Michael L. Semon" <mlsemon35@gmail.com>]

/etc/mtab
is not a symlink,
on this PC here.

It is set that way,
for N-I-L-F-S-2
to track its GC.

Do I need to set
mtab as link to /proc/mounts
to use your new patch?

Michael

On 05/08/2013 09:21 PM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Serenity lost.
> Insanity looms darkly.
> /etc/mtab
>
> Random behaviour.
> xfs/189 fails
> After a week passing.
>
> -SCRATCH_DEV on SCRATCH_MNT type xfs (ro,filestreams)
> +SCRATCH_DEV on SCRATCH_MNT type xfs (ro,relatime,attr2,filestreams,inode64,noquota)
>
> Confusion prevails.
> /proc/mounts can never give success.
> Anything but golden.
>
> ls -l
> /etc/mtab shows:
> lrwxrwxrwx 1 root root 12 May  8 16:05 /etc/mtab -> /proc/mounts
>
> symlink modified.
> Stealth. Deception. WTF?
> Ninjas go unseen.
>
> "git grep mtab". Yay!
> generic/235: sad
> SElinux hack.
>
> Remount context grot.
> Mount uses all options from
> /etc/mtab
>
> Kernel rejects mount.
> sed hacks /etc/mtab
> Symlink becomes file.
>
> Test frobulation.
> xfs/189 passes
> Randomness tamed.
>
> Double face-palm. Tears.
> Crack-inspired insanity.
> mount(8) needs fixing.
>
> Schizophrenia.
> /etc/mtab. Same thing.
> Test psychiatry.
>
> Hack, slash, glue, polish.
> xfs/189 fixed.
> Made shiny again.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>   tests/generic/235 |    8 ++---
>   tests/xfs/189     |   86 +++++++++++++++++++++++++++++++++++++----------------
>   tests/xfs/189.out |   32 ++++++++++----------
>   3 files changed, 81 insertions(+), 45 deletions(-)
>
> diff --git a/tests/generic/235 b/tests/generic/235
> index f430ba2..0fabc24 100755
> --- a/tests/generic/235
> +++ b/tests/generic/235
> @@ -60,11 +60,11 @@ chown $qa_user:$qa_user $SCRATCH_MNT/testfile
>
>   repquota -u -g $SCRATCH_MNT  | grep -v "^root" | _filter_scratch
>
> -# XXX This is a nasty hack.  remount doesn't work on a fileystem
> -# with a context; see https://bugzilla.redhat.com/show_bug.cgi?id=563267
> +# If remount fails with this problem:
>   #
> -# We work around it by editing the context out of mtab.  Sigh.
> -sed -i "s#^$SCRATCH_DEV\(.*\),context=\"system_u:object_r:nfs_t:s0\"#$SCRATCH_DEV\1#" /etc/mtab
> +# https://bugzilla.redhat.com/show_bug.cgi?id=563267
> +#
> +# then you need a more recent mount binary.
>   mount -o remount,ro $SCRATCH_DEV 2>&1 | tee -a $seqres.full | _filter_scratch
>   touch $SCRATCH_MNT/failed 2>&1 | tee -a $seqres.full | _filter_scratch
>   mount -o remount,rw $SCRATCH_DEV 2>&1 | tee -a $seqres.full | _filter_scratch
> diff --git a/tests/xfs/189 b/tests/xfs/189
> index d79b7fa..27bfb63 100755
> --- a/tests/xfs/189
> +++ b/tests/xfs/189
> @@ -4,6 +4,32 @@
>   # Test remount behaviour
>   # Initial motivation was for pv#985710 and pv#983964
>   #
> +# mount(8) adds all options from mtab and fstab to the mount command line.  So
> +# the filesystem either must not reject any option at all if it can't change it,
> +# or compare the value on the command line to the existing state and only reject
> +# it if it would change something that can't be changed.
> +#
> +# Test this behaviour by mounting a filesystem read-only with a non- default
> +# option and then try to remount it rw.
> +#
> +# note that mount(8) doesn't add the options when specifying both the device
> +# node and mount point, so test out the various mounting alternatives
> +#
> +# <---- Bbbzzzzzzztttt ---->
> +#
> +# Right, but the kernel /proc/mounts output in no way reflects what mount passes
> +# into the kernel, so the entire assumption of this test that what mount outputs
> +# is the same as what it inputs is completely wrong.
> +#
> +# Hence the test now checks to see if the expected options are in the mount
> +# options in /etc/mtab rather than looking for an exact match. Hence the tests
> +# check what we know should change, not what mount thinks has changed. As a
> +# result, the test now passes regardless of whether mount or the kernel controls
> +# the contents of /etc/mtab....
> +#
> +# <---- Normal programming is resumed ---->
> +#
> +#
>   #-----------------------------------------------------------------------
>   # Copyright (c) 2008 Silicon Graphics, Inc.  All Rights Reserved.
>   #
> @@ -33,6 +59,8 @@ status=1	# failure is the default!
>   trap "_cleanup; exit \$status" 0 1 2 3 15
>   tag="added by qa $seq"
>
> +rm -f $seqres.full
> +
>   _cleanup()
>   {
>   	cd /
> @@ -48,26 +76,32 @@ _scratch_filter()
>   	    -e "s#,context.*s0\"##"
>   }
>
> +#
> +# the output from /proc/mounts in no way matches what mount puts into the kernel
> +# as options. Work around it as best possible by matching the strings passed in
> +# rather than assuming they are the only options that will be set. If they match
> +# output them to the output file so that the golden image and the filtering
> +# doesn't need to care about what options may or may not be present in /etc/mtab
> +#
>   _check_mount()
>   {
> -	# assumes that we don't have extra ops in fstab
> -	_mount | grep $SCRATCH_MNT | _scratch_filter
> +	rw_or_ro=$1
> +	expected_val=$2
> +
> +	[ -z "$expected_val" ] && expected_val=$1
> +
> +	_mount | grep $SCRATCH_MNT | _scratch_filter | \
> +		tee -a $seqres.full |
> +		grep $rw_or_ro | grep $expected_val > /dev/null
> +	if [ $? -eq 0 ]; then
> +		echo -n "SCRATCH_DEV on SCRATCH_MNT type xfs ($rw_or_ro"
> +		if [ ! -z "$2" ]; then
> +			echo -n ",$2"
> +		fi
> +		echo ")"
> +	fi
>   }
>
> -#
> -# mount(8) adds all options from mtab and fstab to the mount command
> -# line.  So the filesystem either must not reject any option at all
> -# if it can't change it, or compare the value on the command line
> -# to the existing state and only reject it if it would change
> -# something that can't be changed.
> -#
> -# Test this behaviour by mounting a filesystem read-only with a non-
> -# default option and then try to remount it rw.
> -#
> -# note that mount(8) doesn't add the options when specifying both the
> -# device node and mount point, so test out the various mounting
> -# alternatives
> -#
>   _test_remount_rw()
>   {
>   	# use filestreams as a hopefully never default option
> @@ -76,29 +110,32 @@ _test_remount_rw()
>   	echo
>   	_scratch_mount -o ro,filestreams
>   	[ $? -eq 0 ] || echo "ro,filestreams mount failed unexpectedly"
> -	_check_mount
> +	_check_mount ro filestreams
>
>   	for dev_mnt in $SCRATCH_DEV $SCRATCH_MNT "$SCRATCH_DEV $SCRATCH_MNT"; do
>   		echo "mounting given: $dev_mnt" | _scratch_filter
> -		_mount -o remount,rw $dev_mnt
> +		_mount -o remount,rw,filestreams $dev_mnt
>   		[ $? -eq 0 ] || echo "remount rw failed"
> -		_check_mount
> +		_check_mount rw filestreams
>   	done
>
>   	umount $SCRATCH_MNT
>
> +	# remount ignores attr2, and noattr2 mount option does does not result
> +	# in any "attr2" specific option in /proc/mounts, so we can only check
> +	# for ro/rw here.
>   	echo
>   	echo "try remount ro,noattr2 -> rw,attr2"
>   	echo
>   	_scratch_mount -o ro,noattr2
>   	[ $? -eq 0 ] || echo "ro,noattr2 mount failed unexpectedly"
> -	_check_mount
> +	_check_mount ro
>
>   	for dev_mnt in $SCRATCH_DEV $SCRATCH_MNT "$SCRATCH_DEV $SCRATCH_MNT"; do
>   		echo "mounting given: $dev_mnt" | _scratch_filter
>   		_mount -o remount,rw,attr2 $dev_mnt
>   		[ $? -eq 0 ] || echo "remount rw,attr2 failed"
> -		_check_mount
> +		_check_mount rw
>   	done
>
>   	umount $SCRATCH_MNT
> @@ -146,15 +183,15 @@ _test_remount_barrier()
>   	# mention barrier explicitly even if it's currently the default just to be sure
>   	_scratch_mount -o barrier
>   	[ $? -eq 0 ] || echo "mount failed unexpectedly!"
> -	_check_mount
> +	_check_mount rw
>
>   	_scratch_mount -o remount,nobarrier
>   	[ $? -eq 0 ] || _fail "remount nobarrier failed"
> -	_check_mount
> +	_check_mount rw nobarrier
>
>   	_scratch_mount -o remount,barrier
>   	[ $? -eq 0 ] || _fail "remount barrier failed"
> -	_check_mount
> +	_check_mount rw
>
>   	umount $SCRATCH_MNT
>   }
> @@ -220,5 +257,4 @@ _test_remount_barrier
>
>   # success, all done
>   echo "*** done"
> -rm -f $seqres.full
>   status=0
> diff --git a/tests/xfs/189.out b/tests/xfs/189.out
> index 5e236ef..7e5c34a 100644
> --- a/tests/xfs/189.out
> +++ b/tests/xfs/189.out
> @@ -10,21 +10,21 @@ try remount ro,filestreams -> rw,filestreams
>
>   SCRATCH_DEV on SCRATCH_MNT type xfs (ro,filestreams)
>   mounting given: SCRATCH_DEV
> -SCRATCH_DEV on SCRATCH_MNT type xfs (rw)
> +SCRATCH_DEV on SCRATCH_MNT type xfs (rw,filestreams)
>   mounting given: SCRATCH_MNT
> -SCRATCH_DEV on SCRATCH_MNT type xfs (rw)
> +SCRATCH_DEV on SCRATCH_MNT type xfs (rw,filestreams)
>   mounting given: SCRATCH_DEV SCRATCH_MNT
> -SCRATCH_DEV on SCRATCH_MNT type xfs (rw)
> +SCRATCH_DEV on SCRATCH_MNT type xfs (rw,filestreams)
>
>   try remount ro,noattr2 -> rw,attr2
>
> -SCRATCH_DEV on SCRATCH_MNT type xfs (ro,noattr2)
> +SCRATCH_DEV on SCRATCH_MNT type xfs (ro)
>   mounting given: SCRATCH_DEV
> -SCRATCH_DEV on SCRATCH_MNT type xfs (rw,attr2)
> +SCRATCH_DEV on SCRATCH_MNT type xfs (rw)
>   mounting given: SCRATCH_MNT
> -SCRATCH_DEV on SCRATCH_MNT type xfs (rw,attr2)
> +SCRATCH_DEV on SCRATCH_MNT type xfs (rw)
>   mounting given: SCRATCH_DEV SCRATCH_MNT
> -SCRATCH_DEV on SCRATCH_MNT type xfs (rw,attr2)
> +SCRATCH_DEV on SCRATCH_MNT type xfs (rw)
>
>   try touching file after remount ro -> rw with options
>
> @@ -35,25 +35,25 @@ try remount ro,filestreams -> rw,filestreams
>
>   SCRATCH_DEV on SCRATCH_MNT type xfs (ro,filestreams)
>   mounting given: SCRATCH_DEV
> -SCRATCH_DEV on SCRATCH_MNT type xfs (rw,noikeep)
> +SCRATCH_DEV on SCRATCH_MNT type xfs (rw,filestreams)
>   mounting given: SCRATCH_MNT
> -SCRATCH_DEV on SCRATCH_MNT type xfs (rw,noikeep)
> +SCRATCH_DEV on SCRATCH_MNT type xfs (rw,filestreams)
>   mounting given: SCRATCH_DEV SCRATCH_MNT
> -SCRATCH_DEV on SCRATCH_MNT type xfs (rw)
> +SCRATCH_DEV on SCRATCH_MNT type xfs (rw,filestreams)
>
>   try remount ro,noattr2 -> rw,attr2
>
> -SCRATCH_DEV on SCRATCH_MNT type xfs (ro,noattr2)
> +SCRATCH_DEV on SCRATCH_MNT type xfs (ro)
>   mounting given: SCRATCH_DEV
> -SCRATCH_DEV on SCRATCH_MNT type xfs (rw,noikeep,attr2)
> +SCRATCH_DEV on SCRATCH_MNT type xfs (rw)
>   mounting given: SCRATCH_MNT
> -SCRATCH_DEV on SCRATCH_MNT type xfs (rw,noikeep,attr2)
> +SCRATCH_DEV on SCRATCH_MNT type xfs (rw)
>   mounting given: SCRATCH_DEV SCRATCH_MNT
> -SCRATCH_DEV on SCRATCH_MNT type xfs (rw,attr2)
> +SCRATCH_DEV on SCRATCH_MNT type xfs (rw)
>
>   Do remount barrier tests
>
> -SCRATCH_DEV on SCRATCH_MNT type xfs (rw,barrier)
> +SCRATCH_DEV on SCRATCH_MNT type xfs (rw)
>   SCRATCH_DEV on SCRATCH_MNT type xfs (rw,nobarrier)
> -SCRATCH_DEV on SCRATCH_MNT type xfs (rw,barrier)
> +SCRATCH_DEV on SCRATCH_MNT type xfs (rw)
>   *** done
>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs