All of lore.kernel.org
 help / color / mirror / Atom feed
* [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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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-21 23:55       ` Fwd: " Steve French
                         ` (2 more replies)
  0 siblings, 3 replies; 7+ 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] 7+ messages in thread

* Fwd: [PATCH] common/rc: add the function _require_noatime
  2018-05-21 23:54     ` Steve French
@ 2018-05-21 23:55       ` Steve French
  2018-05-22  2:21       ` Eryu Guan
  2018-05-22  2:54       ` Xiaoli Feng
  2 siblings, 0 replies; 7+ messages in thread
From: Steve French @ 2018-05-21 23:55 UTC (permalink / raw)
  To: linux-fsdevel

Adding fsdevel for this question


---------- Forwarded message ----------
From: Steve French <smfrench@gmail.com>
Date: Mon, May 21, 2018 at 6:54 PM
Subject: Re: [PATCH] common/rc: add the function _require_noatime
To: Xiaoli Feng <xifeng@redhat.com>
Cc: Eryu Guan <guaneryu@gmail.com>, CIFS <linux-cifs@vger.kernel.org>,
fstests@vger.kernel.org


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


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] common/rc: add the function _require_noatime
  2018-05-21 23:54     ` Steve French
  2018-05-21 23:55       ` Fwd: " Steve French
@ 2018-05-22  2:21       ` Eryu Guan
  2018-05-22  2:54       ` Xiaoli Feng
  2 siblings, 0 replies; 7+ 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] 7+ messages in thread

* Re: [PATCH] common/rc: add the function _require_noatime
  2018-05-21 23:54     ` Steve French
  2018-05-21 23:55       ` Fwd: " Steve French
  2018-05-22  2:21       ` Eryu Guan
@ 2018-05-22  2:54       ` Xiaoli Feng
  2 siblings, 0 replies; 7+ 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] 7+ messages in thread

end of thread, other threads:[~2018-05-22  2:54 UTC | newest]

Thread overview: 7+ 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-21 23:55       ` Fwd: " Steve French
2018-05-22  2:21       ` Eryu Guan
2018-05-22  2:54       ` Xiaoli Feng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.