/proc/fs/nfsd/versions was extended to allow turning on/off minor
versions by echoing "+4.1" or "-4.1" to /proc/fs/nsfd/versions.
Unfortunately, pre-2.6.30 kernels just stop parsing at first non-digit,
so "-4.1" is interpreted as "-4". If new nfs-utils (on old kernel)
writes "+2", "+3", "+4", then "-4.1", result therefore is to turn off
4.1.
Given that historical behavior, it may have been a mistake to extend the
interface the way we did; but at this point we're probably stuck with
it. So, just reverse the order we write versions in.
Signed-off-by: J. Bruce Fields <[email protected]>
---
utils/nfsd/nfssvc.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
index b8028bb..7bbbaba 100644
--- a/utils/nfsd/nfssvc.c
+++ b/utils/nfsd/nfssvc.c
@@ -238,17 +238,17 @@ nfssvc_setvers(unsigned int ctlbits, int minorvers4)
if (fd < 0)
return;
+ n = minorvers4 >= 0 ? minorvers4 : -minorvers4;
+ if (n >= NFSD_MINMINORVERS4 && n <= NFSD_MAXMINORVERS4)
+ off += snprintf(ptr+off, sizeof(buf) - off, "%c4.%d",
+ minorvers4 > 0 ? '+' : '-',
+ n);
for (n = NFSD_MINVERS; n <= NFSD_MAXVERS; n++) {
if (NFSCTL_VERISSET(ctlbits, n))
off += snprintf(ptr+off, sizeof(buf) - off, "+%d ", n);
else
off += snprintf(ptr+off, sizeof(buf) - off, "-%d ", n);
}
- n = minorvers4 >= 0 ? minorvers4 : -minorvers4;
- if (n >= NFSD_MINMINORVERS4 && n <= NFSD_MAXMINORVERS4)
- off += snprintf(ptr+off, sizeof(buf) - off, "%c4.%d",
- minorvers4 > 0 ? '+' : '-',
- n);
xlog(D_GENERAL, "Writing version string to kernel: %s", buf);
snprintf(ptr+off, sizeof(buf) - off, "\n");
if (write(fd, buf, strlen(buf)) != strlen(buf))
--
1.6.3.3
The current kernel code should not be enabled by default, because it
does not yet attempt to be a conform completely to the rfc; for example,
some required pieces of protocol are missing.
Therefore the kernel defaults to leaving minorversion1 off. When the
code matures sufficiently, that default will change.
That kernel default becomes meaningless if nfs-utils always explicitly
turns 4.1 on or off. So, nfs-utils should by default do nothing.
Provide a --enable-experimental-v41-support option to turn it on
explicitly. The option is intentionally spelled out (and has no short
equivalent), to help ensure that users know what they're getting into.
Once 4.1 defaults to on, that option will become unnecessary (and can
probably just be dropped from nfs-utils), and only the -N 4.1 option
will be necessary.
Signed-off-by: J. Bruce Fields <[email protected]>
---
support/include/nfs/nfs.h | 3 ---
utils/nfsd/nfsd.c | 17 +++++++++++++----
utils/nfsd/nfssvc.c | 10 ++++------
3 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/support/include/nfs/nfs.h b/support/include/nfs/nfs.h
index a64eb0a..824f604 100644
--- a/support/include/nfs/nfs.h
+++ b/support/include/nfs/nfs.h
@@ -13,9 +13,6 @@
#define NFSD_MINVERS 2
#define NFSD_MAXVERS 4
-#define NFSD_MINMINORVERS4 1
-#define NFSD_MAXMINORVERS4 1
-
struct nfs_fh_len {
int fh_size;
u_int8_t fh_handle[NFS3_FHSIZE];
diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index 1cda1e5..c72f73e 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -49,6 +49,7 @@ static struct option longopts[] =
{ "port", 1, 0, 'p' },
{ "debug", 0, 0, 'd' },
{ "syslog", 0, 0, 's' },
+ { "enable-experimental-v41-support", 0, 0, 'X' },
{ NULL, 0, 0, 0 }
};
@@ -103,7 +104,7 @@ main(int argc, char **argv)
char *p, *progname, *port;
char *haddr = NULL;
int socket_up = 0;
- int minorvers4 = NFSD_MAXMINORVERS4; /* nfsv4 minor version */
+ int minorvers41 = 0; /* nfsv4 minor version */
unsigned int versbits = NFSCTL_ALLBITS;
unsigned int protobits = NFSCTL_ALLBITS;
unsigned int proto4 = 0;
@@ -163,7 +164,12 @@ main(int argc, char **argv)
switch((c = strtol(optarg, &p, 0))) {
case 4:
if (*p == '.') {
- minorvers4 = -atoi(p + 1);
+ int i = atoi(p+1);
+ if (i != 1) {
+ fprintf(stderr, "%s: unsupported minor version\n", optarg);
+ exit(1);
+ }
+ minorvers41 = -1;
break;
}
case 3:
@@ -185,6 +191,9 @@ main(int argc, char **argv)
case 'U':
NFSCTL_UDPUNSET(protobits);
break;
+ case 'X':
+ minorvers41 = 1;
+ break;
default:
fprintf(stderr, "Invalid argument: '%c'\n", c);
case 'h':
@@ -257,7 +266,7 @@ main(int argc, char **argv)
* registered with rpcbind. Note that on older kernels w/o the right
* interfaces, these are a no-op.
*/
- nfssvc_setvers(versbits, minorvers4);
+ nfssvc_setvers(versbits, minorvers41);
error = nfssvc_set_sockets(AF_INET, proto4, haddr, port);
if (!error)
@@ -309,7 +318,7 @@ static void
usage(const char *prog)
{
fprintf(stderr, "Usage:\n"
- "%s [-d|--debug] [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version version ] [-s|--syslog] [-T|--no-tcp] [-U|--no-udp] nrservs\n",
+ "%s [-d|--debug] [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version version ] [-s|--syslog] [-T|--no-tcp] [-U|--no-udp] [--enable-experimental-v41-support] nrservs\n",
prog);
exit(2);
}
diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
index 7bbbaba..ec9446a 100644
--- a/utils/nfsd/nfssvc.c
+++ b/utils/nfsd/nfssvc.c
@@ -227,7 +227,7 @@ nfssvc_set_sockets(const int family, const unsigned int protobits,
}
void
-nfssvc_setvers(unsigned int ctlbits, int minorvers4)
+nfssvc_setvers(unsigned int ctlbits, int minorvers41)
{
int fd, n, off;
char *ptr;
@@ -238,11 +238,9 @@ nfssvc_setvers(unsigned int ctlbits, int minorvers4)
if (fd < 0)
return;
- n = minorvers4 >= 0 ? minorvers4 : -minorvers4;
- if (n >= NFSD_MINMINORVERS4 && n <= NFSD_MAXMINORVERS4)
- off += snprintf(ptr+off, sizeof(buf) - off, "%c4.%d",
- minorvers4 > 0 ? '+' : '-',
- n);
+ if (minorvers41)
+ off += snprintf(ptr+off, sizeof(buf) - off, "%c4.1",
+ minorvers41 > 0 ? '+' : '-');
for (n = NFSD_MINVERS; n <= NFSD_MAXVERS; n++) {
if (NFSCTL_VERISSET(ctlbits, n))
off += snprintf(ptr+off, sizeof(buf) - off, "+%d ", n);
--
1.6.3.3
On Fri, Feb 12, 2010 at 04:44:03PM -0500, Steve Dickson wrote:
>
>
> On 02/12/2010 03:05 PM, J. Bruce Fields wrote:
> > On Fri, Feb 12, 2010 at 02:58:23PM -0500, Steve Dickson wrote:
> >>
> >>
> >> On 02/05/2010 03:05 PM, J. Bruce Fields wrote:
> >>> From: J. Bruce Fields <[email protected]>
> >>>
> >>> The current kernel code should not be enabled by default, because it
> >>> does not yet attempt to be a conform completely to the rfc; for example,
> >>> some required pieces of protocol are missing.
> >>>
> >>> Therefore the kernel defaults to leaving minorversion1 off. When the
> >>> code matures sufficiently, that default will change.
> >>>
> >>> That kernel default becomes meaningless if nfs-utils always explicitly
> >>> turns 4.1 on or off. So, nfs-utils should by default do nothing.
> >>>
> >>> Early adopters that want to turn on NFSv4.1 explicitly can still do so
> >>> using
> >>>
> >>> echo "+4.1" >/proc/fs/nfsd/versions
> >>>
> >> When I write to /proc/fs/nfsd/versions I'm getting
> >> write error: Device or resource busy
> >>
> >> What did you do to make the file writeable?
> >
> > You just need to do it before starting nfsd.
> Well before nfsd starts but after the nfsd module is loaded
> (assuming nfsd is a module)
Right.
> > So if it's just a one-off experiment you could
> >
> > /etc/init.d/nfs-server stop
> > echo "+4.1" >/proc/fs/nsfd/versions
> > /etc/init.d/nfs-server start
> >
> > On machines where I was using 4.1 regularly I'd probably at a line to
> > the init script, or to a local init script that ran before it.
> Yea.. I'm looking into do something of this nature...
>
> The odd thing about this patch is 4.1 can only be turned off. There
> is no way to enabled except from doing the above echo... which
> seems to beg the question, why have the 4.1 code in nfsd at all?
So people can test it.
We should not recommend it for production use at this point, at least
not by people who are not fully aware of what they're doing.
If you'd rather we added an option to rpc.nfsd to do the equivalent of
the echo, that's fine. But I think for testers/early adopters, adding a
"echo" to their init script isn't a big deal.
Of course, once it's mature we can flip the kernel default to on, and
nobody will have to do anything.
--b.
On Fri, Feb 05, 2010 at 11:10:02AM -0500, J. Bruce Fields wrote:
> On Thu, Feb 04, 2010 at 05:19:56PM -0500, Steve Dickson wrote:
> > On 02/01/2010 02:58 PM, J. Bruce Fields wrote:
> > > On Wed, Jan 27, 2010 at 05:26:06PM -0500, J. Bruce Fields wrote:
> > >> The current kernel code should not be enabled by default, because it
> > >> does not yet attempt to be a conform completely to the rfc; for example,
> > >> some required pieces of protocol are missing.
> > >>
> > >> Therefore the kernel defaults to leaving minorversion1 off. When the
> > >> code matures sufficiently, that default will change.
> > >>
> > >> That kernel default becomes meaningless if nfs-utils always explicitly
> > >> turns 4.1 on or off. So, nfs-utils should by default do nothing.
> > >>
> > >> Provide a --enable-experimental-v41-support option to turn it on
> > >> explicitly. The option is intentionally spelled out (and has no short
> > >> equivalent), to help ensure that users know what they're getting into.
> > Command options like this are so hard to get rid of.... We just can't introduce
> > an option one release and then have it go away a few releases down the road.
> > That's sure fire way to breaking existing configurations which something that is,
> > has been and will continue to be unacceptable...
>
> OK, fair enough. It shouldn't be a problem to keep it indefinitely,
> though.
Actually, an even simpler option: instead of adding a new option, just
modify the code so that the *absence* of -N4.1 isn't taken to mean
"please turn on 4.1". People can always just do their own
echo +4.1 >/proc/fs/nfsd/version
if that's what they want.
--b.
On Fri, Feb 12, 2010 at 02:58:23PM -0500, Steve Dickson wrote:
>
>
> On 02/05/2010 03:05 PM, J. Bruce Fields wrote:
> > From: J. Bruce Fields <[email protected]>
> >
> > The current kernel code should not be enabled by default, because it
> > does not yet attempt to be a conform completely to the rfc; for example,
> > some required pieces of protocol are missing.
> >
> > Therefore the kernel defaults to leaving minorversion1 off. When the
> > code matures sufficiently, that default will change.
> >
> > That kernel default becomes meaningless if nfs-utils always explicitly
> > turns 4.1 on or off. So, nfs-utils should by default do nothing.
> >
> > Early adopters that want to turn on NFSv4.1 explicitly can still do so
> > using
> >
> > echo "+4.1" >/proc/fs/nfsd/versions
> >
> When I write to /proc/fs/nfsd/versions I'm getting
> write error: Device or resource busy
>
> What did you do to make the file writeable?
You just need to do it before starting nfsd.
So if it's just a one-off experiment you could
/etc/init.d/nfs-server stop
echo "+4.1" >/proc/fs/nsfd/versions
/etc/init.d/nfs-server start
On machines where I was using 4.1 regularly I'd probably at a line to
the init script, or to a local init script that ran before it.
--b.
The fact there needed to be yet another code change to
re-enabled the 4.1 functionality bother me... So This
patch basically does the same as your patch, does not
write "+4.1" to the versions file. But it also introduces
a configuration variable that will allow us to re-enabled
the functionality w/out changing any code...
BTW, there was precedence with adding this type of
configuration variable since there has been
NFS3_SUPPORTED and NFS4_SUPPORTED variables in the
past.
steved.
commit 6d5ac3fa75024be569b458f4d9b6ce05be47f601
Author: Steve Dickson <[email protected]>
Date: Wed Feb 17 14:38:19 2010 -0500
nfsd: Disble NFS 4.1 functionality by default
Due to the fact the current kernel code do not completely
conform to the NFS 4.1 RFC, this patch disable the 4.1 support
on the server.
To control this 41 functionality, the NFS41_SUPPORTED
configuration variable now exist that will allow us to
re enable the functionality without any code changes.
Signed-off-by: Steve Dickson <[email protected]>
diff --git a/configure.ac b/configure.ac
index 1dc4249..f6b1189 100644
--- a/configure.ac
+++ b/configure.ac
@@ -72,6 +72,20 @@ AC_ARG_ENABLE(nfsv4,
AC_SUBST(IDMAPD)
AC_SUBST(enable_nfsv4)
AM_CONDITIONAL(CONFIG_NFSV4, [test "$enable_nfsv4" = "yes"])
+
+AC_ARG_ENABLE(nfsv41,
+ [AC_HELP_STRING([--enable-nfsv41],
+ [enable support for NFSv41 @<:@default=no@:>@])],
+ enable_nfsv41=$enableval,
+ enable_nfsv41=no)
+ if test "$enable_nfsv41" = yes; then
+ AC_DEFINE(NFS41_SUPPORTED, 1, [Define this if you want NFSv41 support compiled in])
+ else
+ enable_nfsv4=
+ fi
+ AC_SUBST(enable_nfsv41)
+ AM_CONDITIONAL(CONFIG_NFSV41, [test "$enable_nfsv41" = "yes"])
+
AC_ARG_ENABLE(gss,
[AC_HELP_STRING([--enable-gss],
[enable support for rpcsec_gss @<:@default=yes@:>@])],
diff --git a/support/include/nfs/nfs.h b/support/include/nfs/nfs.h
index a64eb0a..c939d78 100644
--- a/support/include/nfs/nfs.h
+++ b/support/include/nfs/nfs.h
@@ -1,6 +1,8 @@
#ifndef _NFS_NFS_H
#define _NFS_NFS_H
+#include <config.h>
+
#include <linux/posix_types.h>
#include <sys/types.h>
#include <netinet/in.h>
@@ -14,7 +16,11 @@
#define NFSD_MAXVERS 4
#define NFSD_MINMINORVERS4 1
+#ifdef NFS41_SUPPORTED
#define NFSD_MAXMINORVERS4 1
+#else
+#define NFSD_MAXMINORVERS4 0
+#endif
struct nfs_fh_len {
int fh_size;
On 02/05/2010 03:05 PM, J. Bruce Fields wrote:
> From: J. Bruce Fields <[email protected]>
>
> The current kernel code should not be enabled by default, because it
> does not yet attempt to be a conform completely to the rfc; for example,
> some required pieces of protocol are missing.
>
> Therefore the kernel defaults to leaving minorversion1 off. When the
> code matures sufficiently, that default will change.
>
> That kernel default becomes meaningless if nfs-utils always explicitly
> turns 4.1 on or off. So, nfs-utils should by default do nothing.
>
> Early adopters that want to turn on NFSv4.1 explicitly can still do so
> using
>
> echo "+4.1" >/proc/fs/nfsd/versions
>
When I write to /proc/fs/nfsd/versions I'm getting
write error: Device or resource busy
What did you do to make the file writeable?
steved.
On 02/12/2010 03:05 PM, J. Bruce Fields wrote:
> On Fri, Feb 12, 2010 at 02:58:23PM -0500, Steve Dickson wrote:
>>
>>
>> On 02/05/2010 03:05 PM, J. Bruce Fields wrote:
>>> From: J. Bruce Fields <[email protected]>
>>>
>>> The current kernel code should not be enabled by default, because it
>>> does not yet attempt to be a conform completely to the rfc; for example,
>>> some required pieces of protocol are missing.
>>>
>>> Therefore the kernel defaults to leaving minorversion1 off. When the
>>> code matures sufficiently, that default will change.
>>>
>>> That kernel default becomes meaningless if nfs-utils always explicitly
>>> turns 4.1 on or off. So, nfs-utils should by default do nothing.
>>>
>>> Early adopters that want to turn on NFSv4.1 explicitly can still do so
>>> using
>>>
>>> echo "+4.1" >/proc/fs/nfsd/versions
>>>
>> When I write to /proc/fs/nfsd/versions I'm getting
>> write error: Device or resource busy
>>
>> What did you do to make the file writeable?
>
> You just need to do it before starting nfsd.
Well before nfsd starts but after the nfsd module is loaded
(assuming nfsd is a module)
>
> So if it's just a one-off experiment you could
>
> /etc/init.d/nfs-server stop
> echo "+4.1" >/proc/fs/nsfd/versions
> /etc/init.d/nfs-server start
>
> On machines where I was using 4.1 regularly I'd probably at a line to
> the init script, or to a local init script that ran before it.
Yea.. I'm looking into do something of this nature...
The odd thing about this patch is 4.1 can only be turned off. There
is no way to enabled except from doing the above echo... which
seems to beg the question, why have the 4.1 code in nfsd at all?
steved.
On 02/01/2010 02:58 PM, J. Bruce Fields wrote:
> On Wed, Jan 27, 2010 at 05:26:06PM -0500, J. Bruce Fields wrote:
>> The current kernel code should not be enabled by default, because it
>> does not yet attempt to be a conform completely to the rfc; for example,
>> some required pieces of protocol are missing.
>>
>> Therefore the kernel defaults to leaving minorversion1 off. When the
>> code matures sufficiently, that default will change.
>>
>> That kernel default becomes meaningless if nfs-utils always explicitly
>> turns 4.1 on or off. So, nfs-utils should by default do nothing.
>>
>> Provide a --enable-experimental-v41-support option to turn it on
>> explicitly. The option is intentionally spelled out (and has no short
>> equivalent), to help ensure that users know what they're getting into.
Command options like this are so hard to get rid of.... We just can't introduce
an option one release and then have it go away a few releases down the road.
That's sure fire way to breaking existing configurations which something that is,
has been and will continue to be unacceptable...
>>
>> Once 4.1 defaults to on, that option will become unnecessary (and can
>> probably just be dropped from nfs-utils), and only the -N 4.1 option
>> will be necessary.
>
> We need to figure out how we're going to handle this. The current
> situation (ignoring the kernel's default) isn't acceptable.
Why can't each distro simple turn it off with there init scripts?
>
> We need to figure out something for v4 as well. If every distro has run
> nfsd without -N4, depending on the lack of fsid=0 to keep nfsv4 off by
> default, then we're effectively turning v4 on by default with the
> pseudoroot changes. But there are good reasons why the v4 server code
> is still marked experimental.
I agree that with the latest kernels, v4 will be the default version. But
there are ways to override this on both the server and client. So why let
the users decided what they want?
steved.
On Thu, Feb 04, 2010 at 05:19:56PM -0500, Steve Dickson wrote:
> On 02/01/2010 02:58 PM, J. Bruce Fields wrote:
> > On Wed, Jan 27, 2010 at 05:26:06PM -0500, J. Bruce Fields wrote:
> >> The current kernel code should not be enabled by default, because it
> >> does not yet attempt to be a conform completely to the rfc; for example,
> >> some required pieces of protocol are missing.
> >>
> >> Therefore the kernel defaults to leaving minorversion1 off. When the
> >> code matures sufficiently, that default will change.
> >>
> >> That kernel default becomes meaningless if nfs-utils always explicitly
> >> turns 4.1 on or off. So, nfs-utils should by default do nothing.
> >>
> >> Provide a --enable-experimental-v41-support option to turn it on
> >> explicitly. The option is intentionally spelled out (and has no short
> >> equivalent), to help ensure that users know what they're getting into.
> Command options like this are so hard to get rid of.... We just can't introduce
> an option one release and then have it go away a few releases down the road.
> That's sure fire way to breaking existing configurations which something that is,
> has been and will continue to be unacceptable...
OK, fair enough. It shouldn't be a problem to keep it indefinitely,
though.
> >> Once 4.1 defaults to on, that option will become unnecessary (and can
> >> probably just be dropped from nfs-utils), and only the -N 4.1 option
> >> will be necessary.
> >
> > We need to figure out how we're going to handle this. The current
> > situation (ignoring the kernel's default) isn't acceptable.
> Why can't each distro simple turn it off with there init scripts?
On the server-side the kernel doesn't have a separate config option for
V4.1; the only way for the kernel to indicate whether it has a mature
version of V4.1 is by its choice of default.
> > We need to figure out something for v4 as well. If every distro has run
> > nfsd without -N4, depending on the lack of fsid=0 to keep nfsv4 off by
> > default, then we're effectively turning v4 on by default with the
> > pseudoroot changes. But there are good reasons why the v4 server code
> > is still marked experimental.
> I agree that with the latest kernels, v4 will be the default version. But
> there are ways to override this on both the server and client. So why let
> the users decided what they want?
I don't understand.
--b.
From: J. Bruce Fields <[email protected]>
The current kernel code should not be enabled by default, because it
does not yet attempt to be a conform completely to the rfc; for example,
some required pieces of protocol are missing.
Therefore the kernel defaults to leaving minorversion1 off. When the
code matures sufficiently, that default will change.
That kernel default becomes meaningless if nfs-utils always explicitly
turns 4.1 on or off. So, nfs-utils should by default do nothing.
Early adopters that want to turn on NFSv4.1 explicitly can still do so
using
echo "+4.1" >/proc/fs/nfsd/versions
Signed-off-by: J. Bruce Fields <[email protected]>
---
utils/nfsd/nfssvc.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
index 7bbbaba..1fb420d 100644
--- a/utils/nfsd/nfssvc.c
+++ b/utils/nfsd/nfssvc.c
@@ -239,10 +239,9 @@ nfssvc_setvers(unsigned int ctlbits, int minorvers4)
return;
n = minorvers4 >= 0 ? minorvers4 : -minorvers4;
- if (n >= NFSD_MINMINORVERS4 && n <= NFSD_MAXMINORVERS4)
- off += snprintf(ptr+off, sizeof(buf) - off, "%c4.%d",
- minorvers4 > 0 ? '+' : '-',
- n);
+ if (minorvers4 < 0 && n >= NFSD_MINMINORVERS4
+ && n <= NFSD_MAXMINORVERS4)
+ off += snprintf(ptr+off, sizeof(buf) - off, "-4.%d", n);
for (n = NFSD_MINVERS; n <= NFSD_MAXVERS; n++) {
if (NFSCTL_VERISSET(ctlbits, n))
off += snprintf(ptr+off, sizeof(buf) - off, "+%d ", n);
--
1.6.3.3
On Wed, Jan 27, 2010 at 05:26:06PM -0500, J. Bruce Fields wrote:
> The current kernel code should not be enabled by default, because it
> does not yet attempt to be a conform completely to the rfc; for example,
> some required pieces of protocol are missing.
>
> Therefore the kernel defaults to leaving minorversion1 off. When the
> code matures sufficiently, that default will change.
>
> That kernel default becomes meaningless if nfs-utils always explicitly
> turns 4.1 on or off. So, nfs-utils should by default do nothing.
>
> Provide a --enable-experimental-v41-support option to turn it on
> explicitly. The option is intentionally spelled out (and has no short
> equivalent), to help ensure that users know what they're getting into.
>
> Once 4.1 defaults to on, that option will become unnecessary (and can
> probably just be dropped from nfs-utils), and only the -N 4.1 option
> will be necessary.
We need to figure out how we're going to handle this. The current
situation (ignoring the kernel's default) isn't acceptable.
We need to figure out something for v4 as well. If every distro has run
nfsd without -N4, depending on the lack of fsid=0 to keep nfsv4 off by
default, then we're effectively turning v4 on by default with the
pseudoroot changes. But there are good reasons why the v4 server code
is still marked experimental.
--b.
>
> Signed-off-by: J. Bruce Fields <[email protected]>
> ---
> support/include/nfs/nfs.h | 3 ---
> utils/nfsd/nfsd.c | 17 +++++++++++++----
> utils/nfsd/nfssvc.c | 10 ++++------
> 3 files changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/support/include/nfs/nfs.h b/support/include/nfs/nfs.h
> index a64eb0a..824f604 100644
> --- a/support/include/nfs/nfs.h
> +++ b/support/include/nfs/nfs.h
> @@ -13,9 +13,6 @@
> #define NFSD_MINVERS 2
> #define NFSD_MAXVERS 4
>
> -#define NFSD_MINMINORVERS4 1
> -#define NFSD_MAXMINORVERS4 1
> -
> struct nfs_fh_len {
> int fh_size;
> u_int8_t fh_handle[NFS3_FHSIZE];
> diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> index 1cda1e5..c72f73e 100644
> --- a/utils/nfsd/nfsd.c
> +++ b/utils/nfsd/nfsd.c
> @@ -49,6 +49,7 @@ static struct option longopts[] =
> { "port", 1, 0, 'p' },
> { "debug", 0, 0, 'd' },
> { "syslog", 0, 0, 's' },
> + { "enable-experimental-v41-support", 0, 0, 'X' },
> { NULL, 0, 0, 0 }
> };
>
> @@ -103,7 +104,7 @@ main(int argc, char **argv)
> char *p, *progname, *port;
> char *haddr = NULL;
> int socket_up = 0;
> - int minorvers4 = NFSD_MAXMINORVERS4; /* nfsv4 minor version */
> + int minorvers41 = 0; /* nfsv4 minor version */
> unsigned int versbits = NFSCTL_ALLBITS;
> unsigned int protobits = NFSCTL_ALLBITS;
> unsigned int proto4 = 0;
> @@ -163,7 +164,12 @@ main(int argc, char **argv)
> switch((c = strtol(optarg, &p, 0))) {
> case 4:
> if (*p == '.') {
> - minorvers4 = -atoi(p + 1);
> + int i = atoi(p+1);
> + if (i != 1) {
> + fprintf(stderr, "%s: unsupported minor version\n", optarg);
> + exit(1);
> + }
> + minorvers41 = -1;
> break;
> }
> case 3:
> @@ -185,6 +191,9 @@ main(int argc, char **argv)
> case 'U':
> NFSCTL_UDPUNSET(protobits);
> break;
> + case 'X':
> + minorvers41 = 1;
> + break;
> default:
> fprintf(stderr, "Invalid argument: '%c'\n", c);
> case 'h':
> @@ -257,7 +266,7 @@ main(int argc, char **argv)
> * registered with rpcbind. Note that on older kernels w/o the right
> * interfaces, these are a no-op.
> */
> - nfssvc_setvers(versbits, minorvers4);
> + nfssvc_setvers(versbits, minorvers41);
>
> error = nfssvc_set_sockets(AF_INET, proto4, haddr, port);
> if (!error)
> @@ -309,7 +318,7 @@ static void
> usage(const char *prog)
> {
> fprintf(stderr, "Usage:\n"
> - "%s [-d|--debug] [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version version ] [-s|--syslog] [-T|--no-tcp] [-U|--no-udp] nrservs\n",
> + "%s [-d|--debug] [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version version ] [-s|--syslog] [-T|--no-tcp] [-U|--no-udp] [--enable-experimental-v41-support] nrservs\n",
> prog);
> exit(2);
> }
> diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
> index 7bbbaba..ec9446a 100644
> --- a/utils/nfsd/nfssvc.c
> +++ b/utils/nfsd/nfssvc.c
> @@ -227,7 +227,7 @@ nfssvc_set_sockets(const int family, const unsigned int protobits,
> }
>
> void
> -nfssvc_setvers(unsigned int ctlbits, int minorvers4)
> +nfssvc_setvers(unsigned int ctlbits, int minorvers41)
> {
> int fd, n, off;
> char *ptr;
> @@ -238,11 +238,9 @@ nfssvc_setvers(unsigned int ctlbits, int minorvers4)
> if (fd < 0)
> return;
>
> - n = minorvers4 >= 0 ? minorvers4 : -minorvers4;
> - if (n >= NFSD_MINMINORVERS4 && n <= NFSD_MAXMINORVERS4)
> - off += snprintf(ptr+off, sizeof(buf) - off, "%c4.%d",
> - minorvers4 > 0 ? '+' : '-',
> - n);
> + if (minorvers41)
> + off += snprintf(ptr+off, sizeof(buf) - off, "%c4.1",
> + minorvers41 > 0 ? '+' : '-');
> for (n = NFSD_MINVERS; n <= NFSD_MAXVERS; n++) {
> if (NFSCTL_VERISSET(ctlbits, n))
> off += snprintf(ptr+off, sizeof(buf) - off, "+%d ", n);
> --
> 1.6.3.3
>
On 01/27/2010 05:26 PM, J. Bruce Fields wrote:
> /proc/fs/nfsd/versions was extended to allow turning on/off minor
> versions by echoing "+4.1" or "-4.1" to /proc/fs/nsfd/versions.
>
> Unfortunately, pre-2.6.30 kernels just stop parsing at first non-digit,
> so "-4.1" is interpreted as "-4". If new nfs-utils (on old kernel)
> writes "+2", "+3", "+4", then "-4.1", result therefore is to turn off
> 4.1.
>
> Given that historical behavior, it may have been a mistake to extend the
> interface the way we did; but at this point we're probably stuck with
> it. So, just reverse the order we write versions in.
>
> Signed-off-by: J. Bruce Fields <[email protected]>
> ---
> utils/nfsd/nfssvc.c | 10 +++++-----
> 1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
> index b8028bb..7bbbaba 100644
> --- a/utils/nfsd/nfssvc.c
> +++ b/utils/nfsd/nfssvc.c
> @@ -238,17 +238,17 @@ nfssvc_setvers(unsigned int ctlbits, int minorvers4)
> if (fd < 0)
> return;
>
> + n = minorvers4 >= 0 ? minorvers4 : -minorvers4;
> + if (n >= NFSD_MINMINORVERS4 && n <= NFSD_MAXMINORVERS4)
> + off += snprintf(ptr+off, sizeof(buf) - off, "%c4.%d",
There needs to be a space between the %d and ".
But more importantly what problem is the patch fixing?? I've tested this
on a 2.6.29 and 2.6.31 as well as a 2.6.33 kernel and I see no different
in how the versions are ordered.
Note, as you probably already know, the kernel will always display
the versions in descending order...
steved.
On Thu, Feb 04, 2010 at 04:37:52PM -0500, Steve Dickson wrote:
>
>
> On 01/27/2010 05:26 PM, J. Bruce Fields wrote:
> > /proc/fs/nfsd/versions was extended to allow turning on/off minor
> > versions by echoing "+4.1" or "-4.1" to /proc/fs/nsfd/versions.
> >
> > Unfortunately, pre-2.6.30 kernels just stop parsing at first non-digit,
> > so "-4.1" is interpreted as "-4". If new nfs-utils (on old kernel)
> > writes "+2", "+3", "+4", then "-4.1", result therefore is to turn off
> > 4.1.
> >
> > Given that historical behavior, it may have been a mistake to extend the
> > interface the way we did; but at this point we're probably stuck with
> > it. So, just reverse the order we write versions in.
> >
> > Signed-off-by: J. Bruce Fields <[email protected]>
> > ---
> > utils/nfsd/nfssvc.c | 10 +++++-----
> > 1 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
> > index b8028bb..7bbbaba 100644
> > --- a/utils/nfsd/nfssvc.c
> > +++ b/utils/nfsd/nfssvc.c
> > @@ -238,17 +238,17 @@ nfssvc_setvers(unsigned int ctlbits, int minorvers4)
> > if (fd < 0)
> > return;
> >
> > + n = minorvers4 >= 0 ? minorvers4 : -minorvers4;
> > + if (n >= NFSD_MINMINORVERS4 && n <= NFSD_MAXMINORVERS4)
> > + off += snprintf(ptr+off, sizeof(buf) - off, "%c4.%d",
> There needs to be a space between the %d and ".
>
> But more importantly what problem is the patch fixing?? I've tested this
> on a 2.6.29 and 2.6.31 as well as a 2.6.33 kernel and I see no different
> in how the versions are ordered.
See the patch description. A test case:
- boot 2.6.29
- run 'nfsd -N4.1'
- try to mount v4.0.
See also https://bugzilla.redhat.com/show_bug.cgi?id=512377.
--b.
> Note, as you probably already know, the kernel will always display
> the versions in descending order...
>
> steved.
>
>
On 02/04/2010 04:47 PM, J. Bruce Fields wrote:
> On Thu, Feb 04, 2010 at 04:37:52PM -0500, Steve Dickson wrote:
>>
>>
>> On 01/27/2010 05:26 PM, J. Bruce Fields wrote:
>>> /proc/fs/nfsd/versions was extended to allow turning on/off minor
>>> versions by echoing "+4.1" or "-4.1" to /proc/fs/nsfd/versions.
>>>
>>> Unfortunately, pre-2.6.30 kernels just stop parsing at first non-digit,
>>> so "-4.1" is interpreted as "-4". If new nfs-utils (on old kernel)
>>> writes "+2", "+3", "+4", then "-4.1", result therefore is to turn off
>>> 4.1.
>>>
>>> Given that historical behavior, it may have been a mistake to extend the
>>> interface the way we did; but at this point we're probably stuck with
>>> it. So, just reverse the order we write versions in.
>>>
>>> Signed-off-by: J. Bruce Fields <[email protected]>
>>> ---
>>> utils/nfsd/nfssvc.c | 10 +++++-----
>>> 1 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
>>> index b8028bb..7bbbaba 100644
>>> --- a/utils/nfsd/nfssvc.c
>>> +++ b/utils/nfsd/nfssvc.c
>>> @@ -238,17 +238,17 @@ nfssvc_setvers(unsigned int ctlbits, int minorvers4)
>>> if (fd < 0)
>>> return;
>>>
>>> + n = minorvers4 >= 0 ? minorvers4 : -minorvers4;
>>> + if (n >= NFSD_MINMINORVERS4 && n <= NFSD_MAXMINORVERS4)
>>> + off += snprintf(ptr+off, sizeof(buf) - off, "%c4.%d",
>> There needs to be a space between the %d and ".
>>
>> But more importantly what problem is the patch fixing?? I've tested this
>> on a 2.6.29 and 2.6.31 as well as a 2.6.33 kernel and I see no different
>> in how the versions are ordered.
>
> See the patch description. A test case:
>
> - boot 2.6.29
> - run 'nfsd -N4.1'
> - try to mount v4.0.
>
> See also https://bugzilla.redhat.com/show_bug.cgi?id=512377.
Ah... I see it now... for some reason I thought this was fixed already..
Committed...
steved.
On Thu, Feb 04, 2010 at 05:06:34PM -0500, Steve Dickson wrote:
>
>
> On 02/04/2010 04:47 PM, J. Bruce Fields wrote:
> > On Thu, Feb 04, 2010 at 04:37:52PM -0500, Steve Dickson wrote:
> >>
> >>
> >> On 01/27/2010 05:26 PM, J. Bruce Fields wrote:
> >>> /proc/fs/nfsd/versions was extended to allow turning on/off minor
> >>> versions by echoing "+4.1" or "-4.1" to /proc/fs/nsfd/versions.
> >>>
> >>> Unfortunately, pre-2.6.30 kernels just stop parsing at first non-digit,
> >>> so "-4.1" is interpreted as "-4". If new nfs-utils (on old kernel)
> >>> writes "+2", "+3", "+4", then "-4.1", result therefore is to turn off
> >>> 4.1.
> >>>
> >>> Given that historical behavior, it may have been a mistake to extend the
> >>> interface the way we did; but at this point we're probably stuck with
> >>> it. So, just reverse the order we write versions in.
> >>>
> >>> Signed-off-by: J. Bruce Fields <[email protected]>
> >>> ---
> >>> utils/nfsd/nfssvc.c | 10 +++++-----
> >>> 1 files changed, 5 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
> >>> index b8028bb..7bbbaba 100644
> >>> --- a/utils/nfsd/nfssvc.c
> >>> +++ b/utils/nfsd/nfssvc.c
> >>> @@ -238,17 +238,17 @@ nfssvc_setvers(unsigned int ctlbits, int minorvers4)
> >>> if (fd < 0)
> >>> return;
> >>>
> >>> + n = minorvers4 >= 0 ? minorvers4 : -minorvers4;
> >>> + if (n >= NFSD_MINMINORVERS4 && n <= NFSD_MAXMINORVERS4)
> >>> + off += snprintf(ptr+off, sizeof(buf) - off, "%c4.%d",
> >> There needs to be a space between the %d and ".
> >>
> >> But more importantly what problem is the patch fixing?? I've tested this
> >> on a 2.6.29 and 2.6.31 as well as a 2.6.33 kernel and I see no different
> >> in how the versions are ordered.
> >
> > See the patch description. A test case:
> >
> > - boot 2.6.29
> > - run 'nfsd -N4.1'
> > - try to mount v4.0.
> >
> > See also https://bugzilla.redhat.com/show_bug.cgi?id=512377.
> Ah... I see it now... for some reason I thought this was fixed already..
I think you worked around it in fedora by dropping the -N4.1 from the
initscripts temporarily?
Thanks for catching the missing space, obviously I hadn't tested this
version properly!
--b.