On Thu, 2024-10-17 at 18:39 -0400, Olga Kornievskaia wrote:
> Seeing strangeness in a network trace with this patch series where
> SETATTR is sent with time_deleg_access and server is returning with
> EINVAL. Test is open() with read delegation, triggering a cb_recall
> via a local access. I can see that the client has changed from sending
> just a delegreturn to sending a setattr+delegreturn. Is there no
> server support and this is normal to return EINVAL.
>
No, that's a server bug. I think it's this in nfsd4_setattr:
if (deleg_attrs || (setattr->sa_iattr.ia_valid & ATTR_SIZE)) {
status = nfs4_preprocess_stateid_op(rqstp, cstate,
&cstate->current_fh, &setattr->sa_stateid,
WR_STATE, NULL, &st);
if (status)
return status;
}
We're asking for a WR_STATE in the nfs4_preprocess_stateid_op, but
there isn't one. There is only a read delegation, so we get back
BAD_STATEID and that eventually runs into -EINVAL. I'll need to look
over this and figure out how to fix it properly.
Thanks for the bug report.
> On Mon, Oct 14, 2024 at 3:27 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> >
> > This patchset is an update to the delstid patches that went into Chuck's
> > nfsd-next branch recently. The original versions of the spec left out
> > OPEN_DELEGATE_READ_ATTRS_DELEG and OPEN_DELEGATE_WRITE_ATTRS_DELEG. This
> > set adds proper support for them.
> >
> > My suggestion is to drop these two patches from nfsd-next:
> >
> > 544c67cc0f26 nfsd: handle delegated timestamps in SETATTR
> > eee2c04ca5c1 nfsd: add support for delegated timestamps
> >
> > ...and then apply this set on top of the remaining pile. The resulting
> > set is a bit larger than the original, as I took the liberty of adding
> > some more symbols to the autogenerated part of the spec.
> >
> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> > ---
> > Jeff Layton (6):
> > nfsd: drop inode parameter from nfsd4_change_attribute()
> > nfsd: switch to autogenerated definitions for open_delegation_type4
> > nfsd: rename NFS4_SHARE_WANT_* constants to OPEN4_SHARE_ACCESS_WANT_*
> > nfsd: prepare delegation code for handing out *_ATTRS_DELEG delegations
> > nfsd: add support for delegated timestamps
> > nfsd: handle delegated timestamps in SETATTR
> >
> > Documentation/sunrpc/xdr/nfs4_1.x | 22 ++++-
> > fs/nfsd/nfs4callback.c | 42 ++++++++-
> > fs/nfsd/nfs4proc.c | 26 ++++-
> > fs/nfsd/nfs4state.c | 178 ++++++++++++++++++++++++++---------
> > fs/nfsd/nfs4xdr.c | 57 ++++++++---
> > fs/nfsd/nfs4xdr_gen.c | 19 +++-
> > fs/nfsd/nfs4xdr_gen.h | 2 +-
> > fs/nfsd/nfsd.h | 2 +
> > fs/nfsd/nfsfh.c | 11 +--
> > fs/nfsd/nfsfh.h | 3 +-
> > fs/nfsd/state.h | 18 ++++
> > fs/nfsd/xdr4cb.h | 10 +-
> > include/linux/nfs4.h | 2 +-
> > include/linux/sunrpc/xdrgen/nfs4_1.h | 35 ++++++-
> > include/linux/time64.h | 5 +
> > 15 files changed, 348 insertions(+), 84 deletions(-)
> > ---
> > base-commit: 9f8009c5be9367d01cd1627d6a379b4c642d8a28
> > change-id: 20241014-delstid-bf05220ad941
> >
> > Best regards,
> > --
> > Jeff Layton <jlayton@xxxxxxxxxx>
> >
> >
--
Jeff Layton <jlayton@xxxxxxxxxx>
