Re: [PATCH] nfs4: don't map EACCESS and EPERM to EIO

["Mkrtchyan, Tigran" <tigran.mkrtchyan@desy.de>]

[-- Attachment #1: Type: text/plain, Size: 3120 bytes --]


Hi Trond,

Obviously, the patch is incorrect. The behavior of the upstream kernel and
RHEL kernels are different.

Sorry for the noise,
  Tigran.


----- Original Message -----
> From: "Trond Myklebust" <trondmy@hammerspace.com>
> To: anna@kernel.org, "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de>
> Cc: "linux-nfs" <linux-nfs@vger.kernel.org>
> Sent: Thursday, 8 June, 2023 19:53:07
> Subject: Re: [PATCH] nfs4: don't map EACCESS and EPERM to EIO

> On Thu, 2023-06-08 at 19:42 +0200, Tigran Mkrtchyan wrote:
>> Hi Trond,
>> 
>> I will check and let you know. What we see is EACCESS on layoutget
>> reported as EIO to the applications
>> 
> 
> If this is for a write, then that might just be
> nfs_mapping_set_error(). In newer kernels, it tries to avoid sending
> errors that are unexpected for strictly POSIX applications.
> 
> Cheers
>  Trond
> 
>> Best regards,
>> Tigran
>> 
>> 
>> On June 8, 2023 5:33:16 PM GMT+02:00, Trond Myklebust
>> <trondmy@hammerspace.com> wrote:
>> > Hi Tigran,
>> > 
>> > On Thu, 2023-06-08 at 16:49 +0200, Tigran Mkrtchyan wrote:
>> > > the nfs4_map_errors function converts NFS specific errors to
>> > > userland
>> > > errors. However, it ignores NFS4ERR_PERM and EPERM, which then
>> > > get
>> > > mapped to EIO.
>> > > 
>> > > Signed-off-by: Tigran Mkrtchyan
>> > > <tigran.mkrtchyan@desy.de> fs/nfs/nfs4proc.c | 2 ++
>> > >  1 file changed, 2 insertions(+)
>> > > 
>> > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> > > index d3665390c4cb..795205fe4f30 100644
>> > > --- a/fs/nfs/nfs4proc.c
>> > > +++ b/fs/nfs/nfs4proc.c
>> > > @@ -171,12 +171,14 @@ static int nfs4_map_errors(int err)
>> > >         case -NFS4ERR_LAYOUTTRYLATER:
>> > >         case -NFS4ERR_RECALLCONFLICT:
>> > >                 return -EREMOTEIO;
>> > > +       case -NFS4ERR_PERM:
>> > >         case -NFS4ERR_WRONGSEC:
>> > >         case -NFS4ERR_WRONG_CRED:
>> > >                 return -EPERM;
>> > >         case -NFS4ERR_BADOWNER:
>> > >         case -NFS4ERR_BADNAME:
>> > >                 return -EINVAL;
>> > > +       case -NFS4ERR_ACCESS:
>> > >         case -NFS4ERR_SHARE_DENIED:
>> > >                 return -EACCES;
>> > >         case -NFS4ERR_MINOR_VERS_MISMATCH:
>> > > 
>> > 
>> > Hmm... Aren't both these cases covered by the exception at the top
>> > of
>> > the function?
>> > 
>> > static int nfs4_map_errors(int err)
>> > {
>> >         if (err >= -1000)
>> >                 return err;
>> > 
>> > As I read it, that should mean that err = -NFS4ERR_ACCESS (= -13)
>> > and
>> > err = -NFS4ERR_PERM (= -1) will get returned verbatim.
>> > 
>> > Are you seeing these NFS4ERR_ACCESS and NFS4ERR_PERM cases hitting
>> > the
>> > default: dprintk() when you turn it on?
>> > 
> 
> --
> Trond Myklebust
> CTO, Hammerspace Inc
> 1900 S Norfolk St, Suite 350 - #45
> San Mateo, CA 94403
> 
> www.hammerspace.com

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 2208 bytes --]