Making debugging output a little cleaner in call_verify by displaying the
name of the RPC procedure instead of it's number.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/clnt.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index adbc85c..5aa32fa 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -1431,10 +1431,10 @@ call_verify(struct rpc_task *task)
error = -EPROTONOSUPPORT;
goto out_err;
case RPC_PROC_UNAVAIL:
- dprintk("RPC: %5u %s: proc %p unsupported by program %u, "
+ dprintk("RPC: %5u %s: proc %s unsupported by program %u, "
"version %u on server %s\n",
task->tk_pid, __func__,
- task->tk_msg.rpc_proc,
+ task->tk_msg.rpc_proc->p_name,
task->tk_client->cl_prog,
task->tk_client->cl_vers,
task->tk_client->cl_server);
This being an accepted practice -- as opposed to a clearly
defined requirement in a spec someplace ??
On May 21, 2008, at 7:38 AM, Peter Staubach wrote:
> Talpey, Thomas wrote:
>> At 08:14 AM 5/21/2008, Peter Staubach wrote:
>>
>>> I have seen servers which didn't implement the NULL_PROC procedure
>>> for protocols like NFS_ACL. Beats me why.
>>>
>>
>> Names. We want names. :-)
>
> :-)
>
> Names withheld to prevent embarrassment of the guilty?
>
> Actually, I don't remember anymore who they were, but I suspect
> that I would have complained to them at the time. It just isn't
> that hard to implement a procedure which doesn't do anything but
> respond to the client.
>
> ps
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs"
> in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On May 21, 2008, at 8:14 AM, Peter Staubach wrote:
> Chuck Lever wrote:
>>
>> On May 20, 2008, at 5:21 PM, Trond Myklebust wrote:
>>
>>> On Tue, 2008-05-20 at 16:29 -0400, Chuck Lever wrote:
>>>> Making debugging output a little cleaner in call_verify by
>>>> displaying the
>>>> name of the RPC procedure instead of it's number.
>>>>
>>>> Signed-off-by: Chuck Lever <[email protected]>
>>>> ---
>>>>
>>>> net/sunrpc/clnt.c | 4 ++--
>>>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>>
>>>> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
>>>> index adbc85c..5aa32fa 100644
>>>> --- a/net/sunrpc/clnt.c
>>>> +++ b/net/sunrpc/clnt.c
>>>> @@ -1431,10 +1431,10 @@ call_verify(struct rpc_task *task)
>>>> error = -EPROTONOSUPPORT;
>>>> goto out_err;
>>>> case RPC_PROC_UNAVAIL:
>>>> - dprintk("RPC: %5u %s: proc %p unsupported by program %u, "
>>>> + dprintk("RPC: %5u %s: proc %s unsupported by program %u, "
>>>> "version %u on server %s\n",
>>>> task->tk_pid, __func__,
>>>> - task->tk_msg.rpc_proc,
>>>> + task->tk_msg.rpc_proc->p_name,
>>>> task->tk_client->cl_prog,
>>>> task->tk_client->cl_vers,
>>>> task->tk_client->cl_server);
>>>
>>> This will cause a crash if you ever call it while somebody is
>>> calling
>>> rpc_call_null. There may be other cases too.
>>
>> How could you ever get an RPC_PROC_UNAVAIL with a NULL RPC?
>
> I have seen servers which didn't implement the NULL_PROC procedure
> for protocols like NFS_ACL. Beats me why.
>
> It would be good if the client didn't crash just because someone
> got lazy in the server implementation though.
Agreed, and I have already implemented a fix. I will post the new
patch set later today after a little more testing.
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
On Tue, 2008-05-20 at 16:29 -0400, Chuck Lever wrote:
> Making debugging output a little cleaner in call_verify by displaying the
> name of the RPC procedure instead of it's number.
>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
>
> net/sunrpc/clnt.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
>
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index adbc85c..5aa32fa 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -1431,10 +1431,10 @@ call_verify(struct rpc_task *task)
> error = -EPROTONOSUPPORT;
> goto out_err;
> case RPC_PROC_UNAVAIL:
> - dprintk("RPC: %5u %s: proc %p unsupported by program %u, "
> + dprintk("RPC: %5u %s: proc %s unsupported by program %u, "
> "version %u on server %s\n",
> task->tk_pid, __func__,
> - task->tk_msg.rpc_proc,
> + task->tk_msg.rpc_proc->p_name,
> task->tk_client->cl_prog,
> task->tk_client->cl_vers,
> task->tk_client->cl_server);
This will cause a crash if you ever call it while somebody is calling
rpc_call_null. There may be other cases too.
--
Trond Myklebust
Linux NFS client maintainer
NetApp
[email protected]
http://www.netapp.com
On May 20, 2008, at 5:21 PM, Trond Myklebust wrote:
> On Tue, 2008-05-20 at 16:29 -0400, Chuck Lever wrote:
>> Making debugging output a little cleaner in call_verify by
>> displaying the
>> name of the RPC procedure instead of it's number.
>>
>> Signed-off-by: Chuck Lever <[email protected]>
>> ---
>>
>> net/sunrpc/clnt.c | 4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>
>>
>> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
>> index adbc85c..5aa32fa 100644
>> --- a/net/sunrpc/clnt.c
>> +++ b/net/sunrpc/clnt.c
>> @@ -1431,10 +1431,10 @@ call_verify(struct rpc_task *task)
>> error = -EPROTONOSUPPORT;
>> goto out_err;
>> case RPC_PROC_UNAVAIL:
>> - dprintk("RPC: %5u %s: proc %p unsupported by program %u, "
>> + dprintk("RPC: %5u %s: proc %s unsupported by program %u, "
>> "version %u on server %s\n",
>> task->tk_pid, __func__,
>> - task->tk_msg.rpc_proc,
>> + task->tk_msg.rpc_proc->p_name,
>> task->tk_client->cl_prog,
>> task->tk_client->cl_vers,
>> task->tk_client->cl_server);
>
> This will cause a crash if you ever call it while somebody is calling
> rpc_call_null. There may be other cases too.
How could you ever get an RPC_PROC_UNAVAIL with a NULL RPC?
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
On Tue, 2008-05-20 at 17:39 -0400, Chuck Lever wrote:
> How could you ever get an RPC_PROC_UNAVAIL with a NULL RPC?
You shouldn't, given a well adjusted server, but that's not equivalent
to saying that it can't ever happen.
--
Trond Myklebust
Linux NFS client maintainer
NetApp
[email protected]
http://www.netapp.com
Chuck Lever wrote:
>
> On May 20, 2008, at 5:21 PM, Trond Myklebust wrote:
>
>> On Tue, 2008-05-20 at 16:29 -0400, Chuck Lever wrote:
>>> Making debugging output a little cleaner in call_verify by
>>> displaying the
>>> name of the RPC procedure instead of it's number.
>>>
>>> Signed-off-by: Chuck Lever <[email protected]>
>>> ---
>>>
>>> net/sunrpc/clnt.c | 4 ++--
>>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>>
>>> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
>>> index adbc85c..5aa32fa 100644
>>> --- a/net/sunrpc/clnt.c
>>> +++ b/net/sunrpc/clnt.c
>>> @@ -1431,10 +1431,10 @@ call_verify(struct rpc_task *task)
>>> error = -EPROTONOSUPPORT;
>>> goto out_err;
>>> case RPC_PROC_UNAVAIL:
>>> - dprintk("RPC: %5u %s: proc %p unsupported by program %u, "
>>> + dprintk("RPC: %5u %s: proc %s unsupported by program %u, "
>>> "version %u on server %s\n",
>>> task->tk_pid, __func__,
>>> - task->tk_msg.rpc_proc,
>>> + task->tk_msg.rpc_proc->p_name,
>>> task->tk_client->cl_prog,
>>> task->tk_client->cl_vers,
>>> task->tk_client->cl_server);
>>
>> This will cause a crash if you ever call it while somebody is calling
>> rpc_call_null. There may be other cases too.
>
> How could you ever get an RPC_PROC_UNAVAIL with a NULL RPC?
I have seen servers which didn't implement the NULL_PROC procedure
for protocols like NFS_ACL. Beats me why.
It would be good if the client didn't crash just because someone
got lazy in the server implementation though.
ps
At 08:14 AM 5/21/2008, Peter Staubach wrote:
>I have seen servers which didn't implement the NULL_PROC procedure
>for protocols like NFS_ACL. Beats me why.
Names. We want names. :-)
Tom.
Talpey, Thomas wrote:
> At 08:14 AM 5/21/2008, Peter Staubach wrote:
>
>> I have seen servers which didn't implement the NULL_PROC procedure
>> for protocols like NFS_ACL. Beats me why.
>>
>
> Names. We want names. :-)
:-)
Names withheld to prevent embarrassment of the guilty?
Actually, I don't remember anymore who they were, but I suspect
that I would have complained to them at the time. It just isn't
that hard to implement a procedure which doesn't do anything but
respond to the client.
ps