2013-12-16 18:21:49

by Joakim Tjernlund

[permalink] [raw]
Subject: nfs-utils-1.2.9 does not play well with linux 3.10.x

rpc.nfsd insists on adding "-4.2" when writing /proc/fs/nfsd/versions :
rpc.nfsd: Writing version string to kernel: +4.1 -4.2 -2 +3 +4
Which causes Linux to return an EIVAL error as 3.10 has no support for 4.2
and
does not accept any reference to 4.2

It seems reasonable to me that Linux should accpect -4.2 as a noop and
continue
processing the rest of the options but I am just guessing.
Anyhow, just to test I applied this commit to my 3.10.24 kernel:

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4bdc33ed5bd9fbaa243bda6fdccb22674aed6305
and now it accepts the "-4.2" but I have no idea if this messes up
something else.
Ideas?

Jocke


2013-12-16 20:03:02

by J. Bruce Fields

[permalink] [raw]
Subject: Re: nfs-utils-1.2.9 does not play well with linux 3.10.x

On Mon, Dec 16, 2013 at 01:54:19PM -0500, bfields wrote:
> On Mon, Dec 16, 2013 at 07:16:00PM +0100, Joakim Tjernlund wrote:
> > rpc.nfsd insists on adding "-4.2" when writing /proc/fs/nfsd/versions :
> > rpc.nfsd: Writing version string to kernel: +4.1 -4.2 -2 +3 +4
> > Which causes Linux to return an EIVAL error as 3.10 has no support for 4.2
> > and
> > does not accept any reference to 4.2
> >
> > It seems reasonable to me that Linux should accpect -4.2 as a noop and
> > continue
> > processing the rest of the options but I am just guessing.
> > Anyhow, just to test I applied this commit to my 3.10.24 kernel:
> >
> > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4bdc33ed5bd9fbaa243bda6fdccb22674aed6305
> > and now it accepts the "-4.2" but I have no idea if this messes up
> > something else.
>
> That should be perfectly safe.
>
> I agree that we should teach the kernel to treat "-4.x" at least as a
> no-op for unknown .x. But nfs-utils also has to keep working with older
> kernels which don't do that.
>
> The problem was introduced by 12a590f8d556c00a9502eeebaa763d906222d521
> "rpc.nfsd: Allow v4.2 server support with the -V option". That should
> be using an array of ints not a bit array, so it can make the
> distinction between "off", "on", and "don't care".

So, something like this (untested).--b.

diff --git a/support/include/nfs/nfs.h b/support/include/nfs/nfs.h
index 38db5b5..df4ad76 100644
--- a/support/include/nfs/nfs.h
+++ b/support/include/nfs/nfs.h
@@ -17,7 +17,6 @@

#define NFS4_MINMINOR 1
#define NFS4_MAXMINOR 2
-#define NFS4_VERDEFAULT 0x1 /* minor verion 1 */

struct nfs_fh_len {
int fh_size;
diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index 6db92f0..a9d77ab 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -99,7 +99,7 @@ main(int argc, char **argv)
char *p, *progname, *port;
char *haddr = NULL;
int socket_up = 0;
- int minorvers = NFS4_VERDEFAULT; /* nfsv4 minor version */
+ int minorvers[NFS4_MAXMINOR + 1] = {0};
unsigned int versbits = NFSCTL_VERDEFAULT;
unsigned int protobits = NFSCTL_ALLBITS;
unsigned int proto4 = 0;
@@ -164,7 +164,7 @@ main(int argc, char **argv)
fprintf(stderr, "%s: unsupported minor version\n", optarg);
exit(1);
}
- NFSCTL_VERUNSET(minorvers, i);
+ minorvers[i] = -1;
break;
}
case 3:
@@ -185,7 +185,7 @@ main(int argc, char **argv)
fprintf(stderr, "%s: unsupported minor version\n", optarg);
exit(1);
}
- NFSCTL_VERSET(minorvers, i);
+ minorvers[i] = 1;
break;
}
case 3:
diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
index 8b85846..1b50aba 100644
--- a/utils/nfsd/nfssvc.c
+++ b/utils/nfsd/nfssvc.c
@@ -269,7 +269,7 @@ nfssvc_set_sockets(const int family, const unsigned int protobits,
}

