[PATCH][nfs-utils] configuration: Fix a typo in configure.ac

[PATCH][nfs-utils] configuration: Fix a typo in configure.ac

[Trond Myklebust]

From: Trond Myklebust <Trond.Myklebust@netapp.com>

Fix a typo in commit 6d5ac3fa (nfsd: Disble NFS 4.1 functionality by default).

We did not really intend to make NFSv4.0 support conditional on NFSv4.1
being enabled.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 configure.ac |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


diff --git a/configure.ac b/configure.ac
index b7520d8..518b6d8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -81,7 +81,7 @@ AC_ARG_ENABLE(nfsv41,
 	if test "$enable_nfsv41" = yes; then
 		AC_DEFINE(NFS41_SUPPORTED, 1, [Define this if you want NFSv41 support compiled in])
 	else
-		enable_nfsv4=
+		enable_nfsv41=
 	fi
 	AC_SUBST(enable_nfsv41)
 	AM_CONDITIONAL(CONFIG_NFSV41, [test "$enable_nfsv41" = "yes"])

Re: [PATCH][nfs-utils] configuration: Fix a typo in configure.ac

[J. Bruce Fields]

On Sat, Feb 20, 2010 at 03:45:10PM -0800, Trond Myklebust wrote:
> From: Trond Myklebust <Trond.Myklebust@netapp.com>
> 
> Fix a typo in commit 6d5ac3fa (nfsd: Disble NFS 4.1 functionality by default).

Better: please just rever 6d5ac3fa and apply
http://marc.info/?l=linux-nfs&m=126540028610022&w=2

	- distributions shouldn't be turning on 4.1 by default yet.
	  Even if no clients *use* it by default, future clients will,
	  even with older servers that shouldn't have had 4.1 on yet.
	- In addition to the bug Trond found, 6d5ac3fa also
	  inadvertently disables -N4 in the disable_nfsv41 case.

--b.

> 
> We did not really intend to make NFSv4.0 support conditional on NFSv4.1
> being enabled.
> 
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
> 
>  configure.ac |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> 
> diff --git a/configure.ac b/configure.ac
> index b7520d8..518b6d8 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -81,7 +81,7 @@ AC_ARG_ENABLE(nfsv41,
>  	if test "$enable_nfsv41" = yes; then
>  		AC_DEFINE(NFS41_SUPPORTED, 1, [Define this if you want NFSv41 support compiled in])
>  	else
> -		enable_nfsv4=
> +		enable_nfsv41=
>  	fi
>  	AC_SUBST(enable_nfsv41)
>  	AM_CONDITIONAL(CONFIG_NFSV41, [test "$enable_nfsv41" = "yes"])
> 
> --
> 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

Re: [PATCH][nfs-utils] configuration: Fix a typo in configure.ac

[Steve Dickson]

On 02/22/2010 01:22 PM, J. Bruce Fields wrote:
> On Sat, Feb 20, 2010 at 03:45:10PM -0800, Trond Myklebust wrote:
>> From: Trond Myklebust <Trond.Myklebust@netapp.com>
>>
>> Fix a typo in commit 6d5ac3fa (nfsd: Disble NFS 4.1 functionality by default).
> 
> Better: please just rever 6d5ac3fa and apply
> http://marc.info/?l=linux-nfs&m=126540028610022&w=2
The problem I saw with the this patch was it would take 
a code change to re-enable the 4.1 functional, verses a
enabling a configuration flag. Plus, there has been a 
precedence set of having these types of configuration flags,
note the enable_nfsv3, enable_nfsv4, so adding a enable_nfsv41
seem to me made sense...

> 
> 	- distributions shouldn't be turning on 4.1 by default yet.
Its not... If the --enable_nfsv41 is not set, the 4.1 functional is
off. Having this type of flag enables the distros to enable the
functionality on development kernels but disable it on stable 
kernels, without any additional patches. Again to me, its seems
like the cleanest way to do it... 

> 	- In addition to the bug Trond found, 6d5ac3fa also
> 	  inadvertently disables -N4 in the disable_nfsv41 case.
Fixed...

steved.

Re: [PATCH][nfs-utils] configuration: Fix a typo in configure.ac

[J. Bruce Fields]

On Mon, Mar 01, 2010 at 07:57:28AM -0500, Steve Dickson wrote:
> On 02/22/2010 01:22 PM, J. Bruce Fields wrote:
> > On Sat, Feb 20, 2010 at 03:45:10PM -0800, Trond Myklebust wrote:
> >> From: Trond Myklebust <Trond.Myklebust@netapp.com>
> >>
> >> Fix a typo in commit 6d5ac3fa (nfsd: Disble NFS 4.1 functionality by default).
> > 
> > Better: please just rever 6d5ac3fa and apply
> > http://marc.info/?l=linux-nfs&m=126540028610022&w=2
> The problem I saw with the this patch was it would take 
> a code change to re-enable the 4.1 functional,

Enabling 4.1 will require no code changes to nfs-utils, only to the
kernel.  (But it will require a lot of code changes to nfs-utils, as we
should complete 4.1 support first.)

> verses a
> enabling a configuration flag. Plus, there has been a 
> precedence set of having these types of configuration flags,
> note the enable_nfsv3, enable_nfsv4, so adding a enable_nfsv41
> seem to me made sense...
> 
> > 
> > 	- distributions shouldn't be turning on 4.1 by default yet.
> Its not... If the --enable_nfsv41 is not set, the 4.1 functional is
> off. Having this type of flag enables the distros to enable the
> functionality on development kernels but disable it on stable 

So you want to enable it (for example) in Fedora, but disable it in
RHEL?

I don't think that's a good idea.  If servers running current Fedora
kernels are still around when the client starts trying 4.1 first, then
we'll run into trouble.

--b.

> kernels, without any additional patches. Again to me, its seems
> like the cleanest way to do it... 
> 
> > 	- In addition to the bug Trond found, 6d5ac3fa also
> > 	  inadvertently disables -N4 in the disable_nfsv41 case.
> Fixed...
> 
> steved.

Re: [PATCH][nfs-utils] configuration: Fix a typo in configure.ac

[Steve Dickson]

On 03/01/2010 08:25 AM, J. Bruce Fields wrote:
> On Mon, Mar 01, 2010 at 07:57:28AM -0500, Steve Dickson wrote:
>> On 02/22/2010 01:22 PM, J. Bruce Fields wrote:
>>> On Sat, Feb 20, 2010 at 03:45:10PM -0800, Trond Myklebust wrote:
>>>> From: Trond Myklebust <Trond.Myklebust@netapp.com>
>>>>
>>>> Fix a typo in commit 6d5ac3fa (nfsd: Disble NFS 4.1 functionality by default).
>>>
>>> Better: please just rever 6d5ac3fa and apply
>>> http://marc.info/?l=linux-nfs&m=126540028610022&w=2
>> The problem I saw with the this patch was it would take 
>> a code change to re-enable the 4.1 functional,
> 
> Enabling 4.1 will require no code changes to nfs-utils, only to the
> kernel.  (But it will require a lot of code changes to nfs-utils, as we
> should complete 4.1 support first.)
> 
>> verses a
>> enabling a configuration flag. Plus, there has been a 
>> precedence set of having these types of configuration flags,
>> note the enable_nfsv3, enable_nfsv4, so adding a enable_nfsv41
>> seem to me made sense...
>>
>>>
>>> 	- distributions shouldn't be turning on 4.1 by default yet.
>> Its not... If the --enable_nfsv41 is not set, the 4.1 functional is
>> off. Having this type of flag enables the distros to enable the
>> functionality on development kernels but disable it on stable 
> 
> So you want to enable it (for example) in Fedora, but disable it in
> RHEL?
No, its only enabled in Rawhide which is the untested/developmental 
version of the next fedora release (in this case F14). When F14 is ready,
the determination will be made (with solicited input) if the 4.1 code
should continue to be enabled or disabled.
 
This is why I like the configuration switch. There is no code
changes, code is just being disabled. Which, in theory, should have
no ramification at all..    

> 
> I don't think that's a good idea.  If servers running current Fedora
> kernels are still around when the client starts trying 4.1 first, then
> we'll run into trouble.
Since people actually have to specify the -o minorversion=1 mount
option (which is not documented, btw), I'm going to assume those people
know what they are doing and probably will know how to fix any problems 
that may arise... 

steved.

Re: [PATCH][nfs-utils] configuration: Fix a typo in configure.ac

[J. Bruce Fields]

On Mon, Mar 01, 2010 at 08:48:41AM -0500, Steve Dickson wrote:
> On 03/01/2010 08:25 AM, J. Bruce Fields wrote:
> > On Mon, Mar 01, 2010 at 07:57:28AM -0500, Steve Dickson wrote:
> >> On 02/22/2010 01:22 PM, J. Bruce Fields wrote:
> >>> On Sat, Feb 20, 2010 at 03:45:10PM -0800, Trond Myklebust wrote:
> >>>> From: Trond Myklebust <Trond.Myklebust@netapp.com>
> >>>>
> >>>> Fix a typo in commit 6d5ac3fa (nfsd: Disble NFS 4.1 functionality by default).
> >>>
> >>> Better: please just rever 6d5ac3fa and apply
> >>> http://marc.info/?l=linux-nfs&m=126540028610022&w=2
> >> The problem I saw with the this patch was it would take 
> >> a code change to re-enable the 4.1 functional,
> > 
> > Enabling 4.1 will require no code changes to nfs-utils, only to the
> > kernel.  (But it will require a lot of code changes to nfs-utils, as we
> > should complete 4.1 support first.)
> > 
> >> verses a
> >> enabling a configuration flag. Plus, there has been a 
> >> precedence set of having these types of configuration flags,
> >> note the enable_nfsv3, enable_nfsv4, so adding a enable_nfsv41
> >> seem to me made sense...
> >>
> >>>
> >>> 	- distributions shouldn't be turning on 4.1 by default yet.
> >> Its not... If the --enable_nfsv41 is not set, the 4.1 functional is
> >> off. Having this type of flag enables the distros to enable the
> >> functionality on development kernels but disable it on stable 
> > 
> > So you want to enable it (for example) in Fedora, but disable it in
> > RHEL?
> No, its only enabled in Rawhide which is the untested/developmental 
> version of the next fedora release (in this case F14). When F14 is ready,
> the determination will be made (with solicited input) if the 4.1 code
> should continue to be enabled or disabled.

That decision should depend on whether the kernel 4.1 support is
complete or not.  If it is, then the kernel will default 4.1 on, and
there will be no need for an nfs-utils change.

> This is why I like the configuration switch. There is no code
> changes, code is just being disabled. Which, in theory, should have
> no ramification at all..    
> 
> > 
> > I don't think that's a good idea.  If servers running current Fedora
> > kernels are still around when the client starts trying 4.1 first, then
> > we'll run into trouble.
> Since people actually have to specify the -o minorversion=1 mount
> option (which is not documented, btw), I'm going to assume those people
> know what they are doing and probably will know how to fix any problems 
> that may arise... 

No, that's not enough.  They may be accessing the server through other
clients.

We need users to make a conscious decision to turn on the server-side.
It's not enough to do it only on the client side.

--b.

Re: [PATCH][nfs-utils] configuration: Fix a typo in configure.ac

[Steve Dickson]

On 03/01/2010 08:55 AM, J. Bruce Fields wrote:
> On Mon, Mar 01, 2010 at 08:48:41AM -0500, Steve Dickson wrote:
>> On 03/01/2010 08:25 AM, J. Bruce Fields wrote:
>>> On Mon, Mar 01, 2010 at 07:57:28AM -0500, Steve Dickson wrote:
>>>> On 02/22/2010 01:22 PM, J. Bruce Fields wrote:
>>>>> On Sat, Feb 20, 2010 at 03:45:10PM -0800, Trond Myklebust wrote:
>>>>>> From: Trond Myklebust <Trond.Myklebust@netapp.com>
>>>>>>
>>>>>> Fix a typo in commit 6d5ac3fa (nfsd: Disble NFS 4.1 functionality by default).
>>>>>
>>>>> Better: please just rever 6d5ac3fa and apply
>>>>> http://marc.info/?l=linux-nfs&m=126540028610022&w=2
>>>> The problem I saw with the this patch was it would take 
>>>> a code change to re-enable the 4.1 functional,
>>>
>>> Enabling 4.1 will require no code changes to nfs-utils, only to the
>>> kernel.  (But it will require a lot of code changes to nfs-utils, as we
>>> should complete 4.1 support first.)
>>>
>>>> verses a
>>>> enabling a configuration flag. Plus, there has been a 
>>>> precedence set of having these types of configuration flags,
>>>> note the enable_nfsv3, enable_nfsv4, so adding a enable_nfsv41
>>>> seem to me made sense...
>>>>
>>>>>
>>>>> 	- distributions shouldn't be turning on 4.1 by default yet.
>>>> Its not... If the --enable_nfsv41 is not set, the 4.1 functional is
>>>> off. Having this type of flag enables the distros to enable the
>>>> functionality on development kernels but disable it on stable 
>>>
>>> So you want to enable it (for example) in Fedora, but disable it in
>>> RHEL?
>> No, its only enabled in Rawhide which is the untested/developmental 
>> version of the next fedora release (in this case F14). When F14 is ready,
>> the determination will be made (with solicited input) if the 4.1 code
>> should continue to be enabled or disabled.
> 
> That decision should depend on whether the kernel 4.1 support is
> complete or not.  If it is, then the kernel will default 4.1 on, and
> there will be no need for an nfs-utils change.
> 
>> This is why I like the configuration switch. There is no code
>> changes, code is just being disabled. Which, in theory, should have
>> no ramification at all..    
>>
>>>
>>> I don't think that's a good idea.  If servers running current Fedora
>>> kernels are still around when the client starts trying 4.1 first, then
>>> we'll run into trouble.
>> Since people actually have to specify the -o minorversion=1 mount
>> option (which is not documented, btw), I'm going to assume those people
>> know what they are doing and probably will know how to fix any problems 
>> that may arise... 
> 
> No, that's not enough.  They may be accessing the server through other
> clients.
Understood... just curious, would you happen to know of any clients
that use 4.1 as the default? Was there any at this year's Connectathon?

> 
> We need users to make a conscious decision to turn on the server-side.
> It's not enough to do it only on the client side.
Right. At this point people will either have to recompile nfs-utils using
the --enable-nfsv41 flag or edit their initscript and do the echo trick... 
So isn't having to do one of the above be construed as conscious decision?

steved.

Re: [PATCH][nfs-utils] configuration: Fix a typo in configure.ac

[J. Bruce Fields]

On Mon, Mar 01, 2010 at 09:13:17AM -0500, Steve Dickson wrote:
> 
> 
> On 03/01/2010 08:55 AM, J. Bruce Fields wrote:
> > No, that's not enough.  They may be accessing the server through other
> > clients.
> Understood... just curious, would you happen to know of any clients
> that use 4.1 as the default? Was there any at this year's Connectathon?

I don't know what their policies are.

Someone will turn it on by default at some point, and we've no way to
ensure they'll do that at the same point that the linux 4.1 server is
up to spec.

> > We need users to make a conscious decision to turn on the server-side.
> > It's not enough to do it only on the client side.
> Right. At this point people will either have to recompile nfs-utils using
> the --enable-nfsv41 flag or edit their initscript and do the echo trick... 
> So isn't having to do one of the above be construed as conscious decision?

Recompiling with the --enable-nfsv41 flag isn't a good way for a *user*
to request 4.1 support.  It really only makes sense for *distributors*.

--b.

Re: [PATCH][nfs-utils] configuration: Fix a typo in configure.ac

[Steve Dickson]

On 02/20/2010 06:45 PM, Trond Myklebust wrote:
> From: Trond Myklebust <Trond.Myklebust@netapp.com>
> 
> Fix a typo in commit 6d5ac3fa (nfsd: Disble NFS 4.1 functionality by default).
> 
> We did not really intend to make NFSv4.0 support conditional on NFSv4.1
> being enabled.
> 
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
> 
>  configure.ac |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> 
Committed..

steved