fstests.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] generic/050 fixes
@ 2019-11-05 13:19 Jan Kara
  2019-11-05 13:19 ` [PATCH 1/2] generic/050: Fix test failure for filesystems without journal Jan Kara
  2019-11-05 13:19 ` [PATCH 2/2] generic/050: Handle xfs quota special case with different output Jan Kara
  0 siblings, 2 replies; 5+ messages in thread
From: Jan Kara @ 2019-11-05 13:19 UTC (permalink / raw)
  To: fstests


Hello,

test generic/050 generates false failures on filesystems without journal
(e.g. ext4 in nojournal mode). The test actually makes some sense in
these cases to test behavior with read-only devices so I've decided to
fix the test by providing alternate output for such filesystems. Also
XFS with quota was already special-cased in the test and was faking the
output so the second patch transitions that to alternate output file as
well.

								Honza

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

* [PATCH 1/2] generic/050: Fix test failure for filesystems without journal
  2019-11-05 13:19 [PATCH 0/2] generic/050 fixes Jan Kara
@ 2019-11-05 13:19 ` Jan Kara
  2019-11-05 13:19 ` [PATCH 2/2] generic/050: Handle xfs quota special case with different output Jan Kara
  1 sibling, 0 replies; 5+ messages in thread
From: Jan Kara @ 2019-11-05 13:19 UTC (permalink / raw)
  To: fstests; +Cc: Jan Kara

Filesystems without journal can happily mount unrecovered filesystem
read-only which confuses this test. Handle this by providing different
expected output for filesystems without journal.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 common/rc                                  | 33 +++++++++++++++++++++++-------
 tests/generic/050                          |  8 ++++++++
 tests/generic/050.cfg                      |  1 +
 tests/generic/{050.out => 050.out.default} |  0
 tests/generic/050.out.nojournal            | 22 ++++++++++++++++++++
 5 files changed, 57 insertions(+), 7 deletions(-)
 create mode 100644 tests/generic/050.cfg
 rename tests/generic/{050.out => 050.out.default} (100%)
 create mode 100644 tests/generic/050.out.nojournal

diff --git a/common/rc b/common/rc
index e0b087c10e8f..881802b941ab 100644
--- a/common/rc
+++ b/common/rc
@@ -3143,7 +3143,7 @@ _require_norecovery()
 # It's possible that TEST_DEV and SCRATCH_DEV have different features (it'd be
 # odd, but possible) so check $TEST_DEV by default, but we can optionall pass
 # any dev we want.
-_require_metadata_journaling()
+_has_metadata_journaling()
 {
 	if [ -z $1 ]; then
 		local dev=$TEST_DEV
@@ -3153,26 +3153,37 @@ _require_metadata_journaling()
 
 	case "$FSTYP" in
 	ext2|vfat|msdos|udf)
-		_notrun "$FSTYP does not support metadata journaling"
+		echo "$FSTYP does not support metadata journaling"
+		return 1
 		;;
 	ext4)
 		# ext4 could be mkfs'd without a journal...
 		_require_dumpe2fs
-		$DUMPE2FS_PROG -h $dev 2>&1 | grep -q has_journal || \
-			_notrun "$FSTYP on $dev not configured with metadata journaling"
+		$DUMPE2FS_PROG -h $dev 2>&1 | grep -q has_journal || {
+			echo "$FSTYP on $dev not configured with metadata journaling"
+			return 1
+		}
 		# ext4 might not load a journal
-		_exclude_scratch_mount_option "noload"
+		if _normalize_mount_options | grep -qw "noload"; then
+			echo "mount option \"noload\" not allowed in this test"
+			return 1
+		fi
 		;;
 	overlay)
 		# metadata journaling check is based on base filesystem configurations
 		# and  because -overlay option saves those configurations to OVL_BASE_*,
 		# adding restore/override the configurations before/after the check.
 		if [ ! -z $OVL_BASE_FSTYP -a $OVL_BASE_FSTYP != "overlay" ]; then
+			local ret
+
 			_overlay_config_restore
-			_require_metadata_journaling
+			_has_metadata_journaling
+			ret=$?
 			_overlay_config_override
+			return $ret
 		else
-			_notrun "No metadata journaling support for legacy overlay setup"
+			echo "No metadata journaling support for legacy overlay setup"
+			return 1
 		fi
 		;;
 	*)
@@ -3181,6 +3192,14 @@ _require_metadata_journaling()
 	esac
 }
 