void
-nfssvc_setvers(unsigned int ctlbits, int minorvers)
+nfssvc_setvers(unsigned int ctlbits, int minorvers[])
{
int fd, n, off;
char *ptr;
@@ -281,9 +281,9 @@ nfssvc_setvers(unsigned int ctlbits, int minorvers)
return;

for (n = NFS4_MINMINOR; n <= NFS4_MAXMINOR; n++) {
- if (NFSCTL_VERISSET(minorvers, n))
+ if (minorvers[n] == 1)
off += snprintf(ptr+off, sizeof(buf) - off, "+4.%d ", n);
- else
+ else if (minorvers[n] == -1)
off += snprintf(ptr+off, sizeof(buf) - off, "-4.%d ", n);
}
for (n = NFSD_MINVERS; n <= NFSD_MAXVERS; n++) {
diff --git a/utils/nfsd/nfssvc.h b/utils/nfsd/nfssvc.h
index 08de0fe..2bbd3d3 100644
--- a/utils/nfsd/nfssvc.h
+++ b/utils/nfsd/nfssvc.h
@@ -24,5 +24,5 @@ void nfssvc_mount_nfsdfs(char *progname);
int nfssvc_inuse(void);
int nfssvc_set_sockets(const int family, const unsigned int protobits,
const char *host, const char *port);
-void nfssvc_setvers(unsigned int ctlbits, int minorvers4);
+void nfssvc_setvers(unsigned int ctlbits, int minorvers4[]);
int nfssvc_threads(unsigned short port, int nrservs);

2013-12-17 03:38:51

by J. Bruce Fields

[permalink] [raw]
Subject: Re: nfs-utils-1.2.9 does not play well with linux 3.10.x

On Mon, Dec 16, 2013 at 11:24:03PM +0100, Joakim Tjernlund wrote:
> "J. Bruce Fields" <[email protected]> wrote on 2013/12/16 21:23:45:
>
> > From: "J. Bruce Fields" <[email protected]>
> > To: Joakim Tjernlund <[email protected]>,
> > Cc: [email protected], [email protected]
> > Date: 2013/12/16 21:23
> > Subject: Re: nfs-utils-1.2.9 does not play well with linux 3.10.x
> >
> > On Mon, Dec 16, 2013 at 09:21:15PM +0100, Joakim Tjernlund wrote:
> > > "J. Bruce Fields" <[email protected]> wrote on 2013/12/16 21:03:01:
> > >
> > > > From: "J. Bruce Fields" <[email protected]>
> > > > To: Joakim Tjernlund <[email protected]>,
> > > > Cc: [email protected], [email protected]
> > > > Date: 2013/12/16 21:03
> > > > Subject: Re: nfs-utils-1.2.9 does not play well with linux 3.10.x
> > > >
> > > > On Mon, Dec 16, 2013 at 01:54:19PM -0500, bfields wrote:
> > > > > On Mon, Dec 16, 2013 at 07:16:00PM +0100, Joakim Tjernlund wrote:
> > > > > > rpc.nfsd insists on adding "-4.2" when writing
> > > /proc/fs/nfsd/versions :
> > > > > > rpc.nfsd: Writing version string to kernel: +4.1 -4.2 -2 +3 +4
> > > > > > Which causes Linux to return an EIVAL error as 3.10 has no
> support
> > > for 4.2
> > > > > > and
> > > > > > does not accept any reference to 4.2
> > > > > >
> > > > > > It seems reasonable to me that Linux should accpect -4.2 as a
> noop
> > > and
> > > > > > continue
> > > > > > processing the rest of the options but I am just guessing.
> > > > > > Anyhow, just to test I applied this commit to my 3.10.24 kernel:
> > > > > >
> > > > > >
> > >
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4bdc33ed5bd9fbaa243bda6fdccb22674aed6305
>
> > >
> > > > > > and now it accepts the "-4.2" but I have no idea if this messes
> up
> > > > > > something else.
> > > > >
> > > > > That should be perfectly safe.
> > >
> > > Thanks, I will keep this then until nfs-utils is working.
> > >
> > > > >
> > > > > I agree that we should teach the kernel to treat "-4.x" at least
> as a
> > > > > no-op for unknown .x. But nfs-utils also has to keep working with
>
> > > older
> > > > > kernels which don't do that.
> > > > >
> > > > > The problem was introduced by
> 12a590f8d556c00a9502eeebaa763d906222d521
> > > > > "rpc.nfsd: Allow v4.2 server support with the -V option". That
> should
> > > > > be using an array of ints not a bit array, so it can make the
> > > > > distinction between "off", "on", and "don't care".
> > > >
> > > > So, something like this (untested).--b.
> > >
> > > I tested this on my system(which has the above kernel patch) and I
> noticed
> > > a difference:
> >
> > Thanks!
> >
> > > rpc.nfsd: Writing version string to kernel: -2 +3 +4
> > > which is different than previous
> > > rpc.nfsd: Writing version string to kernel: +4.1 -4.2 -2 +3 +4
> > >
> > > The "+4.1" is missing.
> >
> > Yes, that's intentional. Is it causing you any problem?
>
> I don't know yet, but I don't think it would be a problem for me.
> However, are you not changing the defaults here? In that
> case someone else relying on 4.1 might have a problem I guess.

That's just restoring the behavior we had before
12a590f8d556c00a9502eeebaa763d906222d521, and will still result in 4.1
being turned on if the kernel is recent enough.

--b.

2013-12-16 22:24:06

by Joakim Tjernlund

[permalink] [raw]
Subject: Re: nfs-utils-1.2.9 does not play well with linux 3.10.x

"J. Bruce Fields" <[email protected]> wrote on 2013/12/16 21:23:45:

> From: "J. Bruce Fields" <[email protected]>
> To: Joakim Tjernlund <[email protected]>,
> Cc: [email protected], [email protected]
> Date: 2013/12/16 21:23
> Subject: Re: nfs-utils-1.2.9 does not play well with linux 3.10.x
>
> On Mon, Dec 16, 2013 at 09:21:15PM +0100, Joakim Tjernlund wrote:
> > "J. Bruce Fields" <[email protected]> wrote on 2013/12/16 21:03:01:
> >
> > > From: "J. Bruce Fields" <[email protected]>
> > > To: Joakim Tjernlund <[email protected]>,
> > > Cc: [email protected], [email protected]
> > > Date: 2013/12/16 21:03
> > > Subject: Re: nfs-utils-1.2.9 does not play well with linux 3.10.x
> > >
> > > On Mon, Dec 16, 2013 at 01:54:19PM -0500, bfields wrote:
> > > > On Mon, Dec 16, 2013 at 07:16:00PM +0100, Joakim Tjernlund wrote:
> > > > > rpc.nfsd insists on adding "-4.2" when writing
> > /proc/fs/nfsd/versions :
> > > > > rpc.nfsd: Writing version string to kernel: +4.1 -4.2 -2 +3 +4
> > > > > Which causes Linux to return an EIVAL error as 3.10 has no
support
> > for 4.2
> > > > > and
> > > > > does not accept any reference to 4.2
> > > > >
> > > > > It seems reasonable to me that Linux should accpect -4.2 as a
noop
> > and
> > > > > continue
> > > > > processing the rest of the options but I am just guessing.
> > > > > Anyhow, just to test I applied this commit to my 3.10.24 kernel:
> > > > >
> > > > >
> >
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4bdc33ed5bd9fbaa243bda6fdccb22674aed6305

> >
> > > > > and now it accepts the "-4.2" but I have no idea if this messes
up
> > > > > something else.
> > > >
> > > > That should be perfectly safe.
> >
> > Thanks, I will keep this then until nfs-utils is working.
> >
> > > >
> > > > I agree that we should teach the kernel to treat "-4.x" at least
as a
> > > > no-op for unknown .x. But nfs-utils also has to keep working with

> > older
> > > > kernels which don't do that.
> > > >
> > > > The problem was introduced by
12a590f8d556c00a9502eeebaa763d906222d521
> > > > "rpc.nfsd: Allow v4.2 server support with the -V option". That
should
> > > > be using an array of ints not a bit array, so it can make the
> > > > distinction between "off", "on", and "don't care".
> > >
> > > So, something like this (untested).--b.
> >
> > I tested this on my system(which has the above kernel patch) and I
noticed
> > a difference:
>
> Thanks!
>
> > rpc.nfsd: Writing version string to kernel: -2 +3 +4
> > which is different than previous
> > rpc.nfsd: Writing version string to kernel: +4.1 -4.2 -2 +3 +4
> >
> > The "+4.1" is missing.
>
> Yes, that's intentional. Is it causing you any problem?

I don't know yet, but I don't think it would be a problem for me.
However, are you not changing the defaults here? In that
case someone else relying on 4.1 might have a problem I guess.

Jocke


2013-12-17 06:30:40

by Joakim Tjernlund

[permalink] [raw]
Subject: Re: nfs-utils-1.2.9 does not play well with linux 3.10.x

"J. Bruce Fields" <[email protected]> wrote on 2013/12/17 04:38:49:
>
> On Mon, Dec 16, 2013 at 11:24:03PM +0100, Joakim Tjernlund wrote:
> > "J. Bruce Fields" <[email protected]> wrote on 2013/12/16 21:23:45:
> >
> > > From: "J. Bruce Fields" <[email protected]>
> > > To: Joakim Tjernlund <[email protected]>,
> > > Cc: [email protected], [email protected]
> > > Date: 2013/12/16 21:23
> > > Subject: Re: nfs-utils-1.2.9 does not play well with linux 3.10.x
> > >
> > > On Mon, Dec 16, 2013 at 09:21:15PM +0100, Joakim Tjernlund wrote:
> > > > "J. Bruce Fields" <[email protected]> wrote on 2013/12/16
21:03:01:
> > > >
> > > > > From: "J. Bruce Fields" <[email protected]>
> > > > > To: Joakim Tjernlund <[email protected]>,
> > > > > Cc: [email protected], [email protected]
> > > > > Date: 2013/12/16 21:03
> > > > > Subject: Re: nfs-utils-1.2.9 does not play well with linux
3.10.x
> > > > >
> > > > > On Mon, Dec 16, 2013 at 01:54:19PM -0500, bfields wrote:
> > > > > > On Mon, Dec 16, 2013 at 07:16:00PM +0100, Joakim Tjernlund
wrote:
> > > > > > > rpc.nfsd insists on adding "-4.2" when writing
> > > > /proc/fs/nfsd/versions :
> > > > > > > rpc.nfsd: Writing version string to kernel: +4.1 -4.2 -2
+3 +4
> > > > > > > Which causes Linux to return an EIVAL error as 3.10 has no
> > support
> > > > for 4.2
> > > > > > > and
> > > > > > > does not accept any reference to 4.2
> > > > > > >
> > > > > > > It seems reasonable to me that Linux should accpect -4.2 as
a
> > noop
> > > > and
> > > > > > > continue
> > > > > > > processing the rest of the options but I am just guessing.
> > > > > > > Anyhow, just to test I applied this commit to my 3.10.24
kernel:
> > > > > > >
> > > > > > >
> > > >
> >
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4bdc33ed5bd9fbaa243bda6fdccb22674aed6305

> >
> > > >
> > > > > > > and now it accepts the "-4.2" but I have no idea if this
messes
> > up
> > > > > > > something else.
> > > > > >
> > > > > > That should be perfectly safe.
> > > >
> > > > Thanks, I will keep this then until nfs-utils is working.
> > > >
> > > > > >
> > > > > > I agree that we should teach the kernel to treat "-4.x" at
least
> > as a
> > > > > > no-op for unknown .x. But nfs-utils also has to keep working
with
> >
> > > > older
> > > > > > kernels which don't do that.
> > > > > >
> > > > > > The problem was introduced by
> > 12a590f8d556c00a9502eeebaa763d906222d521
> > > > > > "rpc.nfsd: Allow v4.2 server support with the -V option". That

> > should
> > > > > > be using an array of ints not a bit array, so it can make the
> > > > > > distinction between "off", "on", and "don't care".
> > > > >
> > > > > So, something like this (untested).--b.
> > > >
> > > > I tested this on my system(which has the above kernel patch) and I

> > noticed
> > > > a difference:
> > >
> > > Thanks!
> > >
> > > > rpc.nfsd: Writing version string to kernel: -2 +3 +4
> > > > which is different than previous
> > > > rpc.nfsd: Writing version string to kernel: +4.1 -4.2 -2 +3 +4
> > > >
> > > > The "+4.1" is missing.
> > >
> > > Yes, that's intentional. Is it causing you any problem?
> >
> > I don't know yet, but I don't think it would be a problem for me.
> > However, are you not changing the defaults here? In that
> > case someone else relying on 4.1 might have a problem I guess.
>
> That's just restoring the behavior we had before
> 12a590f8d556c00a9502eeebaa763d906222d521, and will still result in 4.1
> being turned on if the kernel is recent enough.

What is an recent enough kernel? Does not 3.10.24 patched with the patch I
mentioned
earlier? I thought it would make 3.10 look like a 3.11 which have 4.2
supported.
If I am mistaken we good I guess.

Jocke


2013-12-16 20:23:47

by J. Bruce Fields

[permalink] [raw]
Subject: Re: nfs-utils-1.2.9 does not play well with linux 3.10.x

On Mon, Dec 16, 2013 at 09:21:15PM +0100, Joakim Tjernlund wrote:
> "J. Bruce Fields" <[email protected]> wrote on 2013/12/16 21:03:01:
>
> > From: "J. Bruce Fields" <[email protected]>
> > To: Joakim Tjernlund <[email protected]>,
> > Cc: [email protected], [email protected]
> > Date: 2013/12/16 21:03
> > Subject: Re: nfs-utils-1.2.9 does not play well with linux 3.10.x
> >
> > On Mon, Dec 16, 2013 at 01:54:19PM -0500, bfields wrote:
> > > On Mon, Dec 16, 2013 at 07:16:00PM +0100, Joakim Tjernlund wrote:
> > > > rpc.nfsd insists on adding "-4.2" when writing
> /proc/fs/nfsd/versions :
> > > > rpc.nfsd: Writing version string to kernel: +4.1 -4.2 -2 +3 +4
> > > > Which causes Linux to return an EIVAL error as 3.10 has no support
> for 4.2
> > > > and
> > > > does not accept any reference to 4.2
> > > >
> > > > It seems reasonable to me that Linux should accpect -4.2 as a noop
> and
> > > > continue
> > > > processing the rest of the options but I am just guessing.
> > > > Anyhow, just to test I applied this commit to my 3.10.24 kernel:
> > > >
> > > >
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4bdc33ed5bd9fbaa243bda6fdccb22674aed6305
>
> > > > and now it accepts the "-4.2" but I have no idea if this messes up
> > > > something else.
> > >
> > > That should be perfectly safe.
>
> Thanks, I will keep this then until nfs-utils is working.
>
> > >
> > > I agree that we should teach the kernel to treat "-4.x" at least as a
> > > no-op for unknown .x. But nfs-utils also has to keep working with
> older
> > > kernels which don't do that.
> > >
> > > The problem was introduced by 12a590f8d556c00a9502eeebaa763d906222d521
> > > "rpc.nfsd: Allow v4.2 server support with the -V option". That should
> > > be using an array of ints not a bit array, so it can make the
> > > distinction between "off", "on", and "don't care".
> >
> > So, something like this (untested).--b.
>
> I tested this on my system(which has the above kernel patch) and I noticed
> a difference:

Thanks!

> rpc.nfsd: Writing version string to kernel: -2 +3 +4
> which is different than previous
> rpc.nfsd: Writing version string to kernel: +4.1 -4.2 -2 +3 +4
>
> The "+4.1" is missing.

Yes, that's intentional. Is it causing you any problem?

--b.

2013-12-17 15:24:56

by J. Bruce Fields

[permalink] [raw]
Subject: Re: nfs-utils-1.2.9 does not play well with linux 3.10.x

On Tue, Dec 17, 2013 at 07:30:38AM +0100, Joakim Tjernlund wrote:
> "J. Bruce Fields" <[email protected]> wrote on 2013/12/17 04:38:49:
> >
> > On Mon, Dec 16, 2013 at 11:24:03PM +0100, Joakim Tjernlund wrote:
> > > "J. Bruce Fields" <[email protected]> wrote on 2013/12/16 21:23:45:
> > >
> > > > From: "J. Bruce Fields" <[email protected]>
> > > > To: Joakim Tjernlund <[email protected]>,
> > > > Cc: [email protected], [email protected]
> > > > Date: 2013/12/16 21:23
> > > > Subject: Re: nfs-utils-1.2.9 does not play well with linux 3.10.x
> > > >
> > > > On Mon, Dec 16, 2013 at 09:21:15PM +0100, Joakim Tjernlund wrote:
> > > > > I tested this on my system(which has the above kernel patch) and I
>
> > > noticed
> > > > > a difference:
> > > >
> > > > Thanks!
> > > >
> > > > > rpc.nfsd: Writing version string to kernel: -2 +3 +4
> > > > > which is different than previous
> > > > > rpc.nfsd: Writing version string to kernel: +4.1 -4.2 -2 +3 +4
> > > > >
> > > > > The "+4.1" is missing.
> > > >
> > > > Yes, that's intentional. Is it causing you any problem?
> > >
> > > I don't know yet, but I don't think it would be a problem for me.
> > > However, are you not changing the defaults here? In that
> > > case someone else relying on 4.1 might have a problem I guess.
> >
> > That's just restoring the behavior we had before
> > 12a590f8d556c00a9502eeebaa763d906222d521, and will still result in 4.1
> > being turned on if the kernel is recent enough.
>
> What is an recent enough kernel? Does not 3.10.24 patched with the patch I
> mentioned
> earlier? I thought it would make 3.10 look like a 3.11 which have 4.2
> supported.
> If I am mistaken we good I guess.

Sorry, I don't understand exactly what your concern is.

--b.

2013-12-16 20:21:17

by Joakim Tjernlund

[permalink] [raw]
Subject: Re: nfs-utils-1.2.9 does not play well with linux 3.10.x

"J. Bruce Fields" <[email protected]> wrote on 2013/12/16 21:03:01:

> From: "J. Bruce Fields" <[email protected]>
> To: Joakim Tjernlund <[email protected]>,
> Cc: [email protected], [email protected]
> Date: 2013/12/16 21:03
> Subject: Re: nfs-utils-1.2.9 does not play well with linux 3.10.x
>
> On Mon, Dec 16, 2013 at 01:54:19PM -0500, bfields wrote:
> > On Mon, Dec 16, 2013 at 07:16:00PM +0100, Joakim Tjernlund wrote:
> > > rpc.nfsd insists on adding "-4.2" when writing
/proc/fs/nfsd/versions :
> > > rpc.nfsd: Writing version string to kernel: +4.1 -4.2 -2 +3 +4
> > > Which causes Linux to return an EIVAL error as 3.10 has no support
for 4.2
> > > and
> > > does not accept any reference to 4.2
> > >
> > > It seems reasonable to me that Linux should accpect -4.2 as a noop
and
> > > continue
> > > processing the rest of the options but I am just guessing.
> > > Anyhow, just to test I applied this commit to my 3.10.24 kernel:
> > >
> > >
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4bdc33ed5bd9fbaa243bda6fdccb22674aed6305

> > > and now it accepts the "-4.2" but I have no idea if this messes up
> > > something else.
> >
> > That should be perfectly safe.

Thanks, I will keep this then until nfs-utils is working.

> >
> > I agree that we should teach the kernel to treat "-4.x" at least as a
> > no-op for unknown .x. But nfs-utils also has to keep working with
older
> > kernels which don't do that.
> >
> > The problem was introduced by 12a590f8d556c00a9502eeebaa763d906222d521
> > "rpc.nfsd: Allow v4.2 server support with the -V option". That should
> > be using an array of ints not a bit array, so it can make the
> > distinction between "off", "on", and "don't care".
>
> So, something like this (untested).--b.

I tested this on my system(which has the above kernel patch) and I noticed
a difference:
rpc.nfsd: Writing version string to kernel: -2 +3 +4
which is different than previous
rpc.nfsd: Writing version string to kernel: +4.1 -4.2 -2 +3 +4

The "+4.1" is missing.

Jocke

2013-12-17 03:43:33

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH] nfsd: fix minorversion-choosing interface

From: "J. Bruce Fields" <[email protected]>

By unconditionally adding +/-4.2 to the version string written to the
kernel we make nfs-utils incompatible with pre-4.2-supporting kernels.

Ditto for 4.1. This problem was introduced by
12a590f8d556c00a9502eeebaa763d906222d521 "rpc.nfsd: Allow v4.2 server
support with the -V option", which also change nfsd to unconditionally
pass +/-4.2.

Instead, just don't mention 4.1 or 4.2 unless the commandline has
specifically requested that one or the other be turned on or off.

Tested-by: Joakim Tjernlund <[email protected]>
Reported-by: Joakim Tjernlund <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>
---
support/include/nfs/nfs.h | 1 -
utils/nfsd/nfsd.c | 6 +++---
utils/nfsd/nfssvc.c | 6 +++---
utils/nfsd/nfssvc.h | 2 +-
4 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/support/include/nfs/nfs.h b/support/include/nfs/nfs.h
index 38db5b5..df4ad76 100644
--- a/support/include/nfs/nfs.h
+++ b/support/include/nfs/nfs.h
@@ -17,7 +17,6 @@

#define NFS4_MINMINOR 1
#define NFS4_MAXMINOR 2
-#define NFS4_VERDEFAULT 0x1 /* minor verion 1 */

struct nfs_fh_len {
int fh_size;
diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index 6db92f0..a9d77ab 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -99,7 +99,7 @@ main(int argc, char **argv)
char *p, *progname, *port;
char *haddr = NULL;
int socket_up = 0;
- int minorvers = NFS4_VERDEFAULT; /* nfsv4 minor version */
+ int minorvers[NFS4_MAXMINOR + 1] = {0};
unsigned int versbits = NFSCTL_VERDEFAULT;
unsigned int protobits = NFSCTL_ALLBITS;
unsigned int proto4 = 0;
@@ -164,7 +164,7 @@ main(int argc, char **argv)
fprintf(stderr, "%s: unsupported minor version\n", optarg);
exit(1);
}
- NFSCTL_VERUNSET(minorvers, i);
+ minorvers[i] = -1;
break;
}
case 3:
@@ -185,7 +185,7 @@ main(int argc, char **argv)
fprintf(stderr, "%s: unsupported minor version\n", optarg);
exit(1);
}
- NFSCTL_VERSET(minorvers, i);
+ minorvers[i] = 1;
break;
}
case 3:
diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
index 8b85846..1b50aba 100644
--- a/utils/nfsd/nfssvc.c
+++ b/utils/nfsd/nfssvc.c
@@ -269,7 +269,7 @@ nfssvc_set_sockets(const int family, const unsigned int protobits,
}

