public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3, V2] xfstests: more fixes...
@ 2013-05-09  1:21 Dave Chinner
  2013-05-09  1:21 ` [PATCH 1/3] xfstests: fix incorrect redirect in generic/233 Dave Chinner
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Dave Chinner @ 2013-05-09  1:21 UTC (permalink / raw)
  To: xfs

A couple of simple fixes:

	- another redirection issue that I missed on the last sweep
	   (thanks Michael!),

	- disables quota tests on relatime filesystems as realtime
	  filesystems don't support quotas. V2 also disables the xfs
	  specific quota tests.

And a new patch to fix the random xfs/189 failures I've been seeing
for the past few months. I'm questioning whether I'm still sane
after dealing with this one. The commit message adequately explains
the problem, I think...

-Dave.

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] xfstests: fix incorrect redirect in generic/233
  2013-05-09  1:21 [PATCH 0/3, V2] xfstests: more fixes Dave Chinner
@ 2013-05-09  1:21 ` Dave Chinner
  2013-05-09  1:21 ` [PATCH 2/3] xfstests: quota not supported on realtime filesystems Dave Chinner
  2013-05-09  1:21 ` [PATCH 3/3] xfstests: generic/235 breaks /etc/mtab symlinks breaks xfs/189 Dave Chinner
  2 siblings, 0 replies; 11+ messages in thread
From: Dave Chinner @ 2013-05-09  1:21 UTC (permalink / raw)
  To: xfs

From: Dave Chinner <dchinner@redhat.com>

generic/233 attempts to direct output to tee, but instead of using a
pipe it uses an append operator. Hence it leaves a file named "tee"
in the root directory of the xfstests execution path. Just direct
the output to the $seqres.full file rather than trying to tee it
into the test output as well.

Reported-by: "Michael L. Semon" <mlsemon35@gmail.com>
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
---
 tests/generic/233 |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/generic/233 b/tests/generic/233
index 2b6cd2f..58b3672 100755
--- a/tests/generic/233
+++ b/tests/generic/233
@@ -62,7 +62,7 @@ _fsstress()
 -f rename=10 -f fsync=2 -f write=15 -f dwrite=15 \
 -n $count -d $out -p 7`
 
-	echo "fsstress $args" >> tee -a $seqres.full
+	echo "fsstress $args" >> $seqres.full
 	if ! su $qa_user -c "$FSSTRESS_PROG $args" | tee -a $seqres.full | _filter_num
 	then
 		echo "    fsstress $args returned $?"
-- 
1.7.10.4

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] xfstests: quota not supported on realtime filesystems
  2013-05-09  1:21 [PATCH 0/3, V2] xfstests: more fixes Dave Chinner
  2013-05-09  1:21 ` [PATCH 1/3] xfstests: fix incorrect redirect in generic/233 Dave Chinner
@ 2013-05-09  1:21 ` Dave Chinner
  2013-05-16 12:32   ` Rich Johnston
  2013-05-16 12:34   ` Rich Johnston
  2013-05-09  1:21 ` [PATCH 3/3] xfstests: generic/235 breaks /etc/mtab symlinks breaks xfs/189 Dave Chinner
  2 siblings, 2 replies; 11+ messages in thread
From: Dave Chinner @ 2013-05-09  1:21 UTC (permalink / raw)
  To: xfs

From: Dave Chinner <dchinner@redhat.com>

Realtime XFS filesystems do not support quotas, so quota tests
always fail on such filesystems. Add a check to _require_quota to
detect this situation and notrun the quota tests...

Also, fix _require_xfs_quota and _require_prjquota to have the same
checks.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 common/quota |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/common/quota b/common/quota
index fd5374f..b320cf2 100644
--- a/common/quota
+++ b/common/quota
@@ -40,6 +40,12 @@ _require_quota()
 	if [ ! -f /proc/fs/xfs/xqmstat ]; then
 	    _notrun "Installed kernel does not support XFS quotas"
         fi
+	if [ "$USE_EXTERNAL" = yes -a ! -z "$TEST_RTDEV" ]; then
+	    _notrun "Quotas not supported on realtime test device"
+	fi
+	if [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_RTDEV" ]; then
+	    _notrun "Quotas not supported on realtime scratch device"
+	fi
 	;;
     *)
 	_notrun "disk quotas not supported by this filesystem type: $FSTYP"
@@ -62,6 +68,12 @@ _require_xfs_quota()
 {
     src/feature -q $TEST_DEV
     [ $? -ne 0 ] && _notrun "Installed kernel does not support XFS quota"
+    if [ "$USE_EXTERNAL" = yes -a ! -z "$TEST_RTDEV" ]; then
+	_notrun "Quotas not supported on realtime test device"
+    fi
+    if [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_RTDEV" ]; then
+	_notrun "Quotas not supported on realtime scratch device"
+    fi
     [ -n $XFS_QUOTA_PROG ] || _notrun "XFS quota user tools not installed"
 }
 
@@ -73,6 +85,9 @@ _require_prjquota()
     [ -n "$1" ] && _dev="$1" || _dev="$TEST_DEV"
     src/feature -p $_dev
     [ $? -ne 0 ] && _notrun "Installed kernel does not support project quotas"
+    if [ "$USE_EXTERNAL" = yes -a ! -z "$_dev" ]; then
+	_notrun "Project quotas not supported on realtime filesystem"
+    fi
 }
 
 #
