* [PATCH] common/rc: add the function _require_noatime @ 2018-05-18 11:10 XiaoLi Feng 2018-05-21 7:50 ` Eryu Guan 0 siblings, 1 reply; 6+ messages in thread From: XiaoLi Feng @ 2018-05-18 11:10 UTC (permalink / raw) To: fstests; +Cc: xiaoli feng From: xiaoli feng <xifeng@redhat.com> 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@redhat.com> --- 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 +} + +_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 #executable file echo "*** copying file ***" -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] common/rc: add the function _require_noatime 2018-05-18 11:10 [PATCH] common/rc: add the function _require_noatime XiaoLi Feng @ 2018-05-21 7:50 ` Eryu Guan 2018-05-21 8:50 ` Fwd: " Xiaoli Feng 0 siblings, 1 reply; 6+ messages in thread From: Eryu Guan @ 2018-05-21 7:50 UTC (permalink / raw) To: XiaoLi Feng; +Cc: fstests [add linux-cifs@vger.kernel.org to cc list] On Fri, May 18, 2018 at 07:10:29PM +0800, Xiaoli Feng wrote: > From: xiaoli feng <xifeng@redhat.com> > > 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@redhat.com> > --- > 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 6+ messages in thread
* Fwd: [PATCH] common/rc: add the function _require_noatime 2018-05-21 7:50 ` Eryu Guan @ 2018-05-21 8:50 ` Xiaoli Feng 2018-05-21 23:54 ` Steve French 0 siblings, 1 reply; 6+ messages in thread From: Xiaoli Feng @ 2018-05-21 8:50 UTC (permalink / raw) To: guaneryu; +Cc: linux-cifs, fstests [add linux-cifs@vger.kernel.org to cc list] ----- Forwarded Message ----- From: "Eryu Guan" <guaneryu@gmail.com> To: "XiaoLi Feng" <xifeng@redhat.com> Cc: fstests@vger.kernel.org Sent: Monday, May 21, 2018 3:50:27 PM Subject: Re: [PATCH] common/rc: add the function _require_noatime [add linux-cifs@vger.kernel.org to cc list] On Fri, May 18, 2018 at 07:10:29PM +0800, Xiaoli Feng wrote: > From: xiaoli feng <xifeng@redhat.com> > > 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@redhat.com> > --- > 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@vger.kernel.org > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] common/rc: add the function _require_noatime 2018-05-21 8:50 ` Fwd: " Xiaoli Feng @ 2018-05-21 23:54 ` Steve French 2018-05-22 2:21 ` Eryu Guan 2018-05-22 2:54 ` Xiaoli Feng 0 siblings, 2 replies; 6+ messages in thread From: Steve French @ 2018-05-21 23:54 UTC (permalink / raw) To: Xiaoli Feng; +Cc: Eryu Guan, CIFS, fstests 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 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. On Mon, May 21, 2018 at 3:50 AM, Xiaoli Feng <xifeng@redhat.com> wrote: > > [add linux-cifs@vger.kernel.org to cc list] > > ----- Forwarded Message ----- > From: "Eryu Guan" <guaneryu@gmail.com> > To: "XiaoLi Feng" <xifeng@redhat.com> > Cc: fstests@vger.kernel.org > Sent: Monday, May 21, 2018 3:50:27 PM > Subject: Re: [PATCH] common/rc: add the function _require_noatime > > [add linux-cifs@vger.kernel.org to cc list] > > On Fri, May 18, 2018 at 07:10:29PM +0800, Xiaoli Feng wrote: >> From: xiaoli feng <xifeng@redhat.com> >> >> 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@redhat.com> >> --- >> 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@vger.kernel.org >> 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@vger.kernel.org > 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Thanks, Steve ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] common/rc: add the function _require_noatime 2018-05-21 23:54 ` Steve French @ 2018-05-22 2:21 ` Eryu Guan 2018-05-22 2:54 ` Xiaoli Feng 1 sibling, 0 replies; 6+ messages in thread From: Eryu Guan @ 2018-05-22 2:21 UTC (permalink / raw) To: Steve French; +Cc: Xiaoli Feng, CIFS, fstests 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@redhat.com> wrote: > > > > [add linux-cifs@vger.kernel.org to cc list] > > > > ----- Forwarded Message ----- > > From: "Eryu Guan" <guaneryu@gmail.com> > > To: "XiaoLi Feng" <xifeng@redhat.com> > > Cc: fstests@vger.kernel.org > > Sent: Monday, May 21, 2018 3:50:27 PM > > Subject: Re: [PATCH] common/rc: add the function _require_noatime > > > > [add linux-cifs@vger.kernel.org to cc list] > > > > On Fri, May 18, 2018 at 07:10:29PM +0800, Xiaoli Feng wrote: > >> From: xiaoli feng <xifeng@redhat.com> > >> > >> 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@redhat.com> > >> --- > >> 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@vger.kernel.org > >> 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@vger.kernel.org > > 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@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > -- > Thanks, > > Steve ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] common/rc: add the function _require_noatime 2018-05-21 23:54 ` Steve French 2018-05-22 2:21 ` Eryu Guan @ 2018-05-22 2:54 ` Xiaoli Feng 1 sibling, 0 replies; 6+ messages in thread From: Xiaoli Feng @ 2018-05-22 2:54 UTC (permalink / raw) To: Steve French; +Cc: Eryu Guan, CIFS, fstests ----- Original Message ----- > From: "Steve French" <smfrench@gmail.com> > To: "Xiaoli Feng" <xifeng@redhat.com> > Cc: "Eryu Guan" <guaneryu@gmail.com>, "CIFS" <linux-cifs@vger.kernel.org>, fstests@vger.kernel.org > Sent: Tuesday, May 22, 2018 7:54:45 AM > Subject: Re: [PATCH] common/rc: add the function _require_noatime > > Should this be fixed by changing cifs.ko to accept the mount parm but ignore > it? Yes, it should. For cifs, atime/diratime/relatime/norelatime/nostrictatime can be mounted without effect. But mount failed with noatime/nodiatime/strictatime. Seems they don't maintain consistency. I will file a bug about it. Thanks Steve & Eryu. > > If this test works on NFS (the noatime mount option has no meaning for > NFS apparently) we should do the same > > 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. > > On Mon, May 21, 2018 at 3:50 AM, Xiaoli Feng <xifeng@redhat.com> wrote: > > > > [add linux-cifs@vger.kernel.org to cc list] > > > > ----- Forwarded Message ----- > > From: "Eryu Guan" <guaneryu@gmail.com> > > To: "XiaoLi Feng" <xifeng@redhat.com> > > Cc: fstests@vger.kernel.org > > Sent: Monday, May 21, 2018 3:50:27 PM > > Subject: Re: [PATCH] common/rc: add the function _require_noatime > > > > [add linux-cifs@vger.kernel.org to cc list] > > > > On Fri, May 18, 2018 at 07:10:29PM +0800, Xiaoli Feng wrote: > >> From: xiaoli feng <xifeng@redhat.com> > >> > >> 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@redhat.com> > >> --- > >> 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@vger.kernel.org > >> 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@vger.kernel.org > > 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@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > -- > Thanks, > > Steve > -- > 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 > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-05-22 2:54 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-05-18 11:10 [PATCH] common/rc: add the function _require_noatime XiaoLi Feng 2018-05-21 7:50 ` Eryu Guan 2018-05-21 8:50 ` Fwd: " Xiaoli Feng 2018-05-21 23:54 ` Steve French 2018-05-22 2:21 ` Eryu Guan 2018-05-22 2:54 ` Xiaoli Feng
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox