NFS regression in v3.5-rc1: mount.nfs yells about incorrect mount option

NFS regression in v3.5-rc1: mount.nfs yells about incorrect mount option

[Joerg Roedel]

Hi,

since Linux v3.5-rc1 I experience problems using NFS between my
machines. One of my test-machines gives me:

root@chrom:~# mount lemmy:/data/repos /data/lemmy
mount.nfs: an incorrect mount option was specified

The same works fine on v3.4. I tried to bisect but got different errors
in between. In the end, using every non-working case as bad, bisection
pointed to:

	db83335 NFS: Let mount data parsing set the NFS version

but that at that commit the kernel panics when trying to mount the NFS
tree. Please let me know if you need any additional information.

Regards,

	Joerg

Re: NFS regression in v3.5-rc1: mount.nfs yells about incorrect mount option

[Bryan Schumaker]

On 06/07/2012 11:04 AM, Joerg Roedel wrote:
> Hi,
> 
> since Linux v3.5-rc1 I experience problems using NFS between my
> machines. One of my test-machines gives me:
> 
> root@chrom:~# mount lemmy:/data/repos /data/lemmy
> mount.nfs: an incorrect mount option was specified

Hi,

Can you use `mount -v` to get more output?

- Bryan
> 
> The same works fine on v3.4. I tried to bisect but got different errors
> in between. In the end, using every non-working case as bad, bisection
> pointed to:
> 
> 	db83335 NFS: Let mount data parsing set the NFS version
> 
> but that at that commit the kernel panics when trying to mount the NFS
> tree. Please let me know if you need any additional information.
> 
> Regards,
> 
> 	Joerg
> 
> 

Re: NFS regression in v3.5-rc1: mount.nfs yells about incorrect mount option

[Bryan Schumaker]

On 06/07/2012 11:12 AM, Bryan Schumaker wrote:
> On 06/07/2012 11:04 AM, Joerg Roedel wrote:
>> Hi,
>>
>> since Linux v3.5-rc1 I experience problems using NFS between my
>> machines. One of my test-machines gives me:
>>
>> root@chrom:~# mount lemmy:/data/repos /data/lemmy
>> mount.nfs: an incorrect mount option was specified
> 
> Hi,
> 
> Can you use `mount -v` to get more output?

Ah, I'm getting my patch titles confused.  I think I found this bug earlier in the week, does this patch fix it for you?

commit cdf66442fab82916fe38f928b4f91815195a294c
Author: Bryan Schumaker <bjschuma@netapp.com>
Date:   Tue Jun 5 14:59:54 2012 -0400

    NFS4: Set parsed mount data version to 4
    
    This patch only affects mounting through "-t nfs4" since it doesn't set
    up an nfs version to use in the mount data.  The nfs client was trying
    to mount using NFS v0, causing either a BUG() or a protocol not
    supported message.
    
    Signed-off-by: Bryan Schumaker <bjschuma@netapp.com>
    Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index ff656c0..bdd6731 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2637,6 +2637,8 @@ static int nfs4_validate_mount_data(void *options,
 	if (data == NULL)
 		goto out_no_data;
 
+	args->version = 4;
+
 	switch (data->version) {
 	case 1:
 		if (data->host_addrlen > sizeof(args->nfs_server.address))


> 
> - Bryan
>>
>> The same works fine on v3.4. I tried to bisect but got different errors
>> in between. In the end, using every non-working case as bad, bisection
>> pointed to:
>>
>> 	db83335 NFS: Let mount data parsing set the NFS version
>>
>> but that at that commit the kernel panics when trying to mount the NFS
>> tree. Please let me know if you need any additional information.
>>
>> Regards,
>>
>> 	Joerg
>>
>>
> 
> 
> --
> 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: NFS regression in v3.5-rc1: mount.nfs yells about incorrect mount option

[Joerg Roedel]

On Thu, Jun 07, 2012 at 11:12:02AM -0400, Bryan Schumaker wrote:
> On 06/07/2012 11:04 AM, Joerg Roedel wrote:
> 
> Can you use `mount -v` to get more output?

Hmm, that is weird. When I use mount -v it actually does mount the
remote NFS tree. Here is the output:

root@chrom:~# mount -v lemmy:/data/repos /data/lemmy/
mount: no type was given - I'll assume nfs because of the colon
mount.nfs: timeout set for Thu Jun  7 17:21:54 2012
mount.nfs: text-based options: 'addr=165.204.15.93'
mount.nfs: mount(2): Protocol not supported
mount.nfs: trying 165.204.15.93 prog 100003 vers 3 prot UDP port 2049
mount.nfs: trying 165.204.15.93 prog 100005 vers 3 prot UDP port 39332
mount.nfs: text-based options (retry): 'addr=165.204.15.93,vers=3,proto=udp,mountvers=3,mountproto=udp,mountport=39332'
lemmy:/data/repos on /data/lemmy type nfs (rw)

When I unmount and try to mount it again without -v I still get the
error message and the mount fails.

With 3.4 the mount -v output is:

mount: no type was given - I'll assume nfs because of the colon
mount.nfs: timeout set for Thu Jun  7 17:19:44 2012
mount.nfs: text-based options: 'addr=165.204.15.93'
lemmy:/data/repos/ on /data/lemmy type nfs (rw)

So for some reason the text-based options are different. Maybe that has
something to do with the problem.

Regards,

	Joerg

Re: NFS regression in v3.5-rc1: mount.nfs yells about incorrect mount option

[Joerg Roedel]

On Thu, Jun 07, 2012 at 11:17:39AM -0400, Bryan Schumaker wrote:
> On 06/07/2012 11:12 AM, Bryan Schumaker wrote:
> commit cdf66442fab82916fe38f928b4f91815195a294c
> Author: Bryan Schumaker <bjschuma@netapp.com>
> Date:   Tue Jun 5 14:59:54 2012 -0400
> 
>     NFS4: Set parsed mount data version to 4
>     
>     This patch only affects mounting through "-t nfs4" since it doesn't set
>     up an nfs version to use in the mount data.  The nfs client was trying
>     to mount using NFS v0, causing either a BUG() or a protocol not
>     supported message.
>     
>     Signed-off-by: Bryan Schumaker <bjschuma@netapp.com>
>     Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index ff656c0..bdd6731 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2637,6 +2637,8 @@ static int nfs4_validate_mount_data(void *options,
>  	if (data == NULL)
>  		goto out_no_data;
>  
> +	args->version = 4;
> +
>  	switch (data->version) {
>  	case 1:
>  		if (data->host_addrlen > sizeof(args->nfs_server.address))

No, this doesn't fix it for me. Same behavior as with plain v3.5-rc1.


	Joerg

Re: NFS regression in v3.5-rc1: mount.nfs yells about incorrect mount option

[Joerg Roedel]

On Thu, Jun 07, 2012 at 11:12:02AM -0400, Bryan Schumaker wrote:

> Can you use `mount -v` to get more output?

Btw, it also works when I manually set the nfs-version at mount:

mount -o nfsvers=3 ...


	Joerg

Re: NFS regression in v3.5-rc1: mount.nfs yells about incorrect mount option

[Bryan Schumaker]

On 06/07/2012 11:50 AM, Joerg Roedel wrote:
> On Thu, Jun 07, 2012 at 11:12:02AM -0400, Bryan Schumaker wrote:
> 
>> Can you use `mount -v` to get more output?
> 
> Btw, it also works when I manually set the nfs-version at mount:
> 
> mount -o nfsvers=3 ...

What about `mount -o vers=4 ...`?  I'm compiling a kernel right now to see if I can reproduce this, what NFS .config options do you have set? (`cat .config | grep CONFIG_NFS_` should be good enough).

- Bryan
> 
> 
> 	Joerg
> 
> 

Re: NFS regression in v3.5-rc1: mount.nfs yells about incorrect mount option

[Joerg Roedel]

On Thu, Jun 07, 2012 at 11:54:24AM -0400, Bryan Schumaker wrote:
> On 06/07/2012 11:50 AM, Joerg Roedel wrote:
> > On Thu, Jun 07, 2012 at 11:12:02AM -0400, Bryan Schumaker wrote:
> > 
> >> Can you use `mount -v` to get more output?
> > 
> > Btw, it also works when I manually set the nfs-version at mount:
> > 
> > mount -o nfsvers=3 ...
> 
> What about `mount -o vers=4 ...`?  I'm compiling a kernel right now to
> see if I can reproduce this, what NFS .config options do you have set?
> (`cat .config | grep CONFIG_NFS_` should be good enough).

mount -o vers=2 and -o vers=3 works
mount -o vers=4 fails with:

root@chrom:~# mount -o vers=4 lemmy:/data/repos/ /data/lemmy/
mount.nfs: mounting lemmy:/data/repos/ failed, reason given by server:
  No such file or directory

# grep CONFIG_NFS_ .config
CONFIG_NFS_FS=y
CONFIG_NFS_V2=y
CONFIG_NFS_V3=y
CONFIG_NFS_V3_ACL=y
CONFIG_NFS_V4=y
# CONFIG_NFS_V4_1 is not set
CONFIG_NFS_FSCACHE=y
# CONFIG_NFS_USE_LEGACY_DNS is not set
CONFIG_NFS_USE_KERNEL_DNS=y
CONFIG_NFS_ACL_SUPPORT=y
CONFIG_NFS_COMMON=y


	Joerg

Re: NFS regression in v3.5-rc1: mount.nfs yells about incorrect mount option

[Bryan Schumaker]

On 06/07/2012 12:01 PM, Joerg Roedel wrote:
> On Thu, Jun 07, 2012 at 11:54:24AM -0400, Bryan Schumaker wrote:
>> On 06/07/2012 11:50 AM, Joerg Roedel wrote:
>>> On Thu, Jun 07, 2012 at 11:12:02AM -0400, Bryan Schumaker wrote:
>>>
>>>> Can you use `mount -v` to get more output?
>>>
>>> Btw, it also works when I manually set the nfs-version at mount:
>>>
>>> mount -o nfsvers=3 ...
>>
>> What about `mount -o vers=4 ...`?  I'm compiling a kernel right now to
>> see if I can reproduce this, what NFS .config options do you have set?
>> (`cat .config | grep CONFIG_NFS_` should be good enough).
> 
> mount -o vers=2 and -o vers=3 works
> mount -o vers=4 fails with:
> 
> root@chrom:~# mount -o vers=4 lemmy:/data/repos/ /data/lemmy/
> mount.nfs: mounting lemmy:/data/repos/ failed, reason given by server:
>   No such file or directory

What does /etc/exports look like on the server?  You may have to mount it as "lemmy:/" when using NFS v4.

What distro are you using?  I haven't been able to get the incorrect mount option error you're seeing.
> 
> # grep CONFIG_NFS_ .config
> CONFIG_NFS_FS=y
> CONFIG_NFS_V2=y
> CONFIG_NFS_V3=y
> CONFIG_NFS_V3_ACL=y
> CONFIG_NFS_V4=y
> # CONFIG_NFS_V4_1 is not set
> CONFIG_NFS_FSCACHE=y
> # CONFIG_NFS_USE_LEGACY_DNS is not set
> CONFIG_NFS_USE_KERNEL_DNS=y
> CONFIG_NFS_ACL_SUPPORT=y
> CONFIG_NFS_COMMON=y
> 
> 
> 	Joerg
> 
> 

Re: NFS regression in v3.5-rc1: mount.nfs yells about incorrect mount option

[Joerg Roedel]

On Thu, Jun 07, 2012 at 01:23:14PM -0400, Bryan Schumaker wrote:
> On 06/07/2012 12:01 PM, Joerg Roedel wrote:

> What does /etc/exports look like on the server?  You may have to mount
> it as "lemmy:/" when using NFS v4.

It does not use NFSv4. The exact same command line works fine on v3.4 :)
The /etc/exports line is:

/data/repos	*(ro,no_subtree_check)

> 
> What distro are you using?  I haven't been able to get the incorrect mount option error you're seeing.

The client is running Ubuntu 10.04 amd64 desktop with development kernel
and the "server" is my workstation, running Ubuntu 12.04 amd64 desktop
with distro kernel (3.2 based).

Regards,

	Joerg

Re: NFS regression in v3.5-rc1: mount.nfs yells about incorrect mount option

[Joerg Roedel]

On Thu, Jun 07, 2012 at 11:54:24AM -0400, Bryan Schumaker wrote:
> On 06/07/2012 11:50 AM, Joerg Roedel wrote:
> > mount -o nfsvers=3 ...
> 
> What about `mount -o vers=4 ...`?  I'm compiling a kernel right now to
> see if I can reproduce this, what NFS .config options do you have set?
> (`cat .config | grep CONFIG_NFS_` should be good enough).

Okay, I tracked it down somewhat. The problem is that the nfs-version
is not set in my case so that data->version in nfs_init_server is 0. The
function returns -EPROTONOSUPPORT in this case which causes the mount
to fail. The evil commit is db8333519 and reverting it fixes the issue
for me. Patch attached.

Regards,

	Joerg

From: Joerg Roedel <joerg.roedel@amd.com>
Date: Fri, 8 Jun 2012 14:51:01 +0200
Subject: [PATCH] Revert "NFS: Let mount data parsing set the NFS version"

This reverts commit db8333519187d5974cf2ff33910c893bf8727d9f.

The original commit message says, that the version field is
"unconditionally set while parsing mount data".
Unfortunatly this is not the case and the version field may
stay 0 so that nfs_init_server returns -EPROTONOSUPPORT
causing the mount to fail.  Reverting this commit fixes the
issue.

Conflicts:

	fs/nfs/super.c

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 fs/nfs/super.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index ff656c0..2fd4ce5 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -948,7 +948,7 @@ static void nfs_umount_begin(struct super_block *sb)
 		rpc_killall_tasks(rpc);
 }
 
-static struct nfs_parsed_mount_data *nfs_alloc_parsed_mount_data(void)
+static struct nfs_parsed_mount_data *nfs_alloc_parsed_mount_data(unsigned int version)
 {
 	struct nfs_parsed_mount_data *data;
 
@@ -963,6 +963,7 @@ static struct nfs_parsed_mount_data *nfs_alloc_parsed_mount_data(void)
 		data->nfs_server.protocol = XPRT_TRANSPORT_TCP;
 		data->auth_flavors[0]	= RPC_AUTH_UNIX;
 		data->auth_flavor_len	= 1;
+		data->version		= version;
 		data->minorversion	= 0;
 		data->need_mount	= true;
 		data->net		= current->nsproxy->net_ns;
@@ -2479,7 +2480,7 @@ static struct dentry *nfs_fs_mount(struct file_system_type *fs_type,
 	struct dentry *mntroot = ERR_PTR(-ENOMEM);
 	int error;
 
-	mount_info.parsed = nfs_alloc_parsed_mount_data();
+	mount_info.parsed = nfs_alloc_parsed_mount_data(NFS_DEFAULT_VERSION);
 	mount_info.mntfh = nfs_alloc_fhandle();
 	if (mount_info.parsed == NULL || mount_info.mntfh == NULL)
 		goto out;
-- 
1.7.9.5

Re: NFS regression in v3.5-rc1: mount.nfs yells about incorrect mount option

[Bryan Schumaker]

On 06/08/2012 09:03 AM, Joerg Roedel wrote:
> On Thu, Jun 07, 2012 at 11:54:24AM -0400, Bryan Schumaker wrote:
>> On 06/07/2012 11:50 AM, Joerg Roedel wrote:
>>> mount -o nfsvers=3 ...
>>
>> What about `mount -o vers=4 ...`?  I'm compiling a kernel right now to
>> see if I can reproduce this, what NFS .config options do you have set?
>> (`cat .config | grep CONFIG_NFS_` should be good enough).
> 
> Okay, I tracked it down somewhat. The problem is that the nfs-version
> is not set in my case so that data->version in nfs_init_server is 0. The
> function returns -EPROTONOSUPPORT in this case which causes the mount
> to fail. The evil commit is db8333519 and reverting it fixes the issue
> for me. Patch attached.

Thanks!  I wasn't able to reproduce this on Ubuntu 12.04, so now I'm setting up 10.04 to see if that makes a difference.  I'd like to understand what's going on (and why my other patch didn't fix this problem) before reverting.

- Bryan
> 
> Regards,
> 
> 	Joerg
> 
> From: Joerg Roedel <joerg.roedel@amd.com>
> Date: Fri, 8 Jun 2012 14:51:01 +0200
> Subject: [PATCH] Revert "NFS: Let mount data parsing set the NFS version"
> 
> This reverts commit db8333519187d5974cf2ff33910c893bf8727d9f.
> 
> The original commit message says, that the version field is
> "unconditionally set while parsing mount data".
> Unfortunatly this is not the case and the version field may
> stay 0 so that nfs_init_server returns -EPROTONOSUPPORT
> causing the mount to fail.  Reverting this commit fixes the
> issue.
> 
> Conflicts:
> 
> 	fs/nfs/super.c
> 
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  fs/nfs/super.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index ff656c0..2fd4ce5 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -948,7 +948,7 @@ static void nfs_umount_begin(struct super_block *sb)
>  		rpc_killall_tasks(rpc);
>  }
>  
> -static struct nfs_parsed_mount_data *nfs_alloc_parsed_mount_data(void)
> +static struct nfs_parsed_mount_data *nfs_alloc_parsed_mount_data(unsigned int version)
>  {
>  	struct nfs_parsed_mount_data *data;
>  
> @@ -963,6 +963,7 @@ static struct nfs_parsed_mount_data *nfs_alloc_parsed_mount_data(void)
>  		data->nfs_server.protocol = XPRT_TRANSPORT_TCP;
>  		data->auth_flavors[0]	= RPC_AUTH_UNIX;
>  		data->auth_flavor_len	= 1;
> +		data->version		= version;
>  		data->minorversion	= 0;
>  		data->need_mount	= true;
>  		data->net		= current->nsproxy->net_ns;
> @@ -2479,7 +2480,7 @@ static struct dentry *nfs_fs_mount(struct file_system_type *fs_type,
>  	struct dentry *mntroot = ERR_PTR(-ENOMEM);
>  	int error;
>  
> -	mount_info.parsed = nfs_alloc_parsed_mount_data();
> +	mount_info.parsed = nfs_alloc_parsed_mount_data(NFS_DEFAULT_VERSION);
>  	mount_info.mntfh = nfs_alloc_fhandle();
>  	if (mount_info.parsed == NULL || mount_info.mntfh == NULL)
>  		goto out;
> 

Re: NFS regression in v3.5-rc1: mount.nfs yells about incorrect mount option

[Joerg Roedel]

On Fri, Jun 08, 2012 at 03:03:52PM +0200, Joerg Roedel wrote:
> Patch attached.

Hmm, since nfs_fs_mount is now shared between nfs23 and nfs4, a better
"revert" is probably this one:

>From 3517158fc3bc5d778dcba987ac73eea6bb7a889a Mon Sep 17 00:00:00 2001
From: Joerg Roedel <joerg.roedel@amd.com>
Date: Fri, 8 Jun 2012 14:51:01 +0200
Subject: [PATCH] Revert "NFS: Let mount data parsing set the NFS version"

This reverts commit db8333519187d5974cf2ff33910c893bf8727d9f.

The original commit message says, that the version field is
"unconditionally set while parsing mount data".
Unfortunatly this is not the case and the version field may
stay 0 so that nfs_init_server returns -EPROTONOSUPPORT
causing the mount to fail.  Reverting this commit fixes the
issue.

Conflicts:

	fs/nfs/super.c

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 fs/nfs/super.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index ff656c0..d7a237d 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -948,7 +948,7 @@ static void nfs_umount_begin(struct super_block *sb)
 		rpc_killall_tasks(rpc);
 }
 
-static struct nfs_parsed_mount_data *nfs_alloc_parsed_mount_data(void)
+static struct nfs_parsed_mount_data *nfs_alloc_parsed_mount_data(unsigned int version)
 {
 	struct nfs_parsed_mount_data *data;
 
@@ -963,6 +963,7 @@ static struct nfs_parsed_mount_data *nfs_alloc_parsed_mount_data(void)
 		data->nfs_server.protocol = XPRT_TRANSPORT_TCP;
 		data->auth_flavors[0]	= RPC_AUTH_UNIX;
 		data->auth_flavor_len	= 1;
+		data->version		= version;
 		data->minorversion	= 0;
 		data->need_mount	= true;
 		data->net		= current->nsproxy->net_ns;
@@ -2472,6 +2473,7 @@ error_splat_bdi:
 static struct dentry *nfs_fs_mount(struct file_system_type *fs_type,
 	int flags, const char *dev_name, void *raw_data)
 {
+	int version = (fs_type == &nfs_fs_type) ? NFS_DEFAULT_VERSION : 4;
 	struct nfs_mount_info mount_info = {
 		.fill_super = nfs_fill_super,
 		.set_security = nfs_set_sb_security,
@@ -2479,7 +2481,7 @@ static struct dentry *nfs_fs_mount(struct file_system_type *fs_type,
 	struct dentry *mntroot = ERR_PTR(-ENOMEM);
 	int error;
 
-	mount_info.parsed = nfs_alloc_parsed_mount_data();
+	mount_info.parsed = nfs_alloc_parsed_mount_data(version);
 	mount_info.mntfh = nfs_alloc_fhandle();
 	if (mount_info.parsed == NULL || mount_info.mntfh == NULL)
 		goto out;
-- 
1.7.9.5

Re: NFS regression in v3.5-rc1: mount.nfs yells about incorrect mount option

[Joerg Roedel]

On Fri, Jun 08, 2012 at 09:20:02AM -0400, Bryan Schumaker wrote:
> On 06/08/2012 09:03 AM, Joerg Roedel wrote:
> > On Thu, Jun 07, 2012 at 11:54:24AM -0400, Bryan Schumaker wrote:
> >> On 06/07/2012 11:50 AM, Joerg Roedel wrote:
> >>> mount -o nfsvers=3 ...
> >>
> >> What about `mount -o vers=4 ...`?  I'm compiling a kernel right now to
> >> see if I can reproduce this, what NFS .config options do you have set?
> >> (`cat .config | grep CONFIG_NFS_` should be good enough).
> > 
> > Okay, I tracked it down somewhat. The problem is that the nfs-version
> > is not set in my case so that data->version in nfs_init_server is 0. The
> > function returns -EPROTONOSUPPORT in this case which causes the mount
> > to fail. The evil commit is db8333519 and reverting it fixes the issue
> > for me. Patch attached.
> 
> Thanks!  I wasn't able to reproduce this on Ubuntu 12.04, so now I'm
> setting up 10.04 to see if that makes a difference.  I'd like to
> understand what's going on (and why my other patch didn't fix this
> problem) before reverting.

Your other patch only touched the nfs4 path, but in my setup nfs3 was in
use. Therefore the patch didn't help. I just figured out that
nfs_fs_mount is shared between nfs23 and nfs4, so the first patch
probably breaks nfs4. I send another one which takes this into account.

Regards,

	Joerg

Re: NFS regression in v3.5-rc1: mount.nfs yells about incorrect mount option

[Bryan Schumaker]

On 06/08/2012 09:33 AM, Joerg Roedel wrote:
> On Fri, Jun 08, 2012 at 09:20:02AM -0400, Bryan Schumaker wrote:
>> On 06/08/2012 09:03 AM, Joerg Roedel wrote:
>>> On Thu, Jun 07, 2012 at 11:54:24AM -0400, Bryan Schumaker wrote:
>>>> On 06/07/2012 11:50 AM, Joerg Roedel wrote:
>>>>> mount -o nfsvers=3 ...
>>>>
>>>> What about `mount -o vers=4 ...`?  I'm compiling a kernel right now to
>>>> see if I can reproduce this, what NFS .config options do you have set?
>>>> (`cat .config | grep CONFIG_NFS_` should be good enough).
>>>
>>> Okay, I tracked it down somewhat. The problem is that the nfs-version
>>> is not set in my case so that data->version in nfs_init_server is 0. The
>>> function returns -EPROTONOSUPPORT in this case which causes the mount
>>> to fail. The evil commit is db8333519 and reverting it fixes the issue
>>> for me. Patch attached.
>>
>> Thanks!  I wasn't able to reproduce this on Ubuntu 12.04, so now I'm
>> setting up 10.04 to see if that makes a difference.  I'd like to
>> understand what's going on (and why my other patch didn't fix this
>> problem) before reverting.
> 
> Your other patch only touched the nfs4 path, but in my setup nfs3 was in
> use. Therefore the patch didn't help. I just figured out that
> nfs_fs_mount is shared between nfs23 and nfs4, so the first patch
> probably breaks nfs4. I send another one which takes this into account.

Would something like this work? (I haven't tried it yet).  Setting it in nfs_alloc_parsed_mount_data() might work too...

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index bdd6731..906f09c 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1867,6 +1867,7 @@ static int nfs23_validate_mount_data(void *options,
        if (data == NULL)
                goto out_no_data;
 
+       args->version = NFS_DEFAULT_VERSION;
        switch (data->version) {
        case 1:
                data->namlen = 0;
--


> 
> Regards,
> 
> 	Joerg
> 
> 

Re: NFS regression in v3.5-rc1: mount.nfs yells about incorrect mount option

[Joerg Roedel]

On Fri, Jun 08, 2012 at 09:37:01AM -0400, Bryan Schumaker wrote:
> On 06/08/2012 09:33 AM, Joerg Roedel wrote:

> > Your other patch only touched the nfs4 path, but in my setup nfs3 was in
> > use. Therefore the patch didn't help. I just figured out that
> > nfs_fs_mount is shared between nfs23 and nfs4, so the first patch
> > probably breaks nfs4. I send another one which takes this into account.
> 
> Would something like this work? (I haven't tried it yet).  Setting it
> in nfs_alloc_parsed_mount_data() might work too...
> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index bdd6731..906f09c 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -1867,6 +1867,7 @@ static int nfs23_validate_mount_data(void *options,
>         if (data == NULL)
>                 goto out_no_data;
>  
> +       args->version = NFS_DEFAULT_VERSION;
>         switch (data->version) {
>         case 1:
>                 data->namlen = 0;
> --

Yes, this works too (tested). Doing it in nfs_alloc_parsed_mount_data()
need to take into account that the function is also called for nfs4.
This is basically what my second revert-patch does.


	Joerg

Re: NFS regression in v3.5-rc1: mount.nfs yells about incorrect mount option

[Bryan Schumaker]

On 06/08/2012 09:49 AM, Joerg Roedel wrote:
> On Fri, Jun 08, 2012 at 09:37:01AM -0400, Bryan Schumaker wrote:
>> On 06/08/2012 09:33 AM, Joerg Roedel wrote:
> 
>>> Your other patch only touched the nfs4 path, but in my setup nfs3 was in
>>> use. Therefore the patch didn't help. I just figured out that
>>> nfs_fs_mount is shared between nfs23 and nfs4, so the first patch
>>> probably breaks nfs4. I send another one which takes this into account.
>>
>> Would something like this work? (I haven't tried it yet).  Setting it
>> in nfs_alloc_parsed_mount_data() might work too...
>>
>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>> index bdd6731..906f09c 100644
>> --- a/fs/nfs/super.c
>> +++ b/fs/nfs/super.c
>> @@ -1867,6 +1867,7 @@ static int nfs23_validate_mount_data(void *options,
>>         if (data == NULL)
>>                 goto out_no_data;
>>  
>> +       args->version = NFS_DEFAULT_VERSION;
>>         switch (data->version) {
>>         case 1:
>>                 data->namlen = 0;
>> --
> 
> Yes, this works too (tested). Doing it in nfs_alloc_parsed_mount_data()
> need to take into account that the function is also called for nfs4.
> This is basically what my second revert-patch does.

Thanks for testing!  The patch I sent you yesterday sets up "args->version = 4" if it gets into the v4 parsing code, so the v4 case should already be taken care of.

- Bryan
> 
> 
> 	Joerg
> 
>