Skip to contentSkip to conversation

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

DISCUSSION Joerg Roedel 2 participants 17 messages Jun 7, 2012 – Jun 8, 2012

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

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

#1 lore raw
Quoted 7 lines · 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

Hi,

Can you use mount -v to get more output?

- Bryan

Quoted 15 lines

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

#2 lore raw
Quoted 6 lines · Bryan Schumaker
Quoted 5 lines · Joerg Roedel

Hi,

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

Quoted 6 lines
Quoted 2 lines
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 <[email protected]>
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.

fs/nfs/super.c +2 -0
index ff656c0..bdd6731 100644
@@ -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))
Quoted 24 lines

- Bryan

Quoted 5 lines

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:

Quoted 10 lines

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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#3 lore raw
Quoted 20 lines · Bryan Schumaker

On 06/07/2012 11:12 AM, Bryan Schumaker wrote:
commit cdf66442fab82916fe38f928b4f91815195a294c

Author: Bryan Schumaker <[email protected]>
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.

fs/nfs/super.c +0 -0
index ff656c0..bdd6731 100644

fs/nfs/super.c static int nfs4_validate_mount_data(void *options, · line 2644

Quoted 8 lines
fs/nfs/super.c +2 -0
@@ -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

#4 lore raw
In reply to Bryan Schumaker
Quoted 3 lines · Bryan Schumaker

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

#5 lore raw
In reply to Bryan Schumaker
Quoted 1 line · Bryan Schumaker

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

#6 lore raw
Quoted 7 lines · Joerg Roedel
Quoted 1 line · Bryan Schumaker

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

Quoted 5 lines

Joerg

#7 lore raw
Quoted 5 lines · Bryan Schumaker
Quoted 4 lines · Joerg Roedel
Quoted 1 line · Bryan Schumaker

Can you use mount -v to get more output?

Quoted 7 lines
Quoted 3 lines

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

#8 lore raw
Quoted 14 lines · Joerg Roedel
Quoted 5 lines · Bryan Schumaker
Quoted 4 lines · Joerg Roedel
Quoted 1 line · Bryan Schumaker

Can you use mount -v to get more output?

Quoted 7 lines
Quoted 3 lines

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).

Quoted 6 lines

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.

Quoted 18 lines
# 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

#9 lore raw
Quoted 4 lines · Bryan Schumaker

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)

Quoted 2 lines

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

#10 lore raw
In reply to Bryan Schumaker
Quoted 6 lines · Bryan Schumaker
Quoted 1 line · Joerg Roedel

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 <[email protected]>
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

---
 fs/nfs/super.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
fs/nfs/super.c +3 -2
index ff656c0..2fd4ce5 100644
@@ -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;
Signature · 1 line
1.7.9.5
#11 lore raw
Quoted 8 lines · Joerg Roedel
Quoted 6 lines · Bryan Schumaker
Quoted 1 line · Joerg Roedel

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).

Quoted 5 lines

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

Quoted 58 lines

Regards,

Joerg

From: Joerg Roedel <[email protected]>
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

---
 fs/nfs/super.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
fs/nfs/super.c +3 -2
index ff656c0..2fd4ce5 100644
@@ -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;
#12 lore raw
Quoted 15 lines · Bryan Schumaker
Quoted 8 lines · Joerg Roedel
Quoted 6 lines · Bryan Schumaker
Quoted 1 line · Joerg Roedel

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).

Quoted 5 lines

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.

Quoted 4 lines

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

#13 lore raw
Quoted 21 lines · Joerg Roedel
Quoted 15 lines · Bryan Schumaker
Quoted 8 lines · Joerg Roedel
Quoted 6 lines · Bryan Schumaker
Quoted 1 line · Joerg Roedel

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).

Quoted 5 lines

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.

Quoted 4 lines

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.

Quoted 4 lines

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...

fs/nfs/super.c +1 -0
index bdd6731..906f09c 100644
@@ -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;

--

Quoted 6 lines

Regards,

Joerg

#14 lore raw
Quoted 15 lines · Bryan Schumaker
Quoted 4 lines · Joerg Roedel

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...

fs/nfs/super.c +0 -0
index bdd6731..906f09c 100644

fs/nfs/super.c static int nfs23_validate_mount_data(void *options,

Quoted 8 lines
fs/nfs/super.c +1 -1
@@ -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

#15 lore raw
Quoted 19 lines · Joerg Roedel
Quoted 18 lines · Bryan Schumaker
Quoted 4 lines · Joerg Roedel

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...

fs/nfs/super.c +0 -0
index bdd6731..906f09c 100644
@@ -1867,6 +1867,7 @@ static int nfs23_validate_mount_data(void *options,
        if (data == NULL)
                goto out_no_data;
 
Quoted 9 lines

fs/nfs/super.c static int nfs23_validate_mount_data(void *options,

Quoted 5 lines
fs/nfs/super.c +1 -1
+       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

Quoted 5 lines

Joerg

#16 lore raw
In reply to Joerg Roedel
Quoted 1 line · Joerg Roedel

Patch attached.

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

Quoted 1 line

From 3517158fc3bc5d778dcba987ac73eea6bb7a889a Mon Sep 17 00:00:00 2001

From: Joerg Roedel <[email protected]>
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

---
 fs/nfs/super.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
fs/nfs/super.c +4 -2
index ff656c0..d7a237d 100644
@@ -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;
Signature · 1 line
1.7.9.5