On Wed, Nov 27, 2013 at 06:29:22PM +0800, Kinglong Mee wrote:
> If finding backchannel failed, nfsd should not enter setup_callback_client.
>
> Signed-off-by: Kinglong Mee <kinglongmee@xxxxxxxxx>
> ---
> fs/nfsd/nfs4callback.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index 7f05cd1..755d6d6 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -664,7 +664,7 @@ static int setup_callback_client(struct nfs4_client *clp, struct nfs4_cb_conn *c
> args.authflavor = clp->cl_cred.cr_flavor;
> clp->cl_cb_ident = conn->cb_ident;
> } else {
> - if (!conn->cb_xprt)
> + if (!conn->cb_xprt || !ses)
It looks to me like ses should be set whenenver conn is. Do you have
reason to believe the contrary?
> return -EINVAL;
> clp->cl_cb_conn.cb_xprt = conn->cb_xprt;
> clp->cl_cb_session = ses;
> @@ -982,7 +982,7 @@ static void nfsd4_process_cb_update(struct nfsd4_callback *cb)
> }
> spin_unlock(&clp->cl_lock);
>
> - err = setup_callback_client(clp, &conn, ses);
> + err = c ? setup_callback_client(clp, &conn, ses) : -ENOENT;
But, whoops, yes, this looks like a good fix. (Have you hit this
failure in practice?) But I think we should probably adopt the sort of
control flow that's more idiomatic in the kernel:
c = __nfsd4_find_backchannel(clp)
if (!c)
goto out_unlock;
...
spin_unlock(&clp->cl_lock);
err = setup_callback_client(clp, &conn, ses);
if (err)
goto out_err;
etc.
--b.
> if (err) {
> nfsd4_mark_cb_down(clp, err);
> return;
> --
> 1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
