On Thu, 2019-12-19 at 16:39 +1100, NeilBrown wrote: > On Thu, Dec 19 2019, Trond Myklebust wrote: > > > On Thu, 2019-12-19 at 13:56 +1100, NeilBrown wrote: > > > On Wed, Dec 18 2019, Trond Myklebust wrote: > > > > > > > On Thu, 2019-12-19 at 09:47 +1100, NeilBrown wrote: > > > > > If an NFSv4.1 server doesn't support > > > > > NFS4_OPEN_CLAIM_DELEG_CUR_FH > > > > > (e.g. Linux 3.0), and a newer NFS client tries to use it to > > > > > claim > > > > > an open before returning a delegation, the server might > > > > > return > > > > > NFS4ERR_BADXDR. > > > > > That is what Linux 3.0 does, though the RFC doesn't seem to > > > > > be > > > > > explicit > > > > > on which flags must be supported, and what error can be > > > > > returned > > > > > for > > > > > unsupported flags. > > > > > > > > NFS4ERR_BADXDR is defined in RFC5661, section 15.1.1.1 as > > > > meaning > > > > > > > > "The arguments for this operation do not match those specified > > > > in > > > > the > > > > XDR definition." > > > > > > > > That's clearly not the case here, so I'd chalk this down to a > > > > fairly > > > > blatant server bug, at which point it makes no sense to fix it > > > > in > > > > the > > > > client. > > > > > > Ok, but the RFC seems to suggest it is OK to not support this > > > flag, > > > so > > > suppose I fixed the server to return NFS4ERR_NOTSUPP instead. > > > The client still wouldn't handle this response gracefully. > > > > > > > NFS4ERR_NOTSUPP is wrong too as the OPEN operation is clearly > > supported. The only error that might make sense is NFS4ERR_INVAL: > > > > "15.1.1.4. NFS4ERR_INVAL (Error Code 22) > > > > The arguments for this operation are not valid for some reason, > > even > > though they do match those specified in the XDR definition for > > the > > request." > > > > That said, why do we care about supporting NFSv4.1 on this server? > > It > > is clearly broken. > > I care about it because a customer has a support contract, but that > isn't your problem. > > I would think "we" care about it because we want to support the spec, > and the spec (RFC 5661 section 2.4) says: > > where the > server > supports neither the CLAIM_DELEGATE_PREV nor CLAIM_DELEG_CUR_FH > claim > types Given the context, I think that is actually a typo. It looks to me like it is talking about CLAIM_DELEGATE_PREV and CLAIM_DELEG_PREV_FH, since otherwise the talk about releasing delegation state when establishing a new lease makes no sense. > Also you have code in the client to handle the possibility that an > NFSv4.1 or later server might not handle some features of OPEN. > Three separate features are grouped under "NFS_CAP_ATOMIC_OPEN_V1": > If this isn't set, we fall back: > > case NFS4_OPEN_CLAIM_FH: > return NFS4_OPEN_CLAIM_NULL; > case NFS4_OPEN_CLAIM_DELEG_CUR_FH: > return NFS4_OPEN_CLAIM_DELEGATE_CUR; > case NFS4_OPEN_CLAIM_DELEG_PREV_FH: > return NFS4_OPEN_CLAIM_DELEGATE_PREV; > Right. That's a convenience for downgrading NFSv4.1 service to what is supported by NFSv4.0. > However nfs4_map_atomic_open_claim() is not called when > NFS4_OPEN_CLAIM_DELEG_CUR_FH is tried, and fails. This appears > to be an omission in the code. > It is deliberate. There really isn't anything that describes what is and isn't mandatory to implement in NFSv4.1, but if we have to make everything optional, then we're going to have to add a lot of mostly unnecessary complexity to the client. At what point do we then stop? Do we support a NFSv4.1 server that implements no NFSv4.1 features? Why not just let the client downgrade to NFSv4.0 in that case? -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@xxxxxxxxxxxxxxx
