On Mon, May 21, 2018 at 06:54:45PM -0500, Steve French wrote:
> Should this be fixed by changing cifs.ko to accept the mount parm but ignore it?
>
> If this test works on NFS (the noatime mount option has no meaning for
> NFS apparently) we should do the same
NFS works because tests that call _require_atime will _notrun on NFS.
>
> Quoting the NFS man page:
>
> In particular, the atime/noatime, diratime/nodiratime, relatime/norela‐
> time, and strictatime/nostrictatime mount options have no effect on NFS
> mounts.
Yeah, the NFS check in _require_atime was based on this description. If
atime related mount options have no effect on CIFS too, we could simply,
from fstests' prospect of view, _notrun for CIFS in _require_atime too.
Thanks,
Eryu
>
> On Mon, May 21, 2018 at 3:50 AM, Xiaoli Feng <xifeng@xxxxxxxxxx> wrote:
> >
> > [add linux-cifs@xxxxxxxxxxxxxxx to cc list]
> >
> > ----- Forwarded Message -----
> > From: "Eryu Guan" <guaneryu@xxxxxxxxx>
> > To: "XiaoLi Feng" <xifeng@xxxxxxxxxx>
> > Cc: fstests@xxxxxxxxxxxxxxx
> > Sent: Monday, May 21, 2018 3:50:27 PM
> > Subject: Re: [PATCH] common/rc: add the function _require_noatime
> >
> > [add linux-cifs@xxxxxxxxxxxxxxx to cc list]
> >
> > On Fri, May 18, 2018 at 07:10:29PM +0800, Xiaoli Feng wrote:
> >> From: xiaoli feng <xifeng@xxxxxxxxxx>
> >>
> >> In the generic/120, it will make the test not-pass if the filesystem
> >> mounts failed with noatime. Now change this result to norun. The
> >> filesystem cifs doesn't support noatime. Just make the test norun
> >> until it supports noatime.
> >>
> >> Signed-off-by: xiaoli feng <xifeng@xxxxxxxxxx>
> >> ---
> >> common/rc | 8 +++++++-
> >> tests/generic/120 | 7 +------
> >> 2 files changed, 8 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/common/rc b/common/rc
> >> index ffe5323..9c45f1b 100644
> >> --- a/common/rc
> >> +++ b/common/rc
> >> @@ -3244,7 +3244,13 @@ _require_atime()
> >> _exclude_scratch_mount_option "noatime"
> >> if [ "$FSTYP" == "nfs" ]; then
> >> _notrun "atime related mount options have no effect on NFS"
> >> - fi
> >
> > I'm not sure what's the expected behavior from CIFS on atime/noatime, so
> > I added linux-cifs list for input.
> >
> > If CIFS behaves similarly to NFS, looks like that you could simply add
> > another check for cifs in _require_atime(), as what we already do for
> > nfs, so all tests that _require_atime() will _notrun on cifs.
> >
> >> +}
> >> +
> >> +_require_noatime()
> >> +{
> >> + _exclude_scratch_mount_option "atime"
> >> + _try_scratch_mount -o noatime || \
> >> + _notrun "noatime not supported by the current tested filesystem"
> >> }
> >>
> >> _require_relatime()
> >> diff --git a/tests/generic/120 b/tests/generic/120
> >> index 1180c10..ddd61b3 100755
> >> --- a/tests/generic/120
> >> +++ b/tests/generic/120
> >> @@ -60,12 +60,7 @@ _compare_access_times()
> >>
> >> }
> >>
> >> -if ! _try_scratch_mount "-o noatime" >$tmp.out 2>&1
> >> -then
> >> - cat $tmp.out
> >> - echo "!!! mount failed"
> >> - exit
> >> -fi
> >> +_require_noatime
> >
> > Anyway, failing the test when "noatime" mount fails is one of the
> > purposes of the test, and it shouldn't be removed, as we've already made
> > sure current FSTYP supports atime/noatime (by _require rules), so a
> > noatime mount failure indicates a bug in the filesystem.
> >
> > Thanks,
> > Eryu
> >
> >>
> >> #executable file
> >> echo "*** copying file ***"
> >> --
> >> 1.8.3.1
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe fstests" in
> >> the body of a message to majordomo@xxxxxxxxxxxxxxx
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
> > --
> > To unsubscribe from this list: send the line "unsubscribe fstests" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Thanks,
>
> Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