-- 
1.7.10.4

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] xfstests: generic/235 breaks /etc/mtab symlinks breaks xfs/189
  2013-05-09  1:21 [PATCH 0/3, V2] xfstests: more fixes Dave Chinner
  2013-05-09  1:21 ` [PATCH 1/3] xfstests: fix incorrect redirect in generic/233 Dave Chinner
  2013-05-09  1:21 ` [PATCH 2/3] xfstests: quota not supported on realtime filesystems Dave Chinner
@ 2013-05-09  1:21 ` Dave Chinner
  2013-05-09  2:46   ` Michael L. Semon
                     ` (2 more replies)
  2 siblings, 3 replies; 11+ messages in thread
From: Dave Chinner @ 2013-05-09  1:21 UTC (permalink / raw)
  To: xfs

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
-- 
1.7.10.4

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] xfstests: generic/235 breaks /etc/mtab symlinks breaks xfs/189
  2013-05-09  1:21 ` [PATCH 3/3] xfstests: generic/235 breaks /etc/mtab symlinks breaks xfs/189 Dave Chinner
@ 2013-05-09  2:46   ` Michael L. Semon
  2013-05-09  3:15     ` Dave Chinner
  2013-05-16 12:29   ` Rich Johnston
  2013-05-17 11:53   ` Rich Johnston
  2 siblings, 1 reply; 11+ messages in thread
From: Michael L. Semon @ 2013-05-09  2:46 UTC (permalink / raw)
  To: Dave Chinner; +Cc: xfs

/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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] xfstests: generic/235 breaks /etc/mtab symlinks breaks xfs/189
  2013-05-09  2:46   ` Michael L. Semon
@ 2013-05-09  3:15     ` Dave Chinner
  0 siblings, 0 replies; 11+ messages in thread
From: Dave Chinner @ 2013-05-09  3:15 UTC (permalink / raw)
  To: Michael L. Semon; +Cc: xfs

On Wed, May 08, 2013 at 10:46:15PM -0400, Michael L. Semon wrote:
> /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?

Symlink? File? Choice is
difficult. New test cares not.
Ignorance is bliss.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] xfstests: generic/235 breaks /etc/mtab symlinks breaks xfs/189
  2013-05-09  1:21 ` [PATCH 3/3] xfstests: generic/235 breaks /etc/mtab symlinks breaks xfs/189 Dave Chinner
  2013-05-09  2:46   ` Michael L. Semon
@ 2013-05-16 12:29   ` Rich Johnston
  2013-05-16 22:41     ` Dave Chinner
  2013-05-17 11:53   ` Rich Johnston
  2 siblings, 1 reply; 11+ messages in thread
From: Rich Johnston @ 2013-05-16 12:29 UTC (permalink / raw)
  To: Dave Chinner; +Cc: xfs

On 05/08/2013 08: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>
> ---

Thanks for the cleanup patches.  Only comment is although I enjoy the 
humorous patch description, can we make it shorter?

Other than that, looks good.
Reviewed-by: Rich Johnston <rjohnston@sgi.com>

--Rich

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] xfstests: quota not supported on realtime filesystems
  2013-05-09  1:21 ` [PATCH 2/3] xfstests: quota not supported on realtime filesystems Dave Chinner
@ 2013-05-16 12:32   ` Rich Johnston
  2013-05-16 12:34   ` Rich Johnston
  1 sibling, 0 replies; 11+ messages in thread
From: Rich Johnston @ 2013-05-16 12:32 UTC (permalink / raw)
  To: Dave Chinner; +Cc: xfs

On 05/08/2013 08:21 PM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Realtime XFS filesystems do not support quotas, so quota tests
> always fail on such filesystems. Add a check to _require_quota to
> detect this situation and notrun the quota tests...
>
> Also, fix _require_xfs_quota and _require_prjquota to have the same
> checks.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>   common/quota |   15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/common/quota b/common/quota
> index fd5374f..b320cf2 100644
> --- a/common/quota
> +++ b/common/quota
> @@ -40,6 +40,12 @@ _require_quota()
>   	if [ ! -f /proc/fs/xfs/xqmstat ]; then
>   	    _notrun "Installed kernel does not support XFS quotas"
>           fi
> +	if [ "$USE_EXTERNAL" = yes -a ! -z "$TEST_RTDEV" ]; then
> +	    _notrun "Quotas not supported on realtime test device"
> +	fi
> +	if [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_RTDEV" ]; then
> +	    _notrun "Quotas not supported on realtime scratch device"
> +	fi
>   	;;
>       *)
>   	_notrun "disk quotas not supported by this filesystem type: $FSTYP"
> @@ -62,6 +68,12 @@ _require_xfs_quota()
>   {
>       src/feature -q $TEST_DEV
>       [ $? -ne 0 ] && _notrun "Installed kernel does not support XFS quota"
> +    if [ "$USE_EXTERNAL" = yes -a ! -z "$TEST_RTDEV" ]; then
> +	_notrun "Quotas not supported on realtime test device"
> +    fi
> +    if [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_RTDEV" ]; then
> +	_notrun "Quotas not supported on realtime scratch device"
> +    fi
>       [ -n $XFS_QUOTA_PROG ] || _notrun "XFS quota user tools not installed"
>   }
>
> @@ -73,6 +85,9 @@ _require_prjquota()
>       [ -n "$1" ] && _dev="$1" || _dev="$TEST_DEV"
>       src/feature -p $_dev
>       [ $? -ne 0 ] && _notrun "Installed kernel does not support project quotas"
> +    if [ "$USE_EXTERNAL" = yes -a ! -z "$_dev" ]; then
> +	_notrun "Project quotas not supported on realtime filesystem"
> +    fi
>   }
>
>   #
>
Another nice cleanup patch, looks good.