void
-nfssvc_setvers(unsigned int ctlbits, int minorvers)
+nfssvc_setvers(unsigned int ctlbits, int minorvers[])
{
int fd, n, off;
char *ptr;
@@ -281,9 +281,9 @@ nfssvc_setvers(unsigned int ctlbits, int minorvers)
return;

for (n = NFS4_MINMINOR; n <= NFS4_MAXMINOR; n++) {
- if (NFSCTL_VERISSET(minorvers, n))
+ if (minorvers[n] == 1)
off += snprintf(ptr+off, sizeof(buf) - off, "+4.%d ", n);
- else
+ else if (minorvers[n] == -1)
off += snprintf(ptr+off, sizeof(buf) - off, "-4.%d ", n);
}
for (n = NFSD_MINVERS; n <= NFSD_MAXVERS; n++) {
diff --git a/utils/nfsd/nfssvc.h b/utils/nfsd/nfssvc.h
index 08de0fe..2bbd3d3 100644
--- a/utils/nfsd/nfssvc.h
+++ b/utils/nfsd/nfssvc.h
@@ -24,5 +24,5 @@ void nfssvc_mount_nfsdfs(char *progname);
int nfssvc_inuse(void);
int nfssvc_set_sockets(const int family, const unsigned int protobits,
const char *host, const char *port);
-void nfssvc_setvers(unsigned int ctlbits, int minorvers4);
+void nfssvc_setvers(unsigned int ctlbits, int minorvers4[]);
int nfssvc_threads(unsigned short port, int nrservs);
--
1.8.3.1


2013-12-16 18:54:20

by J. Bruce Fields

[permalink] [raw]
Subject: Re: nfs-utils-1.2.9 does not play well with linux 3.10.x

On Mon, Dec 16, 2013 at 07:16:00PM +0100, Joakim Tjernlund wrote:
> rpc.nfsd insists on adding "-4.2" when writing /proc/fs/nfsd/versions :
> rpc.nfsd: Writing version string to kernel: +4.1 -4.2 -2 +3 +4
> Which causes Linux to return an EIVAL error as 3.10 has no support for 4.2
> and
> does not accept any reference to 4.2
>
> It seems reasonable to me that Linux should accpect -4.2 as a noop and
> continue
> processing the rest of the options but I am just guessing.
> Anyhow, just to test I applied this commit to my 3.10.24 kernel:
>
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4bdc33ed5bd9fbaa243bda6fdccb22674aed6305
> and now it accepts the "-4.2" but I have no idea if this messes up
> something else.

That should be perfectly safe.

I agree that we should teach the kernel to treat "-4.x" at least as a
no-op for unknown .x. But nfs-utils also has to keep working with older
kernels which don't do that.

The problem was introduced by 12a590f8d556c00a9502eeebaa763d906222d521
"rpc.nfsd: Allow v4.2 server support with the -V option". That should
be using an array of ints not a bit array, so it can make the
distinction between "off", "on", and "don't care".

--b.

2014-01-09 16:05:02

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] nfsd: fix minorversion-choosing interface



On 07/01/14 17:23, Joakim Tjernlund wrote:
> Steve Dickson <[email protected]> wrote on 2014/01/07 22:02:01:
>>
>>
>> On 16/12/13 22:43, J. Bruce Fields wrote:
>>> From: "J. Bruce Fields" <[email protected]>
>>>
>>> By unconditionally adding +/-4.2 to the version string written to the
>>> kernel we make nfs-utils incompatible with pre-4.2-supporting kernels.
>>>
>>> Ditto for 4.1. This problem was introduced by
>>> 12a590f8d556c00a9502eeebaa763d906222d521 "rpc.nfsd: Allow v4.2 server
>>> support with the -V option", which also change nfsd to unconditionally
>>> pass +/-4.2.
>>>
>>> Instead, just don't mention 4.1 or 4.2 unless the commandline has
>>> specifically requested that one or the other be turned on or off.
>>>
>>> Tested-by: Joakim Tjernlund <[email protected]>
>>> Reported-by: Joakim Tjernlund <[email protected]>
>>> Signed-off-by: J. Bruce Fields <[email protected]>
>> Committed...
>>
>> steved
>
> Thanks, do plan on making a release soon?
Sorry for the delayed response...

Its in my git tree and in Fedora 20 nfs-utils....

Is this what you were looking for?

steved.

>
> Jocke
>

2014-01-07 22:23:20

by Joakim Tjernlund

[permalink] [raw]
Subject: Re: [PATCH] nfsd: fix minorversion-choosing interface

Steve Dickson <[email protected]> wrote on 2014/01/07 22:02:01:
>
>
> On 16/12/13 22:43, J. Bruce Fields wrote:
> > From: "J. Bruce Fields" <[email protected]>
> >
> > By unconditionally adding +/-4.2 to the version string written to the
> > kernel we make nfs-utils incompatible with pre-4.2-supporting kernels.
> >
> > Ditto for 4.1. This problem was introduced by
> > 12a590f8d556c00a9502eeebaa763d906222d521 "rpc.nfsd: Allow v4.2 server
> > support with the -V option", which also change nfsd to unconditionally
> > pass +/-4.2.
> >
> > Instead, just don't mention 4.1 or 4.2 unless the commandline has
> > specifically requested that one or the other be turned on or off.
> >
> > Tested-by: Joakim Tjernlund <[email protected]>
> > Reported-by: Joakim Tjernlund <[email protected]>
> > Signed-off-by: J. Bruce Fields <[email protected]>
> Committed...
>
> steved

Thanks, do plan on making a release soon?

Jocke

2014-01-07 21:21:46

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] nfsd: fix minorversion-choosing interface