+_require_metadata_journaling()
+{
+	msg=$(_has_metadata_journaling $@)
+	if [ -n "$msg" ]; then
+		_notrun "$msg"
+	fi
+}
+
 _count_extents()
 {
 	$XFS_IO_PROG -c "fiemap" $1 | tail -n +2 | grep -v hole | wc -l
diff --git a/tests/generic/050 b/tests/generic/050
index 91632d2d0010..a8d648e5eede 100755
--- a/tests/generic/050
+++ b/tests/generic/050
@@ -6,6 +6,7 @@
 #
 # Check out various mount/remount/unmount scenarious on a read-only blockdev.
 #
+seqfull=$0
 seq=`basename $0`
 seqres=$RESULT_DIR/$seq
 echo "QA output created by $seq"
@@ -34,6 +35,13 @@ _require_scratch_shutdown
 _require_local_device $SCRATCH_DEV
 _require_norecovery
 
+# Select appropriate output file
+features=""
+if ! _has_metadata_journaling $SCRATCH_DEV >/dev/null; then
+	features="nojournal"
+fi
+_link_out_file "$features"
+
 _scratch_mkfs >/dev/null 2>&1
 
 filter_ro_mount() {
diff --git a/tests/generic/050.cfg b/tests/generic/050.cfg
new file mode 100644
index 000000000000..c76bd473873b
--- /dev/null
+++ b/tests/generic/050.cfg
@@ -0,0 +1 @@
+nojournal: nojournal
diff --git a/tests/generic/050.out b/tests/generic/050.out.default
similarity index 100%
rename from tests/generic/050.out
rename to tests/generic/050.out.default
diff --git a/tests/generic/050.out.nojournal b/tests/generic/050.out.nojournal
new file mode 100644
index 000000000000..c652c555aae4
--- /dev/null
+++ b/tests/generic/050.out.nojournal
@@ -0,0 +1,22 @@
+QA output created by 050
+setting device read-only
+mounting read-only block device:
+mount: device write-protected, mounting read-only
+touching file on read-only filesystem (should fail)
+touch: cannot touch 'SCRATCH_MNT/foo': Read-only file system
+unmounting read-only filesystem
+setting device read-write
+mounting read-write block device:
+touch files
+going down:
+unmounting shutdown filesystem:
+setting device read-only
+mounting filesystem that needs recovery on a read-only device:
+mount: device write-protected, mounting read-only
+unmounting read-only filesystem
+mounting filesystem with -o norecovery on a read-only device:
+mount: device write-protected, mounting read-only
+unmounting read-only filesystem
+setting device read-write
+mounting filesystem that needs recovery with -o ro:
+*** done
-- 
2.16.4


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

* [PATCH 2/2] generic/050: Handle xfs quota special case with different output
  2019-11-05 13:19 [PATCH 0/2] generic/050 fixes Jan Kara
  2019-11-05 13:19 ` [PATCH 1/2] generic/050: Fix test failure for filesystems without journal Jan Kara
@ 2019-11-05 13:19 ` Jan Kara
  2019-11-10 14:10   ` Eryu Guan
  1 sibling, 1 reply; 5+ messages in thread
From: Jan Kara @ 2019-11-05 13:19 UTC (permalink / raw)
  To: fstests; +Cc: Jan Kara

Instead of faking output for the case of XFS with quotas, just use a
different output file with appropriate output.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 tests/generic/050              | 29 ++++++-----------------------
 tests/generic/050.cfg          |  1 +
 tests/generic/050.out.xfsquota | 22 ++++++++++++++++++++++
 3 files changed, 29 insertions(+), 23 deletions(-)
 create mode 100644 tests/generic/050.out.xfsquota

diff --git a/tests/generic/050 b/tests/generic/050
index a8d648e5eede..c5375805fd7a 100755
--- a/tests/generic/050
+++ b/tests/generic/050
@@ -39,27 +39,15 @@ _require_norecovery
 features=""
 if ! _has_metadata_journaling $SCRATCH_DEV >/dev/null; then
 	features="nojournal"
+elif [ "$FSTYP" = "xfs" ] && echo "$MOUNT_OPTIONS" | grep -q quota ; then
+	# Mounting with quota on XFS requires a writable fs, which means
+	# we expect to fail the ro blockdev test with with EPERM.
+	features="xfsquota"
 fi
 _link_out_file "$features"
 
 _scratch_mkfs >/dev/null 2>&1
 
-filter_ro_mount() {
-	local arg=""
-
-	if [ -n "$expect_mount_failure" ]; then
-		arg="s|mount: $SCRATCH_MNT: permission denied|mount: device write-protected, mounting read-only|g"
-	fi
-	sed -e "$arg" | _filter_ro_mount
-}
-
-# Mounting with quota on XFS requires a writable fs, which means
-# we expect to fail the ro blockdev test with with EPERM.
-expect_mount_failure=
-if [ "$FSTYP" = "xfs" ] && echo "$MOUNT_OPTIONS" | grep -q quota ; then
-	expect_mount_failure=1
-fi
-
 #
 # Mark the device read-only
 #
@@ -70,7 +58,7 @@ blockdev --setro $SCRATCH_DEV
 # Mount it, and make sure we can't write to it, and we can unmount it again
 #
 echo "mounting read-only block device:"
-_try_scratch_mount 2>&1 | filter_ro_mount
+_try_scratch_mount 2>&1 | _filter_ro_mount
 if [ "${PIPESTATUS[0]}" -eq 0 ]; then
 	echo "touching file on read-only filesystem (should fail)"
 	touch $SCRATCH_MNT/foo 2>&1 | _filter_scratch
@@ -81,11 +69,6 @@ if [ "${PIPESTATUS[0]}" -eq 0 ]; then
 	#
 	echo "unmounting read-only filesystem"
 	_scratch_unmount 2>&1 | _filter_scratch
-elif [ -n "${expect_mount_failure}" ]; then
-	# Mount failed, so simulate EROFS instead of scribbling on root fs
-	echo "touching file on read-only filesystem (should fail)"
-	echo "touch: cannot touch 'SCRATCH_MNT/foo': Read-only file system"
-	echo "unmounting read-only filesystem"
 else
 	echo "Mount failed, though it wasn't supposed to!"
 fi
@@ -124,7 +107,7 @@ _scratch_unmount 2>&1 | _filter_scratch | _filter_ending_dot
 # data recovery hack.
 #
 echo "mounting filesystem with -o norecovery on a read-only device:"
-_try_scratch_mount -o norecovery 2>&1 | filter_ro_mount
+_try_scratch_mount -o norecovery 2>&1 | _filter_ro_mount
 if [ "${PIPESTATUS[0]}" -eq 0 ]; then
 	echo "unmounting read-only filesystem"
 	_scratch_unmount 2>&1 | _filter_scratch
diff --git a/tests/generic/050.cfg b/tests/generic/050.cfg
index c76bd473873b..1d9d60bc69a0 100644
--- a/tests/generic/050.cfg
+++ b/tests/generic/050.cfg
@@ -1 +1,2 @@
 nojournal: nojournal
+xfsquota: xfsquota
diff --git a/tests/generic/050.out.xfsquota b/tests/generic/050.out.xfsquota
new file mode 100644
index 000000000000..1a90d525c40a
--- /dev/null
+++ b/tests/generic/050.out.xfsquota
@@ -0,0 +1,22 @@
+QA output created by 050
+setting device read-only
+mounting read-only block device:
+mount: /mnt-scratch: permission denied
+Mount failed, though it wasn't supposed to!
+setting device read-write
+mounting read-write block device:
+touch files
+going down:
+unmounting shutdown filesystem:
+setting device read-only
+mounting filesystem that needs recovery on a read-only device:
+mount: device write-protected, mounting read-only
+mount: cannot mount device read-only
+unmounting read-only filesystem
+umount: SCRATCH_DEV: not mounted
+mounting filesystem with -o norecovery on a read-only device:
+mount: /mnt-scratch: permission denied
+Mount failed, though it wasn't supposed to!
+setting device read-write
+mounting filesystem that needs recovery with -o ro:
+*** done
-- 
2.16.4


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

* Re: [PATCH 2/2] generic/050: Handle xfs quota special case with different output
  2019-11-05 13:19 ` [PATCH 2/2] generic/050: Handle xfs quota special case with different output Jan Kara
@ 2019-11-10 14:10   ` Eryu Guan
  2019-11-11 14:29     ` Jan Kara
  0 siblings, 1 reply; 5+ messages in thread
From: Eryu Guan @ 2019-11-10 14:10 UTC (permalink / raw)
  To: Jan Kara; +Cc: fstests, Darrick J. Wong

[cc'ed Darrick for his inputs, if there's any]

On Tue, Nov 05, 2019 at 02:19:22PM +0100, Jan Kara wrote:
> Instead of faking output for the case of XFS with quotas, just use a
> different output file with appropriate output.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  tests/generic/050              | 29 ++++++-----------------------
>  tests/generic/050.cfg          |  1 +
>  tests/generic/050.out.xfsquota | 22 ++++++++++++++++++++++
>  3 files changed, 29 insertions(+), 23 deletions(-)
>  create mode 100644 tests/generic/050.out.xfsquota
> 
> diff --git a/tests/generic/050 b/tests/generic/050
> index a8d648e5eede..c5375805fd7a 100755
> --- a/tests/generic/050
> +++ b/tests/generic/050
> @@ -39,27 +39,15 @@ _require_norecovery
>  features=""
>  if ! _has_metadata_journaling $SCRATCH_DEV >/dev/null; then
>  	features="nojournal"
> +elif [ "$FSTYP" = "xfs" ] && echo "$MOUNT_OPTIONS" | grep -q quota ; then
> +	# Mounting with quota on XFS requires a writable fs, which means
> +	# we expect to fail the ro blockdev test with with EPERM.
> +	features="xfsquota"

Given that we already have a non-default output for "nojournal" feature,
this looks fine to me.

>  fi
>  _link_out_file "$features"
>  
>  _scratch_mkfs >/dev/null 2>&1
>  
> -filter_ro_mount() {
> -	local arg=""
> -
> -	if [ -n "$expect_mount_failure" ]; then
> -		arg="s|mount: $SCRATCH_MNT: permission denied|mount: device write-protected, mounting read-only|g"
> -	fi
> -	sed -e "$arg" | _filter_ro_mount
> -}
> -
> -# Mounting with quota on XFS requires a writable fs, which means
> -# we expect to fail the ro blockdev test with with EPERM.
> -expect_mount_failure=
> -if [ "$FSTYP" = "xfs" ] && echo "$MOUNT_OPTIONS" | grep -q quota ; then
> -	expect_mount_failure=1
> -fi
> -
>  #
>  # Mark the device read-only
>  #
> @@ -70,7 +58,7 @@ blockdev --setro $SCRATCH_DEV
>  # Mount it, and make sure we can't write to it, and we can unmount it again
>  #
>  echo "mounting read-only block device:"
> -_try_scratch_mount 2>&1 | filter_ro_mount
> +_try_scratch_mount 2>&1 | _filter_ro_mount
>  if [ "${PIPESTATUS[0]}" -eq 0 ]; then

But I think we could remove above check as a whole, just do

_try_scratch_mount 2>&1 | _filter_ro_mount
echo "touching file on read-only filesystem (should fail)"
touch $SCRATCH_MNT/foo 2>&1 | _filter_scratch
echo "unmounting read-only filesystem"
_scratch_unmount 2>&1 | _filter_scratch

as what we did prior to commit b0415daaa968 ("generic/050: fix ro
blockdev mount of xfs with quota").

>  	echo "touching file on read-only filesystem (should fail)"
>  	touch $SCRATCH_MNT/foo 2>&1 | _filter_scratch
> @@ -81,11 +69,6 @@ if [ "${PIPESTATUS[0]}" -eq 0 ]; then
>  	#
>  	echo "unmounting read-only filesystem"
>  	_scratch_unmount 2>&1 | _filter_scratch
> -elif [ -n "${expect_mount_failure}" ]; then
> -	# Mount failed, so simulate EROFS instead of scribbling on root fs
> -	echo "touching file on read-only filesystem (should fail)"
> -	echo "touch: cannot touch 'SCRATCH_MNT/foo': Read-only file system"
> -	echo "unmounting read-only filesystem"
>  else
>  	echo "Mount failed, though it wasn't supposed to!"
>  fi

So this message could be removed too, which is confusing in xfsquota
case, and _scratch_unmount error message would break golden image in
non-xfsquota case.

> @@ -124,7 +107,7 @@ _scratch_unmount 2>&1 | _filter_scratch | _filter_ending_dot
>  # data recovery hack.
>  #
>  echo "mounting filesystem with -o norecovery on a read-only device:"
> -_try_scratch_mount -o norecovery 2>&1 | filter_ro_mount
> +_try_scratch_mount -o norecovery 2>&1 | _filter_ro_mount
>  if [ "${PIPESTATUS[0]}" -eq 0 ]; then
>  	echo "unmounting read-only filesystem"
>  	_scratch_unmount 2>&1 | _filter_scratch
> diff --git a/tests/generic/050.cfg b/tests/generic/050.cfg
> index c76bd473873b..1d9d60bc69a0 100644
> --- a/tests/generic/050.cfg
> +++ b/tests/generic/050.cfg
> @@ -1 +1,2 @@
>  nojournal: nojournal
> +xfsquota: xfsquota
> diff --git a/tests/generic/050.out.xfsquota b/tests/generic/050.out.xfsquota
> new file mode 100644
> index 000000000000..1a90d525c40a
> --- /dev/null
> +++ b/tests/generic/050.out.xfsquota
> @@ -0,0 +1,22 @@
> +QA output created by 050
> +setting device read-only
> +mounting read-only block device:
> +mount: /mnt-scratch: permission denied
> +Mount failed, though it wasn't supposed to!

This is confusing, we expect a mount failure in xfsquota case.

Thanks,
Eryu

> +setting device read-write
> +mounting read-write block device:
> +touch files
> +going down:
> +unmounting shutdown filesystem:
> +setting device read-only
> +mounting filesystem that needs recovery on a read-only device:
> +mount: device write-protected, mounting read-only
> +mount: cannot mount device read-only
> +unmounting read-only filesystem
> +umount: SCRATCH_DEV: not mounted
> +mounting filesystem with -o norecovery on a read-only device:
> +mount: /mnt-scratch: permission denied
> +Mount failed, though it wasn't supposed to!
> +setting device read-write
> +mounting filesystem that needs recovery with -o ro:
> +*** done
> -- 
> 2.16.4
> 

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

* Re: [PATCH 2/2] generic/050: Handle xfs quota special case with different output
  2019-11-10 14:10   ` Eryu Guan
@ 2019-11-11 14:29     ` Jan Kara
  0 siblings, 0 replies; 5+ messages in thread
From: Jan Kara @ 2019-11-11 14:29 UTC (permalink / raw)
  To: Eryu Guan; +Cc: Jan Kara, fstests, Darrick J. Wong

On Sun 10-11-19 22:10:54, Eryu Guan wrote:
> [cc'ed Darrick for his inputs, if there's any]
> >  fi
> >  _link_out_file "$features"
> >  
> >  _scratch_mkfs >/dev/null 2>&1
> >  
> > -filter_ro_mount() {
> > -	local arg=""
> > -
> > -	if [ -n "$expect_mount_failure" ]; then
> > -		arg="s|mount: $SCRATCH_MNT: permission denied|mount: device write-protected, mounting read-only|g"
> > -	fi
> > -	sed -e "$arg" | _filter_ro_mount
> > -}
> > -
> > -# Mounting with quota on XFS requires a writable fs, which means
> > -# we expect to fail the ro blockdev test with with EPERM.
> > -expect_mount_failure=
> > -if [ "$FSTYP" = "xfs" ] && echo "$MOUNT_OPTIONS" | grep -q quota ; then
> > -	expect_mount_failure=1
> > -fi
> > -
> >  #
> >  # Mark the device read-only
> >  #
> > @@ -70,7 +58,7 @@ blockdev --setro $SCRATCH_DEV
> >  # Mount it, and make sure we can't write to it, and we can unmount it again
> >  #
> >  echo "mounting read-only block device:"
> > -_try_scratch_mount 2>&1 | filter_ro_mount
> > +_try_scratch_mount 2>&1 | _filter_ro_mount
> >  if [ "${PIPESTATUS[0]}" -eq 0 ]; then
> 
> But I think we could remove above check as a whole, just do
> 
> _try_scratch_mount 2>&1 | _filter_ro_mount
> echo "touching file on read-only filesystem (should fail)"
> touch $SCRATCH_MNT/foo 2>&1 | _filter_scratch
> echo "unmounting read-only filesystem"
> _scratch_unmount 2>&1 | _filter_scratch
> 
> as what we did prior to commit b0415daaa968 ("generic/050: fix ro
> blockdev mount of xfs with quota").

Your suggestion looks good to me. I'll update the patch, test, and resend.

								Honza


-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

end of thread, other threads:[~2019-11-11 14:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-05 13:19 [PATCH 0/2] generic/050 fixes Jan Kara
2019-11-05 13:19 ` [PATCH 1/2] generic/050: Fix test failure for filesystems without journal Jan Kara
2019-11-05 13:19 ` [PATCH 2/2] generic/050: Handle xfs quota special case with different output Jan Kara
2019-11-10 14:10   ` Eryu Guan
2019-11-11 14:29     ` Jan Kara

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).