Fwd: [PATCH] common/rc: add the function _require_noatime

[Steve French <smfrench@gmail.com>]

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