* [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).