On 16/12/13 22:43, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <[email protected]>
>
> By unconditionally adding +/-4.2 to the version string written to the
> kernel we make nfs-utils incompatible with pre-4.2-supporting kernels.
>
> Ditto for 4.1. This problem was introduced by
> 12a590f8d556c00a9502eeebaa763d906222d521 "rpc.nfsd: Allow v4.2 server
> support with the -V option", which also change nfsd to unconditionally
> pass +/-4.2.
>
> Instead, just don't mention 4.1 or 4.2 unless the commandline has
> specifically requested that one or the other be turned on or off.
>
> Tested-by: Joakim Tjernlund <[email protected]>
> Reported-by: Joakim Tjernlund <[email protected]>
> Signed-off-by: J. Bruce Fields <[email protected]>
Committed...

steved

> ---
> support/include/nfs/nfs.h | 1 -
> utils/nfsd/nfsd.c | 6 +++---
> utils/nfsd/nfssvc.c | 6 +++---
> utils/nfsd/nfssvc.h | 2 +-
> 4 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/support/include/nfs/nfs.h b/support/include/nfs/nfs.h
> index 38db5b5..df4ad76 100644
> --- a/support/include/nfs/nfs.h
> +++ b/support/include/nfs/nfs.h
> @@ -17,7 +17,6 @@
>
> #define NFS4_MINMINOR 1
> #define NFS4_MAXMINOR 2
> -#define NFS4_VERDEFAULT 0x1 /* minor verion 1 */
>
> struct nfs_fh_len {
> int fh_size;
> diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> index 6db92f0..a9d77ab 100644
> --- a/utils/nfsd/nfsd.c
> +++ b/utils/nfsd/nfsd.c
> @@ -99,7 +99,7 @@ main(int argc, char **argv)
> char *p, *progname, *port;
> char *haddr = NULL;
> int socket_up = 0;
> - int minorvers = NFS4_VERDEFAULT; /* nfsv4 minor version */
> + int minorvers[NFS4_MAXMINOR + 1] = {0};
> unsigned int versbits = NFSCTL_VERDEFAULT;
> unsigned int protobits = NFSCTL_ALLBITS;
> unsigned int proto4 = 0;
> @@ -164,7 +164,7 @@ main(int argc, char **argv)
> fprintf(stderr, "%s: unsupported minor version\n", optarg);
> exit(1);
> }
> - NFSCTL_VERUNSET(minorvers, i);
> + minorvers[i] = -1;
> break;
> }
> case 3:
> @@ -185,7 +185,7 @@ main(int argc, char **argv)
> fprintf(stderr, "%s: unsupported minor version\n", optarg);
> exit(1);
> }
> - NFSCTL_VERSET(minorvers, i);
> + minorvers[i] = 1;
> break;
> }
> case 3:
> diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
> index 8b85846..1b50aba 100644
> --- a/utils/nfsd/nfssvc.c
> +++ b/utils/nfsd/nfssvc.c
> @@ -269,7 +269,7 @@ nfssvc_set_sockets(const int family, const unsigned int protobits,
> }
>
> void
> -nfssvc_setvers(unsigned int ctlbits, int minorvers)
> +nfssvc_setvers(unsigned int ctlbits, int minorvers[])
> {
> int fd, n, off;
> char *ptr;
> @@ -281,9 +281,9 @@ nfssvc_setvers(unsigned int ctlbits, int minorvers)
> return;
>
> for (n = NFS4_MINMINOR; n <= NFS4_MAXMINOR; n++) {
> - if (NFSCTL_VERISSET(minorvers, n))
> + if (minorvers[n] == 1)
> off += snprintf(ptr+off, sizeof(buf) - off, "+4.%d ", n);
> - else
> + else if (minorvers[n] == -1)
> off += snprintf(ptr+off, sizeof(buf) - off, "-4.%d ", n);
> }
> for (n = NFSD_MINVERS; n <= NFSD_MAXVERS; n++) {
> diff --git a/utils/nfsd/nfssvc.h b/utils/nfsd/nfssvc.h
> index 08de0fe..2bbd3d3 100644
> --- a/utils/nfsd/nfssvc.h
> +++ b/utils/nfsd/nfssvc.h
> @@ -24,5 +24,5 @@ void nfssvc_mount_nfsdfs(char *progname);
> int nfssvc_inuse(void);
> int nfssvc_set_sockets(const int family, const unsigned int protobits,
> const char *host, const char *port);
> -void nfssvc_setvers(unsigned int ctlbits, int minorvers4);
> +void nfssvc_setvers(unsigned int ctlbits, int minorvers4[]);
> int nfssvc_threads(unsigned short port, int nrservs);
>

2014-01-09 16:26:11

by Joakim Tjernlund

[permalink] [raw]
Subject: Re: [PATCH] nfsd: fix minorversion-choosing interface

Steve Dickson <[email protected]> wrote on 2014/01/09 17:06:41:
>
>
> On 07/01/14 17:23, Joakim Tjernlund wrote:
> > Steve Dickson <[email protected]> wrote on 2014/01/07 22:02:01:
> >>
> >>
> >> On 16/12/13 22:43, J. Bruce Fields wrote:
> >>> From: "J. Bruce Fields" <[email protected]>
> >>>
> >>> By unconditionally adding +/-4.2 to the version string written to
the
> >>> kernel we make nfs-utils incompatible with pre-4.2-supporting
kernels.
> >>>
> >>> Ditto for 4.1. This problem was introduced by
> >>> 12a590f8d556c00a9502eeebaa763d906222d521 "rpc.nfsd: Allow v4.2
server
> >>> support with the -V option", which also change nfsd to
unconditionally
> >>> pass +/-4.2.
> >>>
> >>> Instead, just don't mention 4.1 or 4.2 unless the commandline has
> >>> specifically requested that one or the other be turned on or off.
> >>>
> >>> Tested-by: Joakim Tjernlund <[email protected]>
> >>> Reported-by: Joakim Tjernlund <[email protected]>
> >>> Signed-off-by: J. Bruce Fields <[email protected]>
> >> Committed...
> >>
> >> steved
> >
> > Thanks, do plan on making a release soon?
> Sorry for the delayed response...
>
> Its in my git tree and in Fedora 20 nfs-utils....
>
> Is this what you were looking for?

No, I am wondering when nfs-utils 1.2.10 will be released, but I see now
that
the tree is tagged with 1-2-10-rc2 so I guess it will be soon.

Jocke