Re: [PATCH 03/15] Add /etc/nfs.conf support to rpc.nfsd

[NeilBrown <neilb@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 1826 bytes --]

On Wed, Dec 07 2016, NeilBrown wrote:

> [ Unknown signature status ]
> On Wed, Dec 07 2016, Steve Dickson wrote:
>
>>> +.B nfsd
>>> +Recognized values:
>>> +.BR threads ,
>>> +.BR grace-time ,
>>> +.BR lease-time ,
>>> +.BR udp ,
>>> +.BR tcp ,
>>> +.BR vers2 ,
>>> +.BR vers3 ,
>>> +.BR vers4 ,
>>> +.BR vers4.0 ,
>> Do we need both ver4 and ver4.0?
>
> "vers4" allows you to enable or disable v4 has a whole.
> The assumption is that if enabled, all minor versions that the kernel
> supports will be enabled by default.
> vers4.x allows individual minor versions to be disabled, so
>  vers4=yes
>  vers4.0=no
> would disable v4.0, just like "-V4 -N4.0"

What I meant here, of course, is
   just like "-V4 -N4.0" *should*

not "just like what it *does*."

To my surprise, when you actually try "nfsd -N4.0",
it opens /proc/fs/nfsd/versions and writes "+4.32".

#define NFSCTL_VERISSET(_cltbits, _v) ((_cltbits) & (1 << ((_v) - 1))) 

That "-1" makes it clear that version 4.0 isn't understood.

The kernel side seem to understand that '0' is an independent minor version:
static bool nfsd_supported_minorversions[NFSD_SUPPORTED_MINOR_VERSION + 1] = {
	[0] = 1,
	[1] = 1,
	[2] = 1,
};

but refuses to let it be disabled:
				if (num != 4)
					return -EINVAL;
				minor = simple_strtoul(minorp+1, NULL, 0);
				if (minor == 0)
					return -EINVAL;

so you can have 4.1 disabled while 4.0 and 4.2 are active, but you
cannot disable 4.0 as well.

Should that be fixed?  Do we care?

At least we should change
					int i = atoi(p+1);
					if (i > NFS4_MAXMINOR) {
						fprintf(stderr, "%s: unsupported minor version\n", optarg);
						exit(1);

in nfsd.c so the test is
  if (i < NFS4_MINMINOR || i > NFS4_MAXMINOR) {

I'll send a patch.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]