diff mbox

[4/7] xfs: filter out mount options that don't work on v4 filesystems

Message ID 153067987261.28393.13229691209356951536.stgit@magnolia (mailing list archive)
State New, archived
Headers show

Commit Message

Darrick J. Wong July 4, 2018, 4:51 a.m. UTC
From: Darrick J. Wong <darrick.wong@oracle.com>

A few tests require v4 filesystems and enforce this by disabling crc's
in the _scratch_mkfs call.  However, if the user specified MOUNT_OPTIONS
that only work with v5 filesystems, these tests fail.  Filter out
incompatible mount options that don't work on v4, such as simultaneous
group/project quota.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/xfs    |   23 +++++++++++++++++++++++
 tests/xfs/002 |    2 ++
 tests/xfs/194 |    2 ++
 tests/xfs/199 |    2 ++
 4 files changed, 29 insertions(+)



--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Eryu Guan July 14, 2018, 11:41 a.m. UTC | #1
On Tue, Jul 03, 2018 at 09:51:12PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> A few tests require v4 filesystems and enforce this by disabling crc's
> in the _scratch_mkfs call.  However, if the user specified MOUNT_OPTIONS
> that only work with v5 filesystems, these tests fail.  Filter out
> incompatible mount options that don't work on v4, such as simultaneous
> group/project quota.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  common/xfs    |   23 +++++++++++++++++++++++
>  tests/xfs/002 |    2 ++
>  tests/xfs/194 |    2 ++
>  tests/xfs/199 |    2 ++
>  4 files changed, 29 insertions(+)
> 
> 
> diff --git a/common/xfs b/common/xfs
> index 5b632df1..33d819d7 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -780,3 +780,26 @@ _scratch_get_bmx_prefix() {
>  	_scratch_xfs_db -c "inode ${ino}" -c 'p' >> $seqres.full
>  	return 1
>  }
> +
> +#
> +# Ensures that we don't pass any mount options incompatible with XFS v4
> +#
> +_filter_xfsv4_mount_options()

"_filter" usually indicates a filter function that gets input from a
pipe, e.g. <some cmd> | _filter_scratch, but this helper doesn't follow
the convention. It'd better to rename it to ... I don't know, just
_xfsv4_mount_options?

> +{
> +	local gquota=0
> +	local pquota=0
> +
> +	# Can't have group and project quotas in XFS v4
> +	echo "$MOUNT_OPTIONS" | egrep -q "(gquota|grpquota|grpjquota=|gqnoenforce)" && gquota=1
> +	echo "$MOUNT_OPTIONS" | egrep -q "(\bpquota|prjquota|pqnoenforce)" && pquota=1
> +
> +	if [ $gquota -gt 0 ] && [ $pquota -gt 0 ]; then
> +		export MOUNT_OPTIONS=$(echo $MOUNT_OPTIONS \
> +			| sed   -e 's/gquota/QUOTA/g'      \
> +				-e 's/grpquota/QUOTA/g'    \
> +				-e 's/grpjquota=[^, ]/QUOTA/g' \
> +				-e 's/gqnoenforce/QUOTA/g' \
> +				-e "s/QUOTA/defaults/g")
> +	fi
> +	echo "MOUNT_OPTIONS = $MOUNT_OPTIONS" >>$seqres.full
> +}
> diff --git a/tests/xfs/002 b/tests/xfs/002
> index bd1c0a7a..f987575c 100755
> --- a/tests/xfs/002
> +++ b/tests/xfs/002
> @@ -45,6 +45,8 @@ rm -f $seqres.full
>  
>  _scratch_mkfs_xfs -m crc=0 -d size=128m >> $seqres.full 2>&1 || _fail "mkfs failed"
>  
> +_filter_xfsv4_mount_options

Hmm, this way future test writers have to remember to add a
_filter_xfsv4_mount_options call everytime after making a v4 xfs. It's
so easy to get lost and hard to maintain.

How about make _scratch_mkfs_xfs do the work automatically if it finds
the newly created fs is a v4 xfs?


And I went through all tests that have explicit "-m crc=0" mkfs option,
and I found that there're some other tests need fix too.

xfs/045, it doesn't fail, but the test expects a mount failure anyway,
just that it's caused by quota options now.

xfs/132, it fails to mount xfs and needs the fix. But it still fails
even after the fix, but that's a different issue.

xfs/300, the test itself passes, it just causes subsequent
_scratch_mount to fail in _check_xfs_filesystem(). I think we could just
umount $SCRATCH_DEV explicitly after test, so _check_xfs_filesystem()
won't try to mount it again.

Thanks,
Eryu

