Re: [PATCH 2/2] NFSD: supports nfsv4 service without rpcbind

[Kinglong Mee <kinglongmee@gmail.com>]

On 12/31/2013 01:54 AM, Chuck Lever wrote:
> 
> On Dec 30, 2013, at 6:25 AM, Kinglong Mee <kinglongmee@gmail.com> wrote:
> 
>> 1. set vs_ignore_err for nfsd_version4
>> 2. don't start lockd when only supports nfsv4
>>
>> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
>> ---
>> fs/nfsd/netns.h    |  1 +
>> fs/nfsd/nfs4proc.c |  1 +
>> fs/nfsd/nfssvc.c   | 26 +++++++++++++++++++++-----
>> 3 files changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
>> index 849a7c3..d32b3aa 100644
>> --- a/fs/nfsd/netns.h
>> +++ b/fs/nfsd/netns.h
>> @@ -95,6 +95,7 @@ struct nfsd_net {
>> 	time_t nfsd4_grace;
>>
>> 	bool nfsd_net_up;
>> +	bool lockd_up;
>>
>> 	/*
>> 	 * Time of server startup
>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>> index 419572f..9320986 100644
>> --- a/fs/nfsd/nfs4proc.c
>> +++ b/fs/nfsd/nfs4proc.c
>> @@ -1881,6 +1881,7 @@ struct svc_version	nfsd_version4 = {
>> 		.vs_proc	= nfsd_procedures4,
>> 		.vs_dispatch	= nfsd_dispatch,
>> 		.vs_xdrsize	= NFS4_SVC_XDRSIZE,
>> +		.vs_ignore_err	= 1,
> 
> It's better, I think, to include this hunk in 1/1.  That way, that one patch can be merged into stable, or cherry-picked by a distribution.
> 
> Also, just a nit: ".vs_ignore_err" is a rather generic name.  Something more specific like ".vs_rpcb_optnl" would be nicer, or reverse the logic and call it ".vs_rpcb_needed".

Make sense.
Thanks for you suggestion.

> 
>> };
>>
>> /*
>> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
>> index 760c85a..55b5b57 100644
>> --- a/fs/nfsd/nfssvc.c
>> +++ b/fs/nfsd/nfssvc.c
>> @@ -241,6 +241,11 @@ static void nfsd_shutdown_generic(void)
>> 	nfsd_racache_shutdown();
>> }
>>
>> +static bool nfsd_needs_lockd(void)
>> +{
>> +	return (nfsd_versions[2] != NULL) || (nfsd_versions[3] != NULL);
>> +}
> 
> How does this logic know which version of nfsd is being started?

In the latest kernel (3.13), nfsd_versions records the version of nfsd which being started,
nfsd_version records the version of nfsd which can be supported by codes.

In function named nfsd_vers, when setting a version of nfsd, nfsd_versions[vers] will be signed
to nfsd_version[vers], in contrast, when clearing, nfsd_versions[vers] will be signed to NULL.

So, we can check nfsd_versions[vers] directly for according version.

thanks,
Kinglong Mee

> 
>> +
>> static int nfsd_startup_net(int nrservs, struct net *net)
>> {
>> 	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
>> @@ -255,9 +260,14 @@ static int nfsd_startup_net(int nrservs, struct net *net)
>> 	ret = nfsd_init_socks(net);
>> 	if (ret)
>> 		goto out_socks;
>> -	ret = lockd_up(net);
>> -	if (ret)
>> -		goto out_socks;
>> +
>> +	if (nfsd_needs_lockd() && !nn->lockd_up) {
>> +		ret = lockd_up(net);
>> +		if (ret)
>> +			goto out_socks;
>> +		nn->lockd_up = 1;
>> +	}
>> +
>> 	ret = nfs4_state_start_net(net);
>> 	if (ret)
>> 		goto out_lockd;
>> @@ -266,7 +276,10 @@ static int nfsd_startup_net(int nrservs, struct net *net)
>> 	return 0;
>>
>> out_lockd:
>> -	lockd_down(net);
>> +	if (nn->lockd_up) {
>> +		lockd_down(net);
>> +		nn->lockd_up = 0;
>> +	}
>> out_socks:
>> 	nfsd_shutdown_generic();
>> 	return ret;
>> @@ -277,7 +290,10 @@ static void nfsd_shutdown_net(struct net *net)
>> 	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
>>
>> 	nfs4_state_shutdown_net(net);
>> -	lockd_down(net);
>> +	if (nn->lockd_up) {
>> +		lockd_down(net);
>> +		nn->lockd_up = 0;
>> +	}
>> 	nn->nfsd_net_up = false;
>> 	nfsd_shutdown_generic();
>> }
>> -- 
>> 1.8.4.2
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
> 
> 
> 
>