On Fri, Sep 29, 2023 at 12:21:28PM -0400, Jeff Layton wrote:
> On Fri, 2023-09-29 at 09:59 -0400, Chuck Lever wrote:
> > From: Chuck Lever <chuck.lever@xxxxxxxxxx>
> >
> > Refactor nfsd4_encode_open() so the open_read_delegation4 type is
> > encoded in a separate function. This makes it more straightforward
> > to later add support for returning an nfsace4 in OPEN responses that
> > offer a delegation.
> >
> > Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> > ---
> > fs/nfsd/nfs4state.c | 6 +++--
> > fs/nfsd/nfs4xdr.c | 61 ++++++++++++++++++++++++++++++++++++++-------------
> > fs/nfsd/xdr4.h | 2 +-
> > 3 files changed, 49 insertions(+), 20 deletions(-)
> >
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index 22e95b9ae82f..b1118050ff52 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -5688,11 +5688,11 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > struct path path;
> >
> > cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client);
> > - open->op_recall = 0;
> > + open->op_recall = false;
> > switch (open->op_claim_type) {
> > case NFS4_OPEN_CLAIM_PREVIOUS:
> > if (!cb_up)
> > - open->op_recall = 1;
> > + open->op_recall = true;
> > break;
> > case NFS4_OPEN_CLAIM_NULL:
> > parent = currentfh;
> > @@ -5746,7 +5746,7 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > if (open->op_claim_type == NFS4_OPEN_CLAIM_PREVIOUS &&
> > open->op_delegate_type != NFS4_OPEN_DELEGATE_NONE) {
> > dprintk("NFSD: WARNING: refusing delegation reclaim\n");
> > - open->op_recall = 1;
> > + open->op_recall = true;
> > }
> >
> > /* 4.1 client asking for a delegation? */
> > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > index ee8a7989f54f..f411fcc435f6 100644
> > --- a/fs/nfsd/nfs4xdr.c
> > +++ b/fs/nfsd/nfs4xdr.c
> > @@ -4074,6 +4074,49 @@ nfsd4_encode_link(struct nfsd4_compoundres *resp, __be32 nfserr,
> > return nfsd4_encode_change_info4(xdr, &link->li_cinfo);
> > }
> >
> > +/*
> > + * This implementation does not yet support returning an ACE in an
> > + * OPEN that offers a delegation.
> > + */
> > +static __be32
> > +nfsd4_encode_open_nfsace4(struct xdr_stream *xdr)
> > +{
> > + __be32 status;
> > +
> > + /* type */
> > + status = nfsd4_encode_acetype4(xdr, NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE);
> > + if (status != nfs_ok)
> > + return nfserr_resource;
> > + /* flag */
> > + status = nfsd4_encode_aceflag4(xdr, 0);
> > + if (status != nfs_ok)
> > + return nfserr_resource;
> > + /* access mask */
> > + status = nfsd4_encode_acemask4(xdr, 0);
> > + if (status != nfs_ok)
> > + return nfserr_resource;
> > + /* who - empty for now */
> > + if (xdr_stream_encode_u32(xdr, 0) != XDR_UNIT)
> > + return nfserr_resource;
> > + return nfs_ok;
> > +}
> > +
> > +static __be32
> > +nfsd4_encode_open_read_delegation4(struct xdr_stream *xdr, struct nfsd4_open *open)
> > +{
> > + __be32 status;
> > +
> > + /* stateid */
> > + status = nfsd4_encode_stateid4(xdr, &open->op_delegate_stateid);
> > + if (status != nfs_ok)
> > + return status;
> > + /* recall */
> > + status = nfsd4_encode_bool(xdr, open->op_recall);
> > + if (status != nfs_ok)
> > + return status;
> > + /* permissions */
> > + return nfsd4_encode_open_nfsace4(xdr);
> > +}
> >
> > static __be32
> > nfsd4_encode_open(struct nfsd4_compoundres *resp, __be32 nfserr,
> > @@ -4106,22 +4149,8 @@ nfsd4_encode_open(struct nfsd4_compoundres *resp, __be32 nfserr,
> > case NFS4_OPEN_DELEGATE_NONE:
> > break;
> > case NFS4_OPEN_DELEGATE_READ:
> > - nfserr = nfsd4_encode_stateid4(xdr, &open->op_delegate_stateid);
> > - if (nfserr)
> > - return nfserr;
> > - p = xdr_reserve_space(xdr, 20);
> > - if (!p)
> > - return nfserr_resource;
> > - *p++ = cpu_to_be32(open->op_recall);
> > -
> > - /*
> > - * TODO: ACE's in delegations
> > - */
> > - *p++ = cpu_to_be32(NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE);
> > - *p++ = cpu_to_be32(0);
> > - *p++ = cpu_to_be32(0);
> > - *p++ = cpu_to_be32(0); /* XXX: is NULL principal ok? */
> > - break;
> > + /* read */
> > + return nfsd4_encode_open_read_delegation4(xdr, open);
> > case NFS4_OPEN_DELEGATE_WRITE:
> > nfserr = nfsd4_encode_stateid4(xdr, &open->op_delegate_stateid);
> > if (nfserr)
> > diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> > index aba07d5378fc..c142a9b5ab98 100644
> > --- a/fs/nfsd/xdr4.h
> > +++ b/fs/nfsd/xdr4.h
> > @@ -389,7 +389,7 @@ struct nfsd4_open {
> > u32 op_deleg_want; /* request */
> > stateid_t op_stateid; /* response */
> > __be32 op_xdr_error; /* see nfsd4_open_omfg() */
> > - u32 op_recall; /* recall */
> > + bool op_recall; /* response */
>
> nit: this would probably pack better if you moved op_recall down beside
> op_truncate.
Fixed. All 15 pushed to nfsd-next with your Reviewed-by. Thanks!
> > struct nfsd4_change_info op_cinfo; /* response */
> > u32 op_rflags; /* response */
> > bool op_truncate; /* used during processing */
> >
> >
>
> --
> Jeff Layton <jlayton@xxxxxxxxxxxxxxx>
--
Chuck Lever