> +
>  # Scribble past a couple V4 secondary superblocks to populate sb_crc
>  # (We can't write to the structure member because it doesn't exist
>  # on a v4 superblock, so we use the data type & "write fill")
> diff --git a/tests/xfs/194 b/tests/xfs/194
> index 6c1eddba..09a3840d 100755
> --- a/tests/xfs/194
> +++ b/tests/xfs/194
> @@ -74,6 +74,8 @@ _require_scratch
>  unset MKFS_OPTIONS
>  unset XFS_MKFS_OPTIONS
>  
> +_filter_xfsv4_mount_options
> +
>  # we need 512 byte block size, so crc's are turned off
>  _scratch_mkfs_xfs -m crc=0 -b size=$blksize >/dev/null 2>&1
>  _scratch_mount
> diff --git a/tests/xfs/199 b/tests/xfs/199
> index dd909b09..54fe54fe 100755
> --- a/tests/xfs/199
> +++ b/tests/xfs/199
> @@ -34,6 +34,8 @@ _supported_os Linux
>  
>  _require_scratch
>  
> +_filter_xfsv4_mount_options
> +
>  # clear any mkfs options so that we can directly specify the options we need to
>  # be able to test the features bitmask behaviour correctly.
>  MKFS_OPTIONS=
> 
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Darrick J. Wong Nov. 1, 2018, 10:51 p.m. UTC | #2
On Sat, Jul 14, 2018 at 07:41:28PM +0800, Eryu Guan wrote:
> On Tue, Jul 03, 2018 at 09:51:12PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > A few tests require v4 filesystems and enforce this by disabling crc's
> > in the _scratch_mkfs call.  However, if the user specified MOUNT_OPTIONS
> > that only work with v5 filesystems, these tests fail.  Filter out
> > incompatible mount options that don't work on v4, such as simultaneous
> > group/project quota.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  common/xfs    |   23 +++++++++++++++++++++++
> >  tests/xfs/002 |    2 ++
> >  tests/xfs/194 |    2 ++
> >  tests/xfs/199 |    2 ++
> >  4 files changed, 29 insertions(+)
> > 
> > 
> > diff --git a/common/xfs b/common/xfs
> > index 5b632df1..33d819d7 100644
> > --- a/common/xfs
> > +++ b/common/xfs
> > @@ -780,3 +780,26 @@ _scratch_get_bmx_prefix() {
> >  	_scratch_xfs_db -c "inode ${ino}" -c 'p' >> $seqres.full
> >  	return 1
> >  }
> > +
> > +#
> > +# Ensures that we don't pass any mount options incompatible with XFS v4
> > +#
> > +_filter_xfsv4_mount_options()
> 
> "_filter" usually indicates a filter function that gets input from a
> pipe, e.g. <some cmd> | _filter_scratch, but this helper doesn't follow
> the convention. It'd better to rename it to ... I don't know, just
> _xfsv4_mount_options?

_force_xfsv4_mount_options?

> > +{
> > +	local gquota=0
> > +	local pquota=0
> > +
> > +	# Can't have group and project quotas in XFS v4
> > +	echo "$MOUNT_OPTIONS" | egrep -q "(gquota|grpquota|grpjquota=|gqnoenforce)" && gquota=1
> > +	echo "$MOUNT_OPTIONS" | egrep -q "(\bpquota|prjquota|pqnoenforce)" && pquota=1
> > +
> > +	if [ $gquota -gt 0 ] && [ $pquota -gt 0 ]; then
> > +		export MOUNT_OPTIONS=$(echo $MOUNT_OPTIONS \
> > +			| sed   -e 's/gquota/QUOTA/g'      \
> > +				-e 's/grpquota/QUOTA/g'    \
> > +				-e 's/grpjquota=[^, ]/QUOTA/g' \
> > +				-e 's/gqnoenforce/QUOTA/g' \
> > +				-e "s/QUOTA/defaults/g")
> > +	fi
> > +	echo "MOUNT_OPTIONS = $MOUNT_OPTIONS" >>$seqres.full
> > +}
> > diff --git a/tests/xfs/002 b/tests/xfs/002
> > index bd1c0a7a..f987575c 100755
> > --- a/tests/xfs/002
> > +++ b/tests/xfs/002
> > @@ -45,6 +45,8 @@ rm -f $seqres.full
> >  
> >  _scratch_mkfs_xfs -m crc=0 -d size=128m >> $seqres.full 2>&1 || _fail "mkfs failed"
> >  
> > +_filter_xfsv4_mount_options
> 
> Hmm, this way future test writers have to remember to add a
> _filter_xfsv4_mount_options call everytime after making a v4 xfs. It's
> so easy to get lost and hard to maintain.
> 
> How about make _scratch_mkfs_xfs do the work automatically if it finds
> the newly created fs is a v4 xfs?

Sure, sounds good to me.

> And I went through all tests that have explicit "-m crc=0" mkfs option,
> and I found that there're some other tests need fix too.
> 
> xfs/045, it doesn't fail, but the test expects a mount failure anyway,
> just that it's caused by quota options now.
> 
> xfs/132, it fails to mount xfs and needs the fix. But it still fails
> even after the fix, but that's a different issue.

