From: "J. Bruce Fields" <bfields@fieldses.org>
To: Benny Halevy <bhalevy@tonian.com>
Cc: "J. Bruce Fields" <bfields@redhat.com>, linux-nfs@vger.kernel.org
Subject: Re: [PATCH 5/7] nfsd4: implement NFS4_SHARE_WANT_NO_DELEG, NFS4_OPEN_DELEGATE_NONE_EXT
Date: Thu, 20 Oct 2011 15:21:37 -0400 [thread overview]
Message-ID: <20111020192137.GB9987@fieldses.org> (raw)
In-Reply-To: <4EA07375.30303@tonian.com>
On Thu, Oct 20, 2011 at 03:16:05PM -0400, Benny Halevy wrote:
> On 2011-10-20 08:47, J. Bruce Fields wrote:
> > Also, I wonder if it's wise for a client to depend on this--it really is
> > an optional feature as far as I can tell. It would be safer to teach
> > the client just to handle delegations in the simplest way possible
> > (possibly just return them immediately?).
> >
>
> That's true.
>
> How about the following? (untested squashme over this patch)
>From a very quick glance--yep, that's sort of what I was thinking. It
doesn't look too bad. Nit: I might split some of this into helper
functions like maybe:
if (flag == NFS4_OPEN_DELEGATE_NONE)
nfsd4_set_nodeleg_why(open);
But, squash this with the previous patch, add a changelog, maybe give us
some idea how to test this (some pynfs tests exercising one or two of
these cases would be nice).... And we'd be done with that.
--b.
>
> >From ab9e79564fee05a0f3f369fc134bbace1543ac4d Mon Sep 17 00:00:00 2001
> From: Benny Halevy <bhalevy@tonian.com>
> Date: Thu, 20 Oct 2011 08:00:03 -0700
> Subject: [PATCH] SQUASHME: nfsd4: implement why_no_deleg
>
> Signed-off-by: Benny Halevy <bhalevy@tonian.com>
> ---
> fs/nfsd/nfs4state.c | 50 +++++++++++++++++++++++++++++++++++++++++++++-----
> fs/nfsd/nfs4xdr.c | 7 ++++++-
> fs/nfsd/xdr4.h | 1 +
> 3 files changed, 52 insertions(+), 6 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index d00c246..2cdd2b3 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -2914,7 +2914,7 @@ static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
> struct nfs4_delegation *dp;
> struct nfs4_openowner *oo = container_of(stp->st_stateowner, struct nfs4_openowner, oo_owner);
> int cb_up;
> - int status, flag = 0;
> + int status = 0, flag = 0;
>
> cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client);
> flag = NFS4_OPEN_DELEGATE_NONE;
> @@ -2955,11 +2955,32 @@ static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
> dprintk("NFSD: delegation stateid=" STATEID_FMT "\n",
> STATEID_VAL(&dp->dl_stid.sc_stateid));
> out:
> - if (open->op_claim_type == NFS4_OPEN_CLAIM_PREVIOUS
> - && flag == NFS4_OPEN_DELEGATE_NONE
> - && open->op_delegate_type != NFS4_OPEN_DELEGATE_NONE)
> - dprintk("NFSD: WARNING: refusing delegation reclaim\n");
> open->op_delegate_type = flag;
> + if (flag == NFS4_OPEN_DELEGATE_NONE) {
> + if (open->op_claim_type == NFS4_OPEN_CLAIM_PREVIOUS &&
> + open->op_delegate_type != NFS4_OPEN_DELEGATE_NONE)
> + dprintk("NFSD: WARNING: refusing delegation reclaim\n");
> +
> + if (open->op_deleg_want) {
> + open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
> + if (status == -EAGAIN)
> + open->op_why_no_deleg = WND4_CONTENTION;
> + else {
> + open->op_why_no_deleg = WND4_RESOURCE;
> + switch (open->op_deleg_want) {
> + case NFS4_SHARE_WANT_READ_DELEG:
> + case NFS4_SHARE_WANT_WRITE_DELEG:
> + case NFS4_SHARE_WANT_ANY_DELEG:
> + break;
> + case NFS4_SHARE_WANT_CANCEL:
> + open->op_why_no_deleg = WND4_CANCELLED;
> + break;
> + case NFS4_SHARE_WANT_NO_DELEG:
> + BUG(); /* not supposed to get here */
> + }
> + }
> + }
> + }
> return;
> out_free:
> nfs4_put_delegation(dp);
> @@ -3036,6 +3057,7 @@ static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
>
> if (open->op_deleg_want & NFS4_SHARE_WANT_NO_DELEG) {
> open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
> + open->op_why_no_deleg = WND4_NOT_WANTED;
> goto nodeleg;
> }
> }
> @@ -3051,6 +3073,24 @@ static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
> dprintk("%s: stateid=" STATEID_FMT "\n", __func__,
> STATEID_VAL(&stp->st_stid.sc_stateid));
> out:
> + /* 4.1 client trying to upgrade/downgrade delegation? */
> + if (open->op_delegate_type == NFS4_OPEN_DELEGATE_NONE && dp &&
> + open->op_deleg_want) {
> + if (open->op_deleg_want == NFS4_SHARE_WANT_READ_DELEG &&
> + dp->dl_type == NFS4_OPEN_DELEGATE_WRITE) {
> + open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
> + open->op_why_no_deleg = WND4_NOT_SUPP_DOWNGRADE;
> + } else if (open->op_deleg_want == NFS4_SHARE_WANT_WRITE_DELEG &&
> + dp->dl_type == NFS4_OPEN_DELEGATE_WRITE) {
> + open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
> + open->op_why_no_deleg = WND4_NOT_SUPP_DOWNGRADE;
> + }
> + /* Otherwise the client must be confused wanting a delegation
> + * it already has, therefore we don't return
> + * NFS4_OPEN_DELEGATE_NONE_EXT and reason.
> + */
> + }
> +
> if (fp)
> put_nfs4_file(fp);
> if (status == 0 && open->op_claim_type == NFS4_OPEN_CLAIM_PREVIOUS)
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 9b838e9..f2c1338 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -3035,7 +3035,12 @@ static __be32 nfsd4_encode_bind_conn_to_session(struct nfsd4_compoundres *resp,
> ADJUST_ARGS();
> break;
> case NFS4_OPEN_DELEGATE_NONE_EXT: /* 4.1 */
> - WRITE32(WND4_NOT_WANTED); /* only reason for now */
> + WRITE32(open->op_why_no_deleg);
> + switch (open->op_why_no_deleg) {
> + case WND4_CONTENTION:
> + case WND4_RESOURCE:
> + WRITE32(0); /* deleg signaling not supported yet */
> + }
> break;
> default:
> BUG();
> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> index 1e5ca95..d20d87e 100644
> --- a/fs/nfsd/xdr4.h
> +++ b/fs/nfsd/xdr4.h
> @@ -214,6 +214,7 @@ struct nfsd4_open {
> struct xdr_netobj op_fname; /* request - everything but CLAIM_PREV */
> u32 op_delegate_type; /* request - CLAIM_PREV only */
> stateid_t op_delegate_stateid; /* request - response */
> + u32 op_why_no_deleg; /* response - DELEG_NONE_EXT only */
> u32 op_create; /* request */
> u32 op_createmode; /* request */
> u32 op_bmval[3]; /* request */
> --
> 1.7.6
>
next prev parent reply other threads:[~2011-10-20 19:21 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-10-20 2:10 [PATCH 0/7] Bakeathon server fixes Benny Halevy
2011-10-20 2:12 ` [PATCH 1/7] nfsd4: implement new 4.1 open reclaim types Benny Halevy
2011-10-20 12:23 ` J. Bruce Fields
2011-10-20 2:12 ` [PATCH 2/7] nfsd41: use SEQ4_STATUS_BACKCHANNEL_FAULT when cb_sequence is invalid Benny Halevy
2011-10-20 2:13 ` [PATCH 3/7] nfsd4: seq->status_flags may be used unitialized Benny Halevy
2011-10-20 2:13 ` [PATCH 4/7] nfsd4: allow NFS4_SHARE_SIGNAL_DELEG_WHEN_RESRC_AVAIL | NFS4_SHARE_PUSH_DELEG_WHEN_UNCONTENDED Benny Halevy
2011-10-20 2:13 ` [PATCH 5/7] nfsd4: implement NFS4_SHARE_WANT_NO_DELEG, NFS4_OPEN_DELEGATE_NONE_EXT Benny Halevy
2011-10-20 11:52 ` J. Bruce Fields
2011-10-20 12:36 ` Benny Halevy
2011-10-20 12:47 ` J. Bruce Fields
2011-10-20 19:16 ` Benny Halevy
2011-10-20 19:21 ` J. Bruce Fields [this message]
2011-10-20 2:13 ` [PATCH 6/7] nfsd4: typo logical vs bitwise negate for want_mask Benny Halevy
2011-10-20 11:53 ` J. Bruce Fields
2011-10-20 2:13 ` [PATCH 7/7] nfsd4: share_access_to_flags should consider only nfs4.x share_access flags Benny Halevy
2011-10-20 11:56 ` J. Bruce Fields
2011-10-20 18:59 ` Benny Halevy
2011-10-20 11:58 ` [PATCH 0/7] Bakeathon server fixes J. Bruce Fields
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20111020192137.GB9987@fieldses.org \
--to=bfields@fieldses.org \
--cc=bfields@redhat.com \
--cc=bhalevy@tonian.com \
--cc=linux-nfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).