On Fri, 2018-03-16 at 10:31 +0100, Mkrtchyan, Tigran wrote: > Hi Neil, > > according to rfc5661, NFS4ERR_INVAL is returned by the server if it > thinks that client sends an invalid request (e.g. points to a client > bug) > or server misinterpret it (broken server). > > With your change instead of failing the mount, client will silently > go for > v4.0, even v4.1 mount was requested and produce undesirable behavior, > e.g. > proxy-io instead of pnfs. I fill prefer fail-fast instead of long > debug > sessions. > > On the other hand, I understand, that it's not always possible to fix > server > or clients in production environment and time-to-time workarounds are > necessary. > > I'd tend to agree with Tigran. Hiding server bugs, should not be a priority and particularly not in this case, where the workaround is simple: either turn off version negotiation altogether, or edit /etc/nfsmount.conf to negotiate a different set of versions. What we might want to do, is make it easier to allow the user to detect that this is indeed a server bug and is not a problem with the arguments supplied to the "mount" utility. Perhaps we might have the kernel log something in the syslogs? Cheers Trond > Tigran. > > > ----- Original Message ----- > > From: "NeilBrown" <neilb@xxxxxxxx> > > To: "Trond Myklebust" <trond.myklebust@xxxxxxxxxxxxxxx>, "Anna > > Schumaker" <anna.schumaker@xxxxxxxxxx> > > Cc: "linux-nfs" <linux-nfs@xxxxxxxxxxxxxxx> > > Sent: Friday, March 16, 2018 12:44:23 AM > > Subject: [PATCH - v2] NFSv4: handle EINVAL from EXCHANGE_ID better. > > nfs4_proc_exchange_id() can return -EINVAL if the server > > reported NFS4INVAL (which I have seen in a packet trace), > > or nfs4_check_cl_exchange_flags() exchange flags detects > > a problem. > > Each of these mean that NFSv4.1 later cannot be use, but > > they should not prevent fallback to NFSv4.0. > > > > Currently this EINVAL error is returned by nfs4_proc_exchange_id() > > to > > nfs41_discover_server_trunking(), and thence to > > nfs4_discover_server_trunking(). > > nfs4_discover_server_trunking() doesn't understand EINVAL, so > > converts > > it to EIO which causes mount.nfs to think something is horribly > > wrong > > and to give up. > > > > It would be a more graceful failure if > > nfs4_discover_server_trunking() > > mapped -EINVAL to -EPROTONOSUPPORT - a failure to negotiate a > > client > > ID clearly shows that NFSv4.1 cannot be supported, but isn't as > > general a failure as EIO. > > > > Signed-off-by: NeilBrown <neilb@xxxxxxxx> > > --- > > > > Sorry - a bit too trigger-happy with that first version of the > > patch. > > > > NeilBrown > > > > fs/nfs/nfs4state.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c > > index 91a4d4eeb235..b988e460553d 100644 > > --- a/fs/nfs/nfs4state.c > > +++ b/fs/nfs/nfs4state.c > > @@ -2219,6 +2219,8 @@ int nfs4_discover_server_trunking(struct > > nfs_client *clp, > > clnt = clp->cl_rpcclient; > > goto again; > > > > + case -NFS4ERR_INVAL: > > + /* Server confused - assume this minor isn't > > supported */ > > case -NFS4ERR_MINOR_VERS_MISMATCH: > > status = -EPROTONOSUPPORT; > > break; > > -- > > 2.14.0.rc0.dirty > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" > in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Trond Myklebust Linux NFS client maintainer, PrimaryData trond.myklebust@xxxxxxxxxxxxxxx ��.n��������+%������w��{.n�����{��w���jg��������ݢj����G�������j:+v���w�m������w�������h�����٥