On Mon, Dec 06, 2021 at 10:30:45PM +0000, Chuck Lever III wrote: > OK, this is really confusing. > > 5142 set_deny(open->op_share_deny, stp); > 5143 fp->fi_share_deny |= (open->op_share_deny & NFS4_SHARE_DENY_BOTH); > > Here set_deny() is treating the contents of open->op_share_deny > as bit positions, but then upon return NFS4_SHARE_DENY_BOTH > is used directly as a bit mask. Am I reading this correctly? > > But that's not your problem, so I'll let that be. This is weird but intentional. For most practical purposes, fi_share_deny is all that matters. BUT, there is also this language in the spec for OPEN_DOWNGRADE: https://datatracker.ietf.org/doc/html/rfc5661#section-18.18.3 The bits in share_deny SHOULD equal the union of the share_deny bits specified for some subset of the OPENs in effect for the current open-owner on the current file. If the above constraints are not respected, the server SHOULD return the error NFS4ERR_INVAL. If you open a file twice, once with DENY_READ, once with DENY_WRITE, then that is not *quite* the same as opening it once with DENY_BOTH. In the former case, you're allowed to, for example, downgrade to DENY_READ. In the latter, you're not. So if we want to the server to follow that SHOULD, we need to remember not only that the union of all the DENYs so far, you also need to remember the different DENY modes that different OPENs were done with. So, we also keep the st_deny_bmap with that information. The same goes for allow bits (hence there's also an st_access_bmap). It's arguably a lot of extra busy work just for one SHOULD that has no justification other than just to be persnickety about client behavior.... --b.