I think this is fixed by a subsequent patch which ... eh, this thread
was from too long ago, I'll just resend the whole thing.

> xfs/300, the test itself passes, it just causes subsequent
> _scratch_mount to fail in _check_xfs_filesystem(). I think we could just
> umount $SCRATCH_DEV explicitly after test, so _check_xfs_filesystem()
> won't try to mount it again.

Hm, I'll take a look at that one too.

--D

> Thanks,
> Eryu
> 
> > +
> >  # Scribble past a couple V4 secondary superblocks to populate sb_crc
> >  # (We can't write to the structure member because it doesn't exist
> >  # on a v4 superblock, so we use the data type & "write fill")
> > diff --git a/tests/xfs/194 b/tests/xfs/194
> > index 6c1eddba..09a3840d 100755
> > --- a/tests/xfs/194
> > +++ b/tests/xfs/194
> > @@ -74,6 +74,8 @@ _require_scratch
> >  unset MKFS_OPTIONS
> >  unset XFS_MKFS_OPTIONS
> >  
> > +_filter_xfsv4_mount_options
> > +
> >  # we need 512 byte block size, so crc's are turned off
> >  _scratch_mkfs_xfs -m crc=0 -b size=$blksize >/dev/null 2>&1
> >  _scratch_mount
> > diff --git a/tests/xfs/199 b/tests/xfs/199
> > index dd909b09..54fe54fe 100755
> > --- a/tests/xfs/199
> > +++ b/tests/xfs/199
> > @@ -34,6 +34,8 @@ _supported_os Linux
> >  
> >  _require_scratch
> >  
> > +_filter_xfsv4_mount_options
> > +
> >  # clear any mkfs options so that we can directly specify the options we need to
> >  # be able to test the features bitmask behaviour correctly.
> >  MKFS_OPTIONS=
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/common/xfs b/common/xfs
index 5b632df1..33d819d7 100644
--- a/common/xfs
+++ b/common/xfs
@@ -780,3 +780,26 @@  _scratch_get_bmx_prefix() {
 	_scratch_xfs_db -c "inode ${ino}" -c 'p' >> $seqres.full
 	return 1
 }
+
+#
+# Ensures that we don't pass any mount options incompatible with XFS v4
+#
+_filter_xfsv4_mount_options()
+{
+	local gquota=0
+	local pquota=0
+
+	# Can't have group and project quotas in XFS v4
+	echo "$MOUNT_OPTIONS" | egrep -q "(gquota|grpquota|grpjquota=|gqnoenforce)" && gquota=1
+	echo "$MOUNT_OPTIONS" | egrep -q "(\bpquota|prjquota|pqnoenforce)" && pquota=1
+
+	if [ $gquota -gt 0 ] && [ $pquota -gt 0 ]; then
+		export MOUNT_OPTIONS=$(echo $MOUNT_OPTIONS \
+			| sed   -e 's/gquota/QUOTA/g'      \
+				-e 's/grpquota/QUOTA/g'    \
+				-e 's/grpjquota=[^, ]/QUOTA/g' \
+				-e 's/gqnoenforce/QUOTA/g' \
+				-e "s/QUOTA/defaults/g")
+	fi
+	echo "MOUNT_OPTIONS = $MOUNT_OPTIONS" >>$seqres.full
+}
diff --git a/tests/xfs/002 b/tests/xfs/002
index bd1c0a7a..f987575c 100755
--- a/tests/xfs/002
+++ b/tests/xfs/002
@@ -45,6 +45,8 @@  rm -f $seqres.full
 
 _scratch_mkfs_xfs -m crc=0 -d size=128m >> $seqres.full 2>&1 || _fail "mkfs failed"
 
+_filter_xfsv4_mount_options
+
 # Scribble past a couple V4 secondary superblocks to populate sb_crc
 # (We can't write to the structure member because it doesn't exist
 # on a v4 superblock, so we use the data type & "write fill")
diff --git a/tests/xfs/194 b/tests/xfs/194
index 6c1eddba..09a3840d 100755
--- a/tests/xfs/194
+++ b/tests/xfs/194
@@ -74,6 +74,8 @@  _require_scratch
 unset MKFS_OPTIONS
 unset XFS_MKFS_OPTIONS
 
+_filter_xfsv4_mount_options
+
 # we need 512 byte block size, so crc's are turned off
 _scratch_mkfs_xfs -m crc=0 -b size=$blksize >/dev/null 2>&1
 _scratch_mount
diff --git a/tests/xfs/199 b/tests/xfs/199
index dd909b09..54fe54fe 100755
--- a/tests/xfs/199
+++ b/tests/xfs/199
@@ -34,6 +34,8 @@  _supported_os Linux
 
 _require_scratch
 
+_filter_xfsv4_mount_options
+
 # clear any mkfs options so that we can directly specify the options we need to
 # be able to test the features bitmask behaviour correctly.
 MKFS_OPTIONS=