| Message ID | 1666867852-134-1-git-send-email-yangx.jy@fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | common/config: Make test and scratch devices use the same mount options | expand |
On Thu, Oct 27, 2022 at 10:50:52AM +0000, Xiao Yang wrote: > Some cases(e.g. generic/519) check commands/features on test device but > do tests on scratch device. If some mount options can impact the check > result, these cases may throw error instead if not run when we use > different mount options for test and scratch devices. > > Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com> > --- This patch makes sense to me. Actually I'm a little curious about the history of _test_mount_opts() function, so I tried to check the commit log of fstests, then I found this function was brought in by 0e9141e49d4a ("common: add cifs support"). This commit set MOUNT_OPTIONS and TEST_FS_MOUNT_OPTS same, then later patches (which trys to support a new fs) update mount_opts() but sometimes don't update _test_mount_opts(). I don't know if they're intended. From my narrow knowledge I think TEST_FS_MOUNT_OPTS and MOUNT_OPTS should keep consistent, if no specific setup. And the log history shows that we've tried to fix their inconsistent problems by some patches. E.g. adf56068 ("common/config: add acl and user_xattr support for TEST_DEV") 643c6850 ("common/config: honor NFS_MOUNT_OPTIONS in _test_mount_opts") I think even if someone fs really need a different default TEST_FS_MOUNT_OPTS, we can change it in _test_mount_opts, or let them same. I gave this patch a little testing, but I can't test it on all fs we support. So I'll keep this patch a week, if anyone has any review points, please feel free to reply this email. If no, I'd like to give it my RVB at first. Reviewed-by: Zorro Lang <zlang@redhat.com> Thanks, Zorro > common/config | 65 +++++++++++++++++++-------------------------------- > 1 file changed, 24 insertions(+), 41 deletions(-) > > diff --git a/common/config b/common/config > index 5eaae447..6f428eae 100644 > --- a/common/config > +++ b/common/config > @@ -334,89 +334,72 @@ if [ "$FSTYP" == "xfs" ]; then > export XFS_MKFS_HAS_NO_META_SUPPORT > fi > > -_mount_opts() > +_common_mount_opts() > { > case $FSTYP in > 9p) > - export MOUNT_OPTIONS=$PLAN9_MOUNT_OPTIONS > + echo $PLAN9_MOUNT_OPTIONS > ;; > xfs) > - export MOUNT_OPTIONS=$XFS_MOUNT_OPTIONS > + echo $XFS_MOUNT_OPTIONS > ;; > udf) > - export MOUNT_OPTIONS=$UDF_MOUNT_OPTIONS > + echo $UDF_MOUNT_OPTIONS > ;; > nfs) > - export MOUNT_OPTIONS=$NFS_MOUNT_OPTIONS > + echo $NFS_MOUNT_OPTIONS > ;; > cifs) > - export MOUNT_OPTIONS=$CIFS_MOUNT_OPTIONS > + echo $CIFS_MOUNT_OPTIONS > ;; > ceph) > - export MOUNT_OPTIONS=$CEPHFS_MOUNT_OPTIONS > + echo $CEPHFS_MOUNT_OPTIONS > ;; > glusterfs) > - export MOUNT_OPTIONS=$GLUSTERFS_MOUNT_OPTIONS > + echo $GLUSTERFS_MOUNT_OPTIONS > ;; > overlay) > - export MOUNT_OPTIONS=$OVERLAY_MOUNT_OPTIONS > + echo $OVERLAY_MOUNT_OPTIONS > ;; > ext2|ext3|ext4|ext4dev) > # acls & xattrs aren't turned on by default on ext$FOO > - export MOUNT_OPTIONS="-o acl,user_xattr $EXT_MOUNT_OPTIONS" > + echo "-o acl,user_xattr $EXT_MOUNT_OPTIONS" > ;; > f2fs) > - export MOUNT_OPTIONS="-o acl,user_xattr $F2FS_MOUNT_OPTIONS" > + echo "-o acl,user_xattr $F2FS_MOUNT_OPTIONS" > ;; > reiserfs) > # acls & xattrs aren't turned on by default on reiserfs > - export MOUNT_OPTIONS="-o acl,user_xattr $REISERFS_MOUNT_OPTIONS" > + echo "-o acl,user_xattr $REISERFS_MOUNT_OPTIONS" > ;; > reiser4) > - # acls & xattrs aren't supported by reiser4 > - export MOUNT_OPTIONS=$REISER4_MOUNT_OPTIONS > - ;; > + # acls & xattrs aren't supported by reiser4 > + echo $REISER4_MOUNT_OPTIONS > + ;; > gfs2) > # acls aren't turned on by default on gfs2 > - export MOUNT_OPTIONS="-o acl $GFS2_MOUNT_OPTIONS" > + echo "-o acl $GFS2_MOUNT_OPTIONS" > ;; > tmpfs) > # We need to specify the size at mount, use 1G by default > - export MOUNT_OPTIONS="-o size=1G $TMPFS_MOUNT_OPTIONS" > + echo "-o size=1G $TMPFS_MOUNT_OPTIONS" > ;; > ubifs) > - export MOUNT_OPTIONS=$UBIFS_MOUNT_OPTIONS > + echo $UBIFS_MOUNT_OPTIONS > ;; > *) > ;; > esac > } > > +_mount_opts() > +{ > + export MOUNT_OPTIONS=$(_common_mount_opts) > +} > + > _test_mount_opts() > { > - case $FSTYP in > - 9p) > - export TEST_FS_MOUNT_OPTS=$PLAN9_MOUNT_OPTIONS > - ;; > - cifs) > - export TEST_FS_MOUNT_OPTS=$CIFS_MOUNT_OPTIONS > - ;; > - ceph) > - export TEST_FS_MOUNT_OPTS=$CEPHFS_MOUNT_OPTIONS > - ;; > - nfs) > - export TEST_FS_MOUNT_OPTS=$NFS_MOUNT_OPTIONS > - ;; > - glusterfs) > - export TEST_FS_MOUNT_OPTS=$GLUSTERFS_MOUNT_OPTIONS > - ;; > - ext2|ext3|ext4|ext4dev) > - # acls & xattrs aren't turned on by default on older ext$FOO > - export TEST_FS_MOUNT_OPTS="-o acl,user_xattr $EXT_MOUNT_OPTIONS" > - ;; > - *) > - ;; > - esac > + export TEST_FS_MOUNT_OPTS=$(_common_mount_opts) > } > > _mkfs_opts() > -- > 2.34.1 >
On 2022/10/30 11:26, Zorro Lang wrote: > On Thu, Oct 27, 2022 at 10:50:52AM +0000, Xiao Yang wrote: >> Some cases(e.g. generic/519) check commands/features on test device but >> do tests on scratch device. If some mount options can impact the check >> result, these cases may throw error instead if not run when we use >> different mount options for test and scratch devices. >> >> Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com> >> --- > > This patch makes sense to me. Hi Zorro, Thanks for your review. Sorry for a typo (s/instead if/instead of/). > > Actually I'm a little curious about the history of _test_mount_opts() function, > so I tried to check the commit log of fstests, then I found this function was > brought in by 0e9141e49d4a ("common: add cifs support"). > > This commit set MOUNT_OPTIONS and TEST_FS_MOUNT_OPTS same, then later patches > (which trys to support a new fs) update mount_opts() but sometimes don't > update _test_mount_opts(). I don't know if they're intended. From my narrow > knowledge I think TEST_FS_MOUNT_OPTS and MOUNT_OPTS should keep consistent, if > no specific setup. And the log history shows that we've tried to fix their > inconsistent problems by some patches. E.g. > adf56068 ("common/config: add acl and user_xattr support for TEST_DEV") > 643c6850 ("common/config: honor NFS_MOUNT_OPTIONS in _test_mount_opts") > Yes, I think they should keep consistent usually. We can use the same TEST_FS_MOUNT_OPTS and MOUNT_OPTIONS by seting $FSTYP_MOUNT_OPTIONS in local.config. For example: export XFS_MOUNT_OPTIONS="-o dax" > I think even if someone fs really need a different default TEST_FS_MOUNT_OPTS, > we can change it in _test_mount_opts, or let them same. If someone wants to use the different TEST_FS_MOUNT_OPTS and MOUNT_OPTS, he can set TEST_FS_MOUNT_OPTS and MOUNT_OPTIONS directly in local.config. The logic is defined in get_next_config() ------------------------------------------------ [ -z "$MOUNT_OPTIONS" ] && _mount_opts [ -z "$TEST_FS_MOUNT_OPTS" ] && _test_mount_opts ------------------------------------------------ > > I gave this patch a little testing, but I can't test it on all fs we support. > So I'll keep this patch a week, if anyone has any review points, please feel > free to reply this email. If no, I'd like to give it my RVB at first. > > Reviewed-by: Zorro Lang <zlang@redhat.com> Thanks a lot. Best Regards, Xiao Yang > > Thanks, > Zorro > >> common/config | 65 +++++++++++++++++++-------------------------------- >> 1 file changed, 24 insertions(+), 41 deletions(-) >> >> diff --git a/common/config b/common/config >> index 5eaae447..6f428eae 100644 >> --- a/common/config >> +++ b/common/config >> @@ -334,89 +334,72 @@ if [ "$FSTYP" == "xfs" ]; then >> export XFS_MKFS_HAS_NO_META_SUPPORT >> fi >> >> -_mount_opts() >> +_common_mount_opts() >> { >> case $FSTYP in >> 9p) >> - export MOUNT_OPTIONS=$PLAN9_MOUNT_OPTIONS >> + echo $PLAN9_MOUNT_OPTIONS >> ;; >> xfs) >> - export MOUNT_OPTIONS=$XFS_MOUNT_OPTIONS >> + echo $XFS_MOUNT_OPTIONS >> ;; >> udf) >> - export MOUNT_OPTIONS=$UDF_MOUNT_OPTIONS >> + echo $UDF_MOUNT_OPTIONS >> ;; >> nfs) >> - export MOUNT_OPTIONS=$NFS_MOUNT_OPTIONS >> + echo $NFS_MOUNT_OPTIONS >> ;; >> cifs) >> - export MOUNT_OPTIONS=$CIFS_MOUNT_OPTIONS >> + echo $CIFS_MOUNT_OPTIONS >> ;; >> ceph) >> - export MOUNT_OPTIONS=$CEPHFS_MOUNT_OPTIONS >> + echo $CEPHFS_MOUNT_OPTIONS >> ;; >> glusterfs) >> - export MOUNT_OPTIONS=$GLUSTERFS_MOUNT_OPTIONS >> + echo $GLUSTERFS_MOUNT_OPTIONS >> ;; >> overlay) >> - export MOUNT_OPTIONS=$OVERLAY_MOUNT_OPTIONS >> + echo $OVERLAY_MOUNT_OPTIONS >> ;; >> ext2|ext3|ext4|ext4dev) >> # acls & xattrs aren't turned on by default on ext$FOO >> - export MOUNT_OPTIONS="-o acl,user_xattr $EXT_MOUNT_OPTIONS" >> + echo "-o acl,user_xattr $EXT_MOUNT_OPTIONS" >> ;; >> f2fs) >> - export MOUNT_OPTIONS="-o acl,user_xattr $F2FS_MOUNT_OPTIONS" >> + echo "-o acl,user_xattr $F2FS_MOUNT_OPTIONS" >> ;; >> reiserfs) >> # acls & xattrs aren't turned on by default on reiserfs >> - export MOUNT_OPTIONS="-o acl,user_xattr $REISERFS_MOUNT_OPTIONS" >> + echo "-o acl,user_xattr $REISERFS_MOUNT_OPTIONS" >> ;; >> reiser4) >> - # acls & xattrs aren't supported by reiser4 >> - export MOUNT_OPTIONS=$REISER4_MOUNT_OPTIONS >> - ;; >> + # acls & xattrs aren't supported by reiser4 >> + echo $REISER4_MOUNT_OPTIONS >> + ;; >> gfs2) >> # acls aren't turned on by default on gfs2 >> - export MOUNT_OPTIONS="-o acl $GFS2_MOUNT_OPTIONS" >> + echo "-o acl $GFS2_MOUNT_OPTIONS" >> ;; >> tmpfs) >> # We need to specify the size at mount, use 1G by default >> - export MOUNT_OPTIONS="-o size=1G $TMPFS_MOUNT_OPTIONS" >> + echo "-o size=1G $TMPFS_MOUNT_OPTIONS" >> ;; >> ubifs) >> - export MOUNT_OPTIONS=$UBIFS_MOUNT_OPTIONS >> + echo $UBIFS_MOUNT_OPTIONS >> ;; >> *) >> ;; >> esac >> } >> >> +_mount_opts() >> +{ >> + export MOUNT_OPTIONS=$(_common_mount_opts) >> +} >> + >> _test_mount_opts() >> { >> - case $FSTYP in >> - 9p) >> - export TEST_FS_MOUNT_OPTS=$PLAN9_MOUNT_OPTIONS >> - ;; >> - cifs) >> - export TEST_FS_MOUNT_OPTS=$CIFS_MOUNT_OPTIONS >> - ;; >> - ceph) >> - export TEST_FS_MOUNT_OPTS=$CEPHFS_MOUNT_OPTIONS >> - ;; >> - nfs) >> - export TEST_FS_MOUNT_OPTS=$NFS_MOUNT_OPTIONS >> - ;; >> - glusterfs) >> - export TEST_FS_MOUNT_OPTS=$GLUSTERFS_MOUNT_OPTIONS >> - ;; >> - ext2|ext3|ext4|ext4dev) >> - # acls & xattrs aren't turned on by default on older ext$FOO >> - export TEST_FS_MOUNT_OPTS="-o acl,user_xattr $EXT_MOUNT_OPTIONS" >> - ;; >> - *) >> - ;; >> - esac >> + export TEST_FS_MOUNT_OPTS=$(_common_mount_opts) >> } >> >> _mkfs_opts() >> -- >> 2.34.1 >> >
diff --git a/common/config b/common/config index 5eaae447..6f428eae 100644 --- a/common/config +++ b/common/config @@ -334,89 +334,72 @@ if [ "$FSTYP" == "xfs" ]; then export XFS_MKFS_HAS_NO_META_SUPPORT fi -_mount_opts() +_common_mount_opts() { case $FSTYP in 9p) - export MOUNT_OPTIONS=$PLAN9_MOUNT_OPTIONS + echo $PLAN9_MOUNT_OPTIONS ;; xfs) - export MOUNT_OPTIONS=$XFS_MOUNT_OPTIONS + echo $XFS_MOUNT_OPTIONS ;; udf) - export MOUNT_OPTIONS=$UDF_MOUNT_OPTIONS + echo $UDF_MOUNT_OPTIONS ;; nfs) - export MOUNT_OPTIONS=$NFS_MOUNT_OPTIONS + echo $NFS_MOUNT_OPTIONS ;; cifs) - export MOUNT_OPTIONS=$CIFS_MOUNT_OPTIONS + echo $CIFS_MOUNT_OPTIONS ;; ceph) - export MOUNT_OPTIONS=$CEPHFS_MOUNT_OPTIONS + echo $CEPHFS_MOUNT_OPTIONS ;; glusterfs) - export MOUNT_OPTIONS=$GLUSTERFS_MOUNT_OPTIONS + echo $GLUSTERFS_MOUNT_OPTIONS ;; overlay) - export MOUNT_OPTIONS=$OVERLAY_MOUNT_OPTIONS + echo $OVERLAY_MOUNT_OPTIONS ;; ext2|ext3|ext4|ext4dev) # acls & xattrs aren't turned on by default on ext$FOO - export MOUNT_OPTIONS="-o acl,user_xattr $EXT_MOUNT_OPTIONS" + echo "-o acl,user_xattr $EXT_MOUNT_OPTIONS" ;; f2fs) - export MOUNT_OPTIONS="-o acl,user_xattr $F2FS_MOUNT_OPTIONS" + echo "-o acl,user_xattr $F2FS_MOUNT_OPTIONS" ;; reiserfs) # acls & xattrs aren't turned on by default on reiserfs - export MOUNT_OPTIONS="-o acl,user_xattr $REISERFS_MOUNT_OPTIONS" + echo "-o acl,user_xattr $REISERFS_MOUNT_OPTIONS" ;; reiser4) - # acls & xattrs aren't supported by reiser4 - export MOUNT_OPTIONS=$REISER4_MOUNT_OPTIONS - ;; + # acls & xattrs aren't supported by reiser4 + echo $REISER4_MOUNT_OPTIONS + ;; gfs2) # acls aren't turned on by default on gfs2 - export MOUNT_OPTIONS="-o acl $GFS2_MOUNT_OPTIONS" + echo "-o acl $GFS2_MOUNT_OPTIONS" ;; tmpfs) # We need to specify the size at mount, use 1G by default - export MOUNT_OPTIONS="-o size=1G $TMPFS_MOUNT_OPTIONS" + echo "-o size=1G $TMPFS_MOUNT_OPTIONS" ;; ubifs) - export MOUNT_OPTIONS=$UBIFS_MOUNT_OPTIONS + echo $UBIFS_MOUNT_OPTIONS ;; *) ;; esac } +_mount_opts() +{ + export MOUNT_OPTIONS=$(_common_mount_opts) +} + _test_mount_opts() { - case $FSTYP in - 9p) - export TEST_FS_MOUNT_OPTS=$PLAN9_MOUNT_OPTIONS - ;; - cifs) - export TEST_FS_MOUNT_OPTS=$CIFS_MOUNT_OPTIONS - ;; - ceph) - export TEST_FS_MOUNT_OPTS=$CEPHFS_MOUNT_OPTIONS - ;; - nfs) - export TEST_FS_MOUNT_OPTS=$NFS_MOUNT_OPTIONS - ;; - glusterfs) - export TEST_FS_MOUNT_OPTS=$GLUSTERFS_MOUNT_OPTIONS - ;; - ext2|ext3|ext4|ext4dev) - # acls & xattrs aren't turned on by default on older ext$FOO - export TEST_FS_MOUNT_OPTS="-o acl,user_xattr $EXT_MOUNT_OPTIONS" - ;; - *) - ;; - esac + export TEST_FS_MOUNT_OPTS=$(_common_mount_opts) } _mkfs_opts()
Some cases(e.g. generic/519) check commands/features on test device but do tests on scratch device. If some mount options can impact the check result, these cases may throw error instead if not run when we use different mount options for test and scratch devices. Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com> --- common/config | 65 +++++++++++++++++++-------------------------------- 1 file changed, 24 insertions(+), 41 deletions(-)