| Message ID | 20200803083838.26222-2-yangx.jy@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Make fstests support new behavior of DAX | expand |
On Mon, Aug 03, 2020 at 04:38:32PM +0800, Xiao Yang wrote: > 1) _check_scratch_dax_mountopt() checks old/new dax mount option and > returns a value. > 2) _require_scratch_dax_mountopt() throws notrun if _check_scratch_dax_mountopt() > returns a non-zero value. > 3) _require_dax_iflag() checks FS_XFLAG_DAX. > > Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com> > --- > common/rc | 42 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 42 insertions(+) > > diff --git a/common/rc b/common/rc > index 1b7b2575..d7926bc5 100644 > --- a/common/rc > +++ b/common/rc > @@ -3188,6 +3188,48 @@ _require_scratch_dax() > _scratch_unmount > } > > +# Only accept dax/dax=always mount option becasue dax=always, dax=inode > +# and dax=never are always introduced together. > +# Return 0 if filesystem/device supports the specified dax option. > +# Return 1 if mount fails with the specified dax option. > +# Return 2 if /proc/mounts shows wrong dax option. > +# Check new dax=inode, dax=always or dax=never option by passing "dax=always". > +# Check old dax or new dax=always by passing "dax". > +_check_scratch_dax_mountopt() So I think I see what you are trying to say here but I'm not sure the comment/doc is clear. How about: # Check if dax mount options are supported # # $1 can be either 'dax=always' or 'dax' # # dax=always # Check for the new dax options (dax=inode, dax=always or dax=never) by # passing "dax=always". # dax # Check for the old dax or new dax=always by passing "dax". # # This only accepts 'dax=always' because dax=always, dax=inode # and dax=never are always supported together. So if the other options are # required checking for 'dax=always' indicates support for the other 2. # # Return 0 if filesystem/device supports the specified dax option. # Return 1 if mount fails with the specified dax option. # Return 2 if /proc/mounts shows wrong dax option. Ira > +{ > + local option=$1 > + > + _require_scratch > + _scratch_mkfs > /dev/null 2>&1 > + > + _try_scratch_mount "-o $option" > /dev/null 2>&1 || return 1 > + > + if _fs_options $SCRATCH_DEV | egrep -q "dax(=always|,|$)"; then > + _scratch_unmount > + return 0 > + else > + _scratch_unmount > + return 2 > + fi > +} > + > +# Throw notrun if _check_scratch_dax_mountopt() returns a non-zero value. > +_require_scratch_dax_mountopt() > +{ > + local mountopt=$1 > + > + _check_scratch_dax_mountopt "$mountopt" > + local res=$? > + > + [ $res -eq 1 ] && _notrun "mount $SCRATCH_DEV with $mountopt failed" > + [ $res -eq 2 ] && _notrun "$SCRATCH_DEV $FSTYP does not support -o $mountopt" > +} > + > +_require_dax_iflag() > +{ > + _require_xfs_io_command "chattr" "x" > +} > + > # Does norecovery support by this fs? > _require_norecovery() > { > -- > 2.21.0 > > >
On Mon, Aug 03, 2020 at 12:49:19PM -0700, Ira Weiny wrote: > On Mon, Aug 03, 2020 at 04:38:32PM +0800, Xiao Yang wrote: > > 1) _check_scratch_dax_mountopt() checks old/new dax mount option and > > returns a value. > > 2) _require_scratch_dax_mountopt() throws notrun if _check_scratch_dax_mountopt() > > returns a non-zero value. > > 3) _require_dax_iflag() checks FS_XFLAG_DAX. > > > > Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com> > > --- > > common/rc | 42 ++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 42 insertions(+) > > > > diff --git a/common/rc b/common/rc > > index 1b7b2575..d7926bc5 100644 > > --- a/common/rc > > +++ b/common/rc > > @@ -3188,6 +3188,48 @@ _require_scratch_dax() > > _scratch_unmount > > } > > > > +# Only accept dax/dax=always mount option becasue dax=always, dax=inode > > +# and dax=never are always introduced together. > > +# Return 0 if filesystem/device supports the specified dax option. > > +# Return 1 if mount fails with the specified dax option. > > +# Return 2 if /proc/mounts shows wrong dax option. > > +# Check new dax=inode, dax=always or dax=never option by passing "dax=always". > > +# Check old dax or new dax=always by passing "dax". > > +_check_scratch_dax_mountopt() > > So I think I see what you are trying to say here but I'm not sure the > comment/doc is clear. How about: > > # Check if dax mount options are supported > # > # $1 can be either 'dax=always' or 'dax' > # > # dax=always > # Check for the new dax options (dax=inode, dax=always or dax=never) by > # passing "dax=always". > # dax > # Check for the old dax or new dax=always by passing "dax". > # > # This only accepts 'dax=always' because dax=always, dax=inode > # and dax=never are always supported together. So if the other options are > # required checking for 'dax=always' indicates support for the other 2. > # > # Return 0 if filesystem/device supports the specified dax option. > # Return 1 if mount fails with the specified dax option. > # Return 2 if /proc/mounts shows wrong dax option. /me likes this better. --D > Ira > > > +{ > > + local option=$1 > > + > > + _require_scratch > > + _scratch_mkfs > /dev/null 2>&1 > > + > > + _try_scratch_mount "-o $option" > /dev/null 2>&1 || return 1 > > + > > + if _fs_options $SCRATCH_DEV | egrep -q "dax(=always|,|$)"; then > > + _scratch_unmount > > + return 0 > > + else > > + _scratch_unmount > > + return 2 > > + fi > > +} > > + > > +# Throw notrun if _check_scratch_dax_mountopt() returns a non-zero value. > > +_require_scratch_dax_mountopt() > > +{ > > + local mountopt=$1 > > + > > + _check_scratch_dax_mountopt "$mountopt" > > + local res=$? > > + > > + [ $res -eq 1 ] && _notrun "mount $SCRATCH_DEV with $mountopt failed" > > + [ $res -eq 2 ] && _notrun "$SCRATCH_DEV $FSTYP does not support -o $mountopt" > > +} > > + > > +_require_dax_iflag() > > +{ > > + _require_xfs_io_command "chattr" "x" > > +} > > + > > # Does norecovery support by this fs? > > _require_norecovery() > > { > > -- > > 2.21.0 > > > > > >
On 2020/8/4 6:26, Darrick J. Wong wrote: > On Mon, Aug 03, 2020 at 12:49:19PM -0700, Ira Weiny wrote: >> On Mon, Aug 03, 2020 at 04:38:32PM +0800, Xiao Yang wrote: >>> 1) _check_scratch_dax_mountopt() checks old/new dax mount option and >>> returns a value. >>> 2) _require_scratch_dax_mountopt() throws notrun if _check_scratch_dax_mountopt() >>> returns a non-zero value. >>> 3) _require_dax_iflag() checks FS_XFLAG_DAX. >>> >>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com> >>> --- >>> common/rc | 42 ++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 42 insertions(+) >>> >>> diff --git a/common/rc b/common/rc >>> index 1b7b2575..d7926bc5 100644 >>> --- a/common/rc >>> +++ b/common/rc >>> @@ -3188,6 +3188,48 @@ _require_scratch_dax() >>> _scratch_unmount >>> } >>> >>> +# Only accept dax/dax=always mount option becasue dax=always, dax=inode >>> +# and dax=never are always introduced together. >>> +# Return 0 if filesystem/device supports the specified dax option. >>> +# Return 1 if mount fails with the specified dax option. >>> +# Return 2 if /proc/mounts shows wrong dax option. >>> +# Check new dax=inode, dax=always or dax=never option by passing "dax=always". >>> +# Check old dax or new dax=always by passing "dax". >>> +_check_scratch_dax_mountopt() >> So I think I see what you are trying to say here but I'm not sure the >> comment/doc is clear. How about: >> >> # Check if dax mount options are supported >> # >> # $1 can be either 'dax=always' or 'dax' >> # >> # dax=always >> # Check for the new dax options (dax=inode, dax=always or dax=never) by >> # passing "dax=always". >> # dax >> # Check for the old dax or new dax=always by passing "dax". >> # >> # This only accepts 'dax=always' because dax=always, dax=inode >> # and dax=never are always supported together. So if the other options are >> # required checking for 'dax=always' indicates support for the other 2. >> # >> # Return 0 if filesystem/device supports the specified dax option. >> # Return 1 if mount fails with the specified dax option. >> # Return 2 if /proc/mounts shows wrong dax option. > /me likes this better. Hi Ira, Darrick It is clearer and better, I will update the comment in v9 patch set. :-) Besides, could you help me review another new test which has been sent to mail list: "generic: Verify how to change the S_DAX flag on an existing file" I want to squash it into v9 patch set after you review it. Thanks, Xiao Yang > --D > >> Ira >> >>> +{ >>> + local option=$1 >>> + >>> + _require_scratch >>> + _scratch_mkfs> /dev/null 2>&1 >>> + >>> + _try_scratch_mount "-o $option"> /dev/null 2>&1 || return 1 >>> + >>> + if _fs_options $SCRATCH_DEV | egrep -q "dax(=always|,|$)"; then >>> + _scratch_unmount >>> + return 0 >>> + else >>> + _scratch_unmount >>> + return 2 >>> + fi >>> +} >>> + >>> +# Throw notrun if _check_scratch_dax_mountopt() returns a non-zero value. >>> +_require_scratch_dax_mountopt() >>> +{ >>> + local mountopt=$1 >>> + >>> + _check_scratch_dax_mountopt "$mountopt" >>> + local res=$? >>> + >>> + [ $res -eq 1 ]&& _notrun "mount $SCRATCH_DEV with $mountopt failed" >>> + [ $res -eq 2 ]&& _notrun "$SCRATCH_DEV $FSTYP does not support -o $mountopt" >>> +} >>> + >>> +_require_dax_iflag() >>> +{ >>> + _require_xfs_io_command "chattr" "x" >>> +} >>> + >>> # Does norecovery support by this fs? >>> _require_norecovery() >>> { >>> -- >>> 2.21.0 >>> >>> >>> > > . >
diff --git a/common/rc b/common/rc index 1b7b2575..d7926bc5 100644 --- a/common/rc +++ b/common/rc @@ -3188,6 +3188,48 @@ _require_scratch_dax() _scratch_unmount } +# Only accept dax/dax=always mount option becasue dax=always, dax=inode +# and dax=never are always introduced together. +# Return 0 if filesystem/device supports the specified dax option. +# Return 1 if mount fails with the specified dax option. +# Return 2 if /proc/mounts shows wrong dax option. +# Check new dax=inode, dax=always or dax=never option by passing "dax=always". +# Check old dax or new dax=always by passing "dax". +_check_scratch_dax_mountopt() +{ + local option=$1 + + _require_scratch + _scratch_mkfs > /dev/null 2>&1 + + _try_scratch_mount "-o $option" > /dev/null 2>&1 || return 1 + + if _fs_options $SCRATCH_DEV | egrep -q "dax(=always|,|$)"; then + _scratch_unmount + return 0 + else + _scratch_unmount + return 2 + fi +} + +# Throw notrun if _check_scratch_dax_mountopt() returns a non-zero value. +_require_scratch_dax_mountopt() +{ + local mountopt=$1 + + _check_scratch_dax_mountopt "$mountopt" + local res=$? + + [ $res -eq 1 ] && _notrun "mount $SCRATCH_DEV with $mountopt failed" + [ $res -eq 2 ] && _notrun "$SCRATCH_DEV $FSTYP does not support -o $mountopt" +} + +_require_dax_iflag() +{ + _require_xfs_io_command "chattr" "x" +} + # Does norecovery support by this fs? _require_norecovery() {
1) _check_scratch_dax_mountopt() checks old/new dax mount option and returns a value. 2) _require_scratch_dax_mountopt() throws notrun if _check_scratch_dax_mountopt() returns a non-zero value. 3) _require_dax_iflag() checks FS_XFLAG_DAX. Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com> --- common/rc | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)