On Mon, 2024-09-09 at 09:05 +0300, Roi Azarzar wrote: > Isn't the following clearer? > if ((res.attr_bitmask[2] & > FATTR4_WORD2_TIME_DELEG_MODIFY) && > (res.open_caps.oa_share_access_want[0] & > NFS4_SHARE_WANT_DELEG_TIMESTAMPS)) > server->caps |= NFS_CAP_DELEGTIME; > > What about TIME_DELEG_ACCESS? shouldn't we add it to the above > condition in the same manner ? Sure. In any case you are talking about support for a completely stupid server implementation here, because it makes zero sense for a server to support TIME_DELEG_MODIFY or TIME_DELEG_ACCESS without also supporting the WANT_DELEG_TIMESTAMPS flag. However if we're going to be pedantic, then let's do it properly. > > On Sun, Sep 8, 2024 at 7:55 PM Trond Myklebust > <trondmy@xxxxxxxxxxxxxxx> wrote: > > > > On Sun, 2024-09-08 at 17:34 +0300, Roi Azarzar wrote: > > > Hi, > > > > > > as discussed with @Jeff Layton sending a suggested patch that > > > aims to > > > fix NFS_CAP_DELEGTIME capability indication in the client side by > > > setting it according to FATTR4_OPEN_ARGUMENTS response (and not > > > according to TIME_DELEG_MODIFY) support as draft-ietf-nfsv4- > > > delstid- > > > 02 > > > suggested. > > > > > > > NACK. I agree that we should turn off NFS_CAP_DELEGTIME if the open > > arguments don't support it, but we should not turn it on unless the > > server has indicated that it supports the attribute. > > > > i.e. the correct change here is > > > > + if (!(res.open_caps.oa_share_access_want[0] & > > + NFS4_SHARE_WANT_DELEG_TIMESTAMPS)) > > + server->caps &= ~NFS_CAP_DELEGTIME; > > > > -- > > Trond Myklebust > > Linux NFS client maintainer, Hammerspace > > trond.myklebust@xxxxxxxxxxxxxxx > > > > > -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@xxxxxxxxxxxxxxx