Reviewed-by: Rich Johnston <rjohnston@sgi.com>

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] xfstests: quota not supported on realtime filesystems
  2013-05-09  1:21 ` [PATCH 2/3] xfstests: quota not supported on realtime filesystems Dave Chinner
  2013-05-16 12:32   ` Rich Johnston
@ 2013-05-16 12:34   ` Rich Johnston
  1 sibling, 0 replies; 11+ messages in thread
From: Rich Johnston @ 2013-05-16 12:34 UTC (permalink / raw)
  To: Dave Chinner; +Cc: xfs

On 05/08/2013 08:21 PM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Realtime XFS filesystems do not support quotas, so quota tests
> always fail on such filesystems. Add a check to _require_quota to
> detect this situation and notrun the quota tests...
>
> Also, fix _require_xfs_quota and _require_prjquota to have the same
> checks.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---

This patch has been committed:

commit 4e787f8fd42fdfdc6e988a8667119a21ab3b194e
Author: Dave Chinner <dchinner@redhat.com>
Date:   Thu May 16 06:52:05 2013 -0500

     xfstests: quota not supported on realtime filesystems

Thanks
--Rich

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] xfstests: generic/235 breaks /etc/mtab symlinks breaks xfs/189
  2013-05-16 12:29   ` Rich Johnston
@ 2013-05-16 22:41     ` Dave Chinner
  0 siblings, 0 replies; 11+ messages in thread
From: Dave Chinner @ 2013-05-16 22:41 UTC (permalink / raw)
  To: Rich Johnston; +Cc: xfs

On Thu, May 16, 2013 at 07:29:34AM -0500, Rich Johnston wrote:
> On 05/08/2013 08: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>
> >---
> 
> Thanks for the cleanup patches.  Only comment is although I enjoy
> the humorous patch description, can we make it shorter?

No, I'm not going to rewrite it - there's nothing wrong with having
a long commit message to describe the change.

FWIW, the notes I wrote as I tracked down this problem are about
twice as long as this summary, so any straight up and down
description of the problem is not going to make the commit message
smaller.  And a boring commit message will simply not convey the
insanity of the situation....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] xfstests: generic/235 breaks /etc/mtab symlinks breaks xfs/189
  2013-05-09  1:21 ` [PATCH 3/3] xfstests: generic/235 breaks /etc/mtab symlinks breaks xfs/189 Dave Chinner
  2013-05-09  2:46   ` Michael L. Semon
  2013-05-16 12:29   ` Rich Johnston
@ 2013-05-17 11:53   ` Rich Johnston
  2 siblings, 0 replies; 11+ messages in thread
From: Rich Johnston @ 2013-05-17 11:53 UTC (permalink / raw)
  To: Dave Chinner; +Cc: xfs

On 05/08/2013 08:21 PM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Serenity lost.
> Insanity looms darkly.
> /etc/mtab
>
Fair enough Dave.

Reviewed-by: Rich Johnston <rjohnston@sgi.com>

This has been committed:

commit 1bd086437fac4007610ab0d42f75ae6701f6da1a
Author: Dave Chinner <dchinner@redhat.com>
Date:   Thu May 16 06:53:05 2013 -0500

     xfstests: generic/235 breaks /etc/mtab symlinks breaks xfs/189

Thanks
--Rich

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2013-05-17 11:53 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-09  1:21 [PATCH 0/3, V2] xfstests: more fixes Dave Chinner
2013-05-09  1:21 ` [PATCH 1/3] xfstests: fix incorrect redirect in generic/233 Dave Chinner
2013-05-09  1:21 ` [PATCH 2/3] xfstests: quota not supported on realtime filesystems Dave Chinner
2013-05-16 12:32   ` Rich Johnston
2013-05-16 12:34   ` Rich Johnston
2013-05-09  1:21 ` [PATCH 3/3] xfstests: generic/235 breaks /etc/mtab symlinks breaks xfs/189 Dave Chinner
2013-05-09  2:46   ` Michael L. Semon
2013-05-09  3:15     ` Dave Chinner
2013-05-16 12:29   ` Rich Johnston
2013-05-16 22:41     ` Dave Chinner
2013-05-17 11:53   ` Rich Johnston

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox