On 06/26/2012 02:23 PM, Chuck Lever wrote: > > On Jun 26, 2012, at 2:12 PM, bjschuma@xxxxxxxxxx wrote: > >> From: Bryan Schumaker <bjschuma@xxxxxxxxxx> >> >> I was treating a NULL return value as an error, but this function will >> instead return an ERR_PTR(). Now that I'm checking if the function >> returns an error, I should also pass the error up the call stack. > > >> Signed-off-by: Bryan Schumaker <bjschuma@xxxxxxxxxx> >> --- >> fs/nfs/nfs4namespace.c | 4 ++-- >> fs/nfs/nfs4proc.c | 15 ++++----------- >> 2 files changed, 6 insertions(+), 13 deletions(-) >> >> diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c >> index 017b4b0..a8aafc6 100644 >> --- a/fs/nfs/nfs4namespace.c >> +++ b/fs/nfs/nfs4namespace.c >> @@ -205,9 +205,9 @@ struct rpc_clnt *nfs4_create_sec_client(struct rpc_clnt *clnt, struct inode *ino >> return clone; >> >> auth = rpcauth_create(flavor, clone); >> - if (!auth) { >> + if (IS_ERR(auth)) { >> rpc_shutdown_client(clone); >> - clone = ERR_PTR(-EIO); >> + clone = ERR_CAST(auth); >> } > > This echoes nfs_init_server_rpcclient(), so it should be adequate. > >> >> return clone; >> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c >> index 5a7b372..f817587 100644 >> --- a/fs/nfs/nfs4proc.c >> +++ b/fs/nfs/nfs4proc.c >> @@ -2356,17 +2356,10 @@ out: >> static int nfs4_lookup_root_sec(struct nfs_server *server, struct nfs_fh *fhandle, >> struct nfs_fsinfo *info, rpc_authflavor_t flavor) >> { >> - struct rpc_auth *auth; >> - int ret; >> - >> - auth = rpcauth_create(flavor, server->client); >> - if (!auth) { >> - ret = -EIO; >> - goto out; >> - } >> - ret = nfs4_lookup_root(server, fhandle, info); >> -out: >> - return ret; >> + struct rpc_auth *auth = rpcauth_create(flavor, server->client); >> + if (IS_ERR(auth)) >> + return PTR_ERR(auth); >> + return nfs4_lookup_root(server, fhandle, info); >> } >> >> static int nfs4_find_root_sec(struct nfs_server *server, struct nfs_fh *fhandle, > > This logic calls rpcauth_create() in a loop, correct? The expectation is that rpcauth_create() should simply replace the nfs_client's rpc_auth with the next flavor each time it is called? > Yeah, that's what I'm expecting to happen. > With the current code in nfs4_find_root_sec(), rpcauth_create() with a GSS flavor fails the second time you call it because (I suspect) there's a pipe dentry left over from the previous call. So there's no way to walk an entire list returned from gss_mech_list_flavors(), it will fail on the second GSS flavor every time. rpcauth_create() has this bit of code in it: if (clnt->cl_auth) rpcauth_release(clnt->cl_auth); clnt->cl_auth = auth; I guess I figured rpcauth_release() was already doing the necessary cleanup. - Bryan > > We could: > > a) have gss_pipes_dentries_create_net() ignore -EEXIST and return zero instead, or > > b) make sure rpcauth_create() releases the old rpc_auth before calling op->create() > > c) ?? > -- 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