Bug#669314: NFS: kernel forces trailing slash for export in /proc/self/mounts

Bug#669314: NFS: kernel forces trailing slash for export in /proc/self/mounts

[Chris Hiestand]

Hi Alexander Viro et al,

This is an escalation of Debian Bug #669314 http://bugs.debian.org/669314, which I will
re-elaborate in this email for your convenience.

You committed a change to the way the linux kernel reports NFS mounts - now with a
trailing slash for the remote export (among other changes). The change happened here:
> commit c7f404b40a3665d9f4e9a927cc5c1ee0479ed8f9
> Author: Al Viro <viro@zeniv.linux.org.uk>
> Date:   Wed Mar 16 06:59:40 2011 -0400
> 
>     vfs: new superblock methods to override /proc/*/mount{s,info}
>     
>     a) ->show_devname(m, mnt) - what to put into devname columns in mounts,
>     mountinfo and mountstats
>     b) ->show_path(m, mnt) - what to put into relative path column in mountinfo
>     
>     Leaving those NULL gives old behaviour.  NFS switched to using those.
>     
>     Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> 

The "problematic" behavior is that NFS exports now have a trailing slash in
/proc/self/mounts.

This still seems to be the case in newer kernels, for example in Debian's
3.3.2-1~experimental.1.

and HEAD in Linus Torvalds' master branch, presently commit:
02edf6abe01610a5fb379df442de3c837ad99467


I believe it is/was convention to leave a trailing slash off of the nfs export
in /etc/fstab, e.g.:
nfsserver:/srv/ubuntu-32 /mnt/ubuntu-32  nfs ro,nfsvers=3,soft,intr,tcp,nodev,noatime,nosuid,rsize=32768,wsize=32768

and I'd also expect the same in /proc/self/mounts


Expected Result:
I expect /proc/self/mounts to show (notice no trailing slash on the export):
nfsserver:/srv/ubuntu-32 /mnt/ubuntu-32 nfs ro,nosuid,nodev,noatime,vers=3,rsize=32768,wsize=32768,namlen=255,soft,proto=tcp,timeo=600,retrans=2,sec=sys,mountaddr=198.202.1.1,mountvers=3,mountport=41576,mountproto=tcp,local_lock=none,addr=198.202.1.1 0 0


Actual Result:
But instead in /proc/self/mounts I get (notice the trailing slash):
nfsserver:/srv/ubuntu-32/ /mnt/ubuntu-32 nfs ro,nosuid,nodev,noatime,vers=3,rsize=32768,wsize=32768,namlen=255,soft,proto=tcp,timeo=600,retrans=2,sec=sys,mountaddr=198.202.1.1,mountvers=3,mountport=41576,mountproto=tcp,local_lock=none,addr=198.202.1.1 0 0


Rammifications:
This simple change has a lot of implications because lots of things parse
/etc/fstab and /proc/self/mounts, for example system configuration tools
and mount.nfs and umount.nfs.

If you use the former convention and try to umount an export on a newer
kernel it will fail:

user@hostname:/proc/self$ sudo umount.nfs nfsserver:/srv/ubuntu-32
umount.nfs: nfsserver:/srv/ubuntu-32: not found

And if you run "sudo mount -va", mount will not recognize that the fstab mounts
have already been mounted; mounting all mounts twice on the same mount point.
This quickly gets messy.

If there is a new convention to display the trailing slash, we need to update
our tools to handle this change. If there is not a new convention, I'd argue
this is a bug.

So is this a new convention or not? What is the appropriate way for Debian to move forward?

Thanks,
Chris Hiestand

Re: NFS: kernel forces trailing slash for export in /proc/self/mounts

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 5907 bytes --]

On Tue, 2012-06-19 at 12:43 -0700, Chris Hiestand wrote:
> Hi Alexander Viro et al,
> 
> This is an escalation of Debian Bug #669314 http://bugs.debian.org/669314, which I will
> re-elaborate in this email for your convenience.
> 
> You committed a change to the way the linux kernel reports NFS mounts - now with a
> trailing slash for the remote export (among other changes). The change happened here:
> > commit c7f404b40a3665d9f4e9a927cc5c1ee0479ed8f9
> > Author: Al Viro <viro@zeniv.linux.org.uk>
> > Date:   Wed Mar 16 06:59:40 2011 -0400
> > 
> >     vfs: new superblock methods to override /proc/*/mount{s,info}
> >     
> >     a) ->show_devname(m, mnt) - what to put into devname columns in mounts,
> >     mountinfo and mountstats
> >     b) ->show_path(m, mnt) - what to put into relative path column in mountinfo
> >     
> >     Leaving those NULL gives old behaviour.  NFS switched to using those.
> >     
> >     Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> > 
> 
> The "problematic" behavior is that NFS exports now have a trailing slash in
> /proc/self/mounts.
[...]
> If there is a new convention to display the trailing slash, we need to update
> our tools to handle this change. If there is not a new convention, I'd argue
> this is a bug.
> 
> So is this a new convention or not? What is the appropriate way for
> Debian to move forward?

Al, Trond, what's going on here?  This sure looks like it broke
userland, in which case we need to revert the change in behaviour.

How about the following (untested) change?

Ben.
---
Subject: nfs: Show original device name verbatim in /proc/*/mount{s,info}

Since commit c7f404b ('vfs: new superblock methods to override
/proc/*/mount{s,info}'), nfs_path() is used to generate the mounted
device name reported back to userland.

nfs_path() always generates a trailing slash when the given dentry is
the root of an NFS mount, but userland may expect the original device
name to be returned verbatim (as it used to be).  Make this
canonicalisation optional and change the callers accordingly.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Cc: <stable@vger.kernel.org> # v2.6.39+
---
 fs/nfs/internal.h      |    4 ++--
 fs/nfs/namespace.c     |   15 ++++++++++-----
 fs/nfs/nfs4namespace.c |    2 +-
 fs/nfs/super.c         |    2 +-
 4 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index d554438..c38224a 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -354,7 +354,7 @@ extern void nfs_sb_deactive(struct super_block *sb);
 
 /* namespace.c */
 extern char *nfs_path(char **p, struct dentry *dentry,
-		      char *buffer, ssize_t buflen);
+		      char *buffer, ssize_t buflen, bool canonical);
 extern struct vfsmount *nfs_d_automount(struct path *path);
 struct vfsmount *nfs_submount(struct nfs_server *, struct dentry *,
 			      struct nfs_fh *, struct nfs_fattr *);
@@ -491,7 +491,7 @@ static inline char *nfs_devname(struct dentry *dentry,
 				char *buffer, ssize_t buflen)
 {
 	char *dummy;
-	return nfs_path(&dummy, dentry, buffer, buflen);
+	return nfs_path(&dummy, dentry, buffer, buflen, true);
 }
 
 /*
diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
index 6559253..059975e 100644
--- a/fs/nfs/namespace.c
+++ b/fs/nfs/namespace.c
@@ -33,6 +33,8 @@ int nfs_mountpoint_expiry_timeout = 500 * HZ;
  * @dentry - pointer to dentry
  * @buffer - result buffer
  * @buflen - length of buffer
+ * @canonical - ensure there is exactly one slash after the original
+ *		device (export) name; if false, return it verbatim
  *
  * Helper function for constructing the server pathname
  * by arbitrary hashed dentry.
@@ -41,7 +43,8 @@ int nfs_mountpoint_expiry_timeout = 500 * HZ;
  * server side when automounting on top of an existing partition
  * and in generating /proc/mounts and friends.
  */
-char *nfs_path(char **p, struct dentry *dentry, char *buffer, ssize_t buflen)
+char *nfs_path(char **p, struct dentry *dentry, char *buffer, ssize_t buflen,
+	       bool canonical)
 {
 	char *end;
 	int namelen;
@@ -74,7 +77,7 @@ rename_retry:
 		rcu_read_unlock();
 		goto rename_retry;
 	}
-	if (*end != '/') {
+	if (canonical && *end != '/') {
 		if (--buflen < 0) {
 			spin_unlock(&dentry->d_lock);
 			rcu_read_unlock();
@@ -91,9 +94,11 @@ rename_retry:
 		return end;
 	}
 	namelen = strlen(base);
-	/* Strip off excess slashes in base string */
-	while (namelen > 0 && base[namelen - 1] == '/')
-		namelen--;
+	if (canonical) {
+		/* Strip off excess slashes in base string */
+		while (namelen > 0 && base[namelen - 1] == '/')
+			namelen--;
+	}
 	buflen -= namelen;
 	if (buflen < 0) {
 		spin_unlock(&dentry->d_lock);
diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
index 017b4b0..94e8652 100644
--- a/fs/nfs/nfs4namespace.c
+++ b/fs/nfs/nfs4namespace.c
@@ -81,7 +81,7 @@ static char *nfs_path_component(const char *nfspath, const char *end)
 static char *nfs4_path(struct dentry *dentry, char *buffer, ssize_t buflen)
 {
 	char *limit;
-	char *path = nfs_path(&limit, dentry, buffer, buflen);
+	char *path = nfs_path(&limit, dentry, buffer, buflen, true);
 	if (!IS_ERR(path)) {
 		char *path_component = nfs_path_component(path, limit);
 		if (path_component)
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index d2c7f5d..630a1e2 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -765,7 +765,7 @@ int nfs_show_devname(struct seq_file *m, struct dentry *root)
 	int err = 0;
 	if (!page)
 		return -ENOMEM;
-	devname = nfs_path(&dummy, root, page, PAGE_SIZE);
+	devname = nfs_path(&dummy, root, page, PAGE_SIZE, false);
 	if (IS_ERR(devname))
 		err = PTR_ERR(devname);
 	else

-- 
Ben Hutchings
Experience is what causes a person to make new mistakes instead of old ones.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Bug#669314: NFS: kernel forces trailing slash for export in /proc/self/mounts

[Chris Hiestand]

[-- Attachment #1.1: Type: text/plain, Size: 255 bytes --]

I had a couple friends over today and we made a trivial patch to remove trailing slashes. We do not know C and have never created a patch for the kernel before, so there is undoubtedly a better way to do it. However we hope this helps in your efforts.


[-- Attachment #1.2: 0001-Fixes-trailing-slash-in-nfs-devname.patch --]
[-- Type: application/octet-stream, Size: 851 bytes --]

From 81baa04c07107f81c56c4f163da2178b01294717 Mon Sep 17 00:00:00 2001
From: Chris Hiestand <chiestand@salk.edu>
Date: Sat, 15 Sep 2012 18:40:45 -0700
Subject: [PATCH] Fixes trailing slash in nfs devname

---
 fs/nfs/super.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 79bc61f..e300a84 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -742,8 +742,12 @@ static int nfs_show_devname(struct seq_file *m, struct vfsmount *mnt)
 	devname = nfs_path(&dummy, mnt->mnt_root, page, PAGE_SIZE);
 	if (IS_ERR(devname))
 		err = PTR_ERR(devname);
-	else
+	else {
+		size_t len = strlen(devname);
+		if (devname[len-1] == '/')
+			devname[len-1] = '\0';
 		seq_escape(m, devname, " \t\n\\");
+	}
 	free_page((unsigned long)page);
 	return err;
 }
-- 
1.7.10.4


[-- Attachment #1.3: Type: text/plain, Size: 166 bytes --]


This patch was created from the offending commit (c7f404b40a366). But I've also applied it to to Linus Torvalds' master HEAD (3f0c3c8fe30c7) with success.

-Chris

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4798 bytes --]

Re: Bug#669314: NFS: kernel forces trailing slash for export in /proc/self/mounts

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 764 bytes --]

On Sun, 2012-09-16 at 03:24 -0700, Chris Hiestand wrote:
> I had a couple friends over today and we made a trivial patch to
> remove trailing slashes. We do not know C and have never created a
> patch for the kernel before, so there is undoubtedly a better way to
> do it. However we hope this helps in your efforts.
> 
> This patch was created from the offending commit (c7f404b40a366). But
> I've also applied it to to Linus Torvalds' master HEAD (3f0c3c8fe30c7)
> with success.

This was my first thought - but what if userland provides a device name
with a slash on the end?  I think we have to report it back with the
slash in that case.

Ben.

-- 
Ben Hutchings
Experience is what causes a person to make new mistakes instead of old ones.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: Bug#669314: NFS: kernel forces trailing slash for export in /proc/self/mounts

[Chris Hiestand]

[-- Attachment #1: Type: text/plain, Size: 676 bytes --]


On Sep 16, 2012, at 7:00 AM, Ben Hutchings wrote:

> This was my first thought - but what if userland provides a device name
> with a slash on the end?  I think we have to report it back with the
> slash in that case.


That is a great point. We could not find any standard as to whether or not there should be a trailing slash.
And from all the examples I could find, there seems to be a convention of omitting a trailing slash.
However, if userland provides a trailing slash, it would seem appropriate to retain it.

As a point of comparison, matching the given input is the behavior of 2.6.32-5 in Debian Squeeze.
So I think your approach is better.

-Chris

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4798 bytes --]

Re: NFS: kernel forces trailing slash for export in /proc/self/mounts

[Jonathan Nieder]

Hi Chris,

Chris Hiestand wrote:
> On Sep 16, 2012, at 7:00 AM, Ben Hutchings wrote:

>> This was my first thought - but what if userland provides a device name
>> with a slash on the end?  I think we have to report it back with the
>> slash in that case.
[...]
> As a point of comparison, matching the given input is the behavior of 2.6.32-5 in Debian Squeeze.
> So I think your approach is better.

Thanks for looking it over.  Did you get a chance to test Ben's patch?

Curious,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: NFS: kernel forces trailing slash for export in /proc/self/mounts

[Chris Hiestand]

[-- Attachment #1: Type: text/plain, Size: 368 bytes --]

I haven't had a chance to test it yet thanks. I'll be too busy until at least mid-next week, but I will test it then if nobody beats me to it.

-Chris


On Sep 30, 2012, at 2:23 PM, Jonathan Nieder <jrnieder-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> Thanks for looking it over.  Did you get a chance to test Ben's patch?
> 
> Curious,
> Jonathan


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4798 bytes --]

Re: NFS: kernel forces trailing slash for export in /proc/self/mounts

[Chris Hiestand]

[-- Attachment #1: Type: text/plain, Size: 512 bytes --]

I've applied the patch to Linus' master HEAD 
(43c422eda99b894f18d1cca17bcd2401efaf7bd0, at the time) and the patch seems to 
work fine.

/proc/self/mounts correctly reflects whether or not the user specified to use a 
trailing slash - and nothing is obviously broken.

If the patch looks fine to others, I propose we move this patch into wherever 
is the next step for testing it more broadly:
e.g. Debian experimental, and wherever it needs to go to end up in 
Linus' master branch.

Thanks,
Chris

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4798 bytes --]

[PATCH] nfs: Show original device name verbatim in /proc/*/mount{s,info}

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 4310 bytes --]

Since commit c7f404b ('vfs: new superblock methods to override
/proc/*/mount{s,info}'), nfs_path() is used to generate the mounted
device name reported back to userland.

nfs_path() always generates a trailing slash when the given dentry is
the root of an NFS mount, but userland may expect the original device
name to be returned verbatim (as it used to be).  Make this
canonicalisation optional and change the callers accordingly.

Reported-and-tested-by: Chris Hiestand <chiestand-xrR1t/MlNfQ@public.gmane.org>
Reference: http://bugs.debian.org/669314
Signed-off-by: Ben Hutchings <ben-/+tVBieCtBitmTQ+vhA3Yw@public.gmane.org>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v2.6.39+
---
 fs/nfs/internal.h      |    4 ++--
 fs/nfs/namespace.c     |   15 ++++++++++-----
 fs/nfs/nfs4namespace.c |    2 +-
 fs/nfs/super.c         |    2 +-
 4 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 31fdb03..5eaf902 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -354,7 +354,7 @@ extern void nfs_sb_deactive(struct super_block *sb);
 
 /* namespace.c */
 extern char *nfs_path(char **p, struct dentry *dentry,
-		      char *buffer, ssize_t buflen);
+		      char *buffer, ssize_t buflen, bool canonical);
 extern struct vfsmount *nfs_d_automount(struct path *path);
 struct vfsmount *nfs_submount(struct nfs_server *, struct dentry *,
 			      struct nfs_fh *, struct nfs_fattr *);
@@ -491,7 +491,7 @@ static inline char *nfs_devname(struct dentry *dentry,
 				char *buffer, ssize_t buflen)
 {
 	char *dummy;
-	return nfs_path(&dummy, dentry, buffer, buflen);
+	return nfs_path(&dummy, dentry, buffer, buflen, true);
 }
 
 /*
diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
index 6559253..059975e 100644
--- a/fs/nfs/namespace.c
+++ b/fs/nfs/namespace.c
@@ -33,6 +33,8 @@ int nfs_mountpoint_expiry_timeout = 500 * HZ;
  * @dentry - pointer to dentry
  * @buffer - result buffer
  * @buflen - length of buffer
+ * @canonical - ensure there is exactly one slash after the original
+ *		device (export) name; if false, return it verbatim
  *
  * Helper function for constructing the server pathname
  * by arbitrary hashed dentry.
@@ -41,7 +43,8 @@ int nfs_mountpoint_expiry_timeout = 500 * HZ;
  * server side when automounting on top of an existing partition
  * and in generating /proc/mounts and friends.
  */
-char *nfs_path(char **p, struct dentry *dentry, char *buffer, ssize_t buflen)
+char *nfs_path(char **p, struct dentry *dentry, char *buffer, ssize_t buflen,
+	       bool canonical)
 {
 	char *end;
 	int namelen;
@@ -74,7 +77,7 @@ rename_retry:
 		rcu_read_unlock();
 		goto rename_retry;
 	}
-	if (*end != '/') {
+	if (canonical && *end != '/') {
 		if (--buflen < 0) {
 			spin_unlock(&dentry->d_lock);
 			rcu_read_unlock();
@@ -91,9 +94,11 @@ rename_retry:
 		return end;
 	}
 	namelen = strlen(base);
-	/* Strip off excess slashes in base string */
-	while (namelen > 0 && base[namelen - 1] == '/')
-		namelen--;
+	if (canonical) {
+		/* Strip off excess slashes in base string */
+		while (namelen > 0 && base[namelen - 1] == '/')
+			namelen--;
+	}
 	buflen -= namelen;
 	if (buflen < 0) {
 		spin_unlock(&dentry->d_lock);
diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
index 017b4b0..94e8652 100644
--- a/fs/nfs/nfs4namespace.c
+++ b/fs/nfs/nfs4namespace.c
@@ -81,7 +81,7 @@ static char *nfs_path_component(const char *nfspath, const char *end)
 static char *nfs4_path(struct dentry *dentry, char *buffer, ssize_t buflen)
 {
 	char *limit;
-	char *path = nfs_path(&limit, dentry, buffer, buflen);
+	char *path = nfs_path(&limit, dentry, buffer, buflen, true);
 	if (!IS_ERR(path)) {
 		char *path_component = nfs_path_component(path, limit);
 		if (path_component)
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index ac6a3c5..6087ed0 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -730,7 +730,7 @@ int nfs_show_devname(struct seq_file *m, struct dentry *root)
 	int err = 0;
 	if (!page)
 		return -ENOMEM;
-	devname = nfs_path(&dummy, root, page, PAGE_SIZE);
+	devname = nfs_path(&dummy, root, page, PAGE_SIZE, false);
 	if (IS_ERR(devname))
 		err = PTR_ERR(devname);
 	else


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] nfs: Show original device name verbatim in /proc/*/mount{s,info}

[Jonathan Nieder]

(cc-ing Laurent in case he wants to test.  Laurent, a "git am"-ready
 patch is at [1])
Ben Hutchings wrote:

> Since commit c7f404b ('vfs: new superblock methods to override
> /proc/*/mount{s,info}'), nfs_path() is used to generate the mounted
> device name reported back to userland.
>
> nfs_path() always generates a trailing slash when the given dentry is
> the root of an NFS mount, but userland may expect the original device
> name to be returned verbatim (as it used to be).  Make this
> canonicalisation optional and change the callers accordingly.
>
> Reported-and-tested-by: Chris Hiestand <chiestand@salk.edu>
> Reference: http://bugs.debian.org/669314
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> Cc: <stable@vger.kernel.org> # v2.6.39+

Changing the content of /proc/mounts broke 

 user@hostname:/proc/self$ sudo umount.nfs nfsserver:/srv/ubuntu-32
 umount.nfs: nfsserver:/srv/ubuntu-32: not found

with nfs2 and nfs3 and this looks like the minimal change to get it
working again, so for what it's worth,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

How about something like the following on top?

-- >8 --
Subject: nfs: convert boolean nfs_path() argument to a flag word

If nfs_path() gains any other boolean settings, they can share the
flag argument, and this way call sites look like "nfs_path(&limit,
dentry, buffer, buflen, NFS_PATH_CANONICAL);" so the reader does not
need to guess the sense of true and false.  No functional change
intended.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
[1] http://download.gmane.org/gmane.linux.nfs/52755/52756

 fs/nfs/internal.h      |  5 +++--
 fs/nfs/namespace.c     | 14 +++++++++-----
 fs/nfs/nfs4namespace.c |  3 ++-
 fs/nfs/super.c         |  2 +-
 4 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 9c9603373d64..a54fe51c1dfb 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -353,8 +353,9 @@ extern void nfs_sb_active(struct super_block *sb);
 extern void nfs_sb_deactive(struct super_block *sb);
 
 /* namespace.c */
+#define NFS_PATH_CANONICAL 1
 extern char *nfs_path(char **p, struct dentry *dentry,
-		      char *buffer, ssize_t buflen, bool canonical);
+		      char *buffer, ssize_t buflen, unsigned flags);
 extern struct vfsmount *nfs_d_automount(struct path *path);
 struct vfsmount *nfs_submount(struct nfs_server *, struct dentry *,
 			      struct nfs_fh *, struct nfs_fattr *);
@@ -498,7 +499,7 @@ static inline char *nfs_devname(struct dentry *dentry,
 				char *buffer, ssize_t buflen)
 {
 	char *dummy;
-	return nfs_path(&dummy, dentry, buffer, buflen, true);
+	return nfs_path(&dummy, dentry, buffer, buflen, NFS_PATH_CANONICAL);
 }
 
 /*
diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
index 059975e492e1..dd057bc6b65b 100644
--- a/fs/nfs/namespace.c
+++ b/fs/nfs/namespace.c
@@ -33,8 +33,7 @@ int nfs_mountpoint_expiry_timeout = 500 * HZ;
  * @dentry - pointer to dentry
  * @buffer - result buffer
  * @buflen - length of buffer
- * @canonical - ensure there is exactly one slash after the original
- *		device (export) name; if false, return it verbatim
+ * @flags - options (see below)
  *
  * Helper function for constructing the server pathname
  * by arbitrary hashed dentry.
@@ -42,9 +41,14 @@ int nfs_mountpoint_expiry_timeout = 500 * HZ;
  * This is mainly for use in figuring out the path on the
  * server side when automounting on top of an existing partition
  * and in generating /proc/mounts and friends.
+ *
+ * Supported flags:
+ * NFS_PATH_CANONICAL: ensure there is exactly one slash after
+ *		       the original device (export) name
+ *		       (if unset, the original name is returned verbatim)
  */
 char *nfs_path(char **p, struct dentry *dentry, char *buffer, ssize_t buflen,
-	       bool canonical)
+	       unsigned flags)
 {
 	char *end;
 	int namelen;
@@ -77,7 +81,7 @@ rename_retry:
 		rcu_read_unlock();
 		goto rename_retry;
 	}
-	if (canonical && *end != '/') {
+	if ((flags & NFS_PATH_CANONICAL) && *end != '/') {
 		if (--buflen < 0) {
 			spin_unlock(&dentry->d_lock);
 			rcu_read_unlock();
@@ -94,7 +98,7 @@ rename_retry:
 		return end;
 	}
 	namelen = strlen(base);
-	if (canonical) {
+	if (flags & NFS_PATH_CANONICAL) {
 		/* Strip off excess slashes in base string */
 		while (namelen > 0 && base[namelen - 1] == '/')
 			namelen--;
diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
index 2f6f16331769..1e09eb78543b 100644
--- a/fs/nfs/nfs4namespace.c
+++ b/fs/nfs/nfs4namespace.c
@@ -81,7 +81,8 @@ static char *nfs_path_component(const char *nfspath, const char *end)
 static char *nfs4_path(struct dentry *dentry, char *buffer, ssize_t buflen)
 {
 	char *limit;
-	char *path = nfs_path(&limit, dentry, buffer, buflen, true);
+	char *path = nfs_path(&limit, dentry, buffer, buflen,
+			      NFS_PATH_CANONICAL);
 	if (!IS_ERR(path)) {
 		char *path_component = nfs_path_component(path, limit);
 		if (path_component)
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index c89a73da13d9..13c2a5be4765 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -771,7 +771,7 @@ int nfs_show_devname(struct seq_file *m, struct dentry *root)
 	int err = 0;
 	if (!page)
 		return -ENOMEM;
-	devname = nfs_path(&dummy, root, page, PAGE_SIZE, false);
+	devname = nfs_path(&dummy, root, page, PAGE_SIZE, 0);
 	if (IS_ERR(devname))
 		err = PTR_ERR(devname);
 	else
-- 
1.8.0.rc3

Re: [PATCH] nfs: Show original device name verbatim in /proc/*/mount{s,info}

[Myklebust, Trond]

On Sun, 2012-10-21 at 17:01 -0700, Jonathan Nieder wrote:
> (cc-ing Laurent in case he wants to test.  Laurent, a "git am"-ready
>  patch is at [1])
> Ben Hutchings wrote:
> 
> > Since commit c7f404b ('vfs: new superblock methods to override
> > /proc/*/mount{s,info}'), nfs_path() is used to generate the mounted
> > device name reported back to userland.
> >
> > nfs_path() always generates a trailing slash when the given dentry is
> > the root of an NFS mount, but userland may expect the original device
> > name to be returned verbatim (as it used to be).  Make this
> > canonicalisation optional and change the callers accordingly.
> >
> > Reported-and-tested-by: Chris Hiestand <chiestand@salk.edu>
> > Reference: http://bugs.debian.org/669314
> > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> > Cc: <stable@vger.kernel.org> # v2.6.39+


This patch will not apply to the upstream kernel; it seems rather to
have been generated incrementally against an older patch attempt.

Please regenerate and resend if you want this merged...

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

[PATCH v3] nfs: Show original device name verbatim in /proc/*/mount{s,info}

[Jonathan Nieder]

From: Ben Hutchings <ben-/+tVBieCtBitmTQ+vhA3Yw@public.gmane.org>
Date: Sun, 21 Oct 2012 19:23:52 +0100

Since commit c7f404b ('vfs: new superblock methods to override
/proc/*/mount{s,info}'), nfs_path() is used to generate the mounted
device name reported back to userland.

nfs_path() always generates a trailing slash when the given dentry is
the root of an NFS mount, but userland may expect the original device
name to be returned verbatim (as it used to be).  Make this
canonicalisation optional and change the callers accordingly.

[jrnieder-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org: use flag instead of bool argument]
Reported-and-tested-by: Chris Hiestand <chiestand-xrR1t/MlNfQ@public.gmane.org>
Reference: http://bugs.debian.org/669314
Signed-off-by: Ben Hutchings <ben-/+tVBieCtBitmTQ+vhA3Yw@public.gmane.org>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v2.6.39+
Signed-off-by: Jonathan Nieder <jrnieder-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
Myklebust, Trond wrote:

> This patch will not apply to the upstream kernel; it seems rather to
> have been generated incrementally against an older patch attempt.

I assume you mean the patch

  nfs: convert boolean nfs_path() argument to a flag word

does not apply against master.  It is just an optional code style
improvement on top of Ben's

  nfs: Show original device name verbatim in /proc/*/mount{s,info}

Here are the two squashed together for convenience.

Thanks,
Jonathan

 fs/nfs/internal.h      |  5 +++--
 fs/nfs/namespace.c     | 19 ++++++++++++++-----
 fs/nfs/nfs4namespace.c |  3 ++-
 fs/nfs/super.c         |  2 +-
 4 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 59b133c5d652..a54fe51c1dfb 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -353,8 +353,9 @@ extern void nfs_sb_active(struct super_block *sb);
 extern void nfs_sb_deactive(struct super_block *sb);
 
 /* namespace.c */
+#define NFS_PATH_CANONICAL 1
 extern char *nfs_path(char **p, struct dentry *dentry,
-		      char *buffer, ssize_t buflen);
+		      char *buffer, ssize_t buflen, unsigned flags);
 extern struct vfsmount *nfs_d_automount(struct path *path);
 struct vfsmount *nfs_submount(struct nfs_server *, struct dentry *,
 			      struct nfs_fh *, struct nfs_fattr *);
@@ -498,7 +499,7 @@ static inline char *nfs_devname(struct dentry *dentry,
 				char *buffer, ssize_t buflen)
 {
 	char *dummy;
-	return nfs_path(&dummy, dentry, buffer, buflen);
+	return nfs_path(&dummy, dentry, buffer, buflen, NFS_PATH_CANONICAL);
 }
 
 /*
diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
index 655925373b91..dd057bc6b65b 100644
--- a/fs/nfs/namespace.c
+++ b/fs/nfs/namespace.c
@@ -33,6 +33,7 @@ int nfs_mountpoint_expiry_timeout = 500 * HZ;
  * @dentry - pointer to dentry
  * @buffer - result buffer
  * @buflen - length of buffer
+ * @flags - options (see below)
  *
  * Helper function for constructing the server pathname
  * by arbitrary hashed dentry.
@@ -40,8 +41,14 @@ int nfs_mountpoint_expiry_timeout = 500 * HZ;
  * This is mainly for use in figuring out the path on the
  * server side when automounting on top of an existing partition
  * and in generating /proc/mounts and friends.
+ *
+ * Supported flags:
+ * NFS_PATH_CANONICAL: ensure there is exactly one slash after
+ *		       the original device (export) name
+ *		       (if unset, the original name is returned verbatim)
  */
-char *nfs_path(char **p, struct dentry *dentry, char *buffer, ssize_t buflen)
+char *nfs_path(char **p, struct dentry *dentry, char *buffer, ssize_t buflen,
+	       unsigned flags)
 {
 	char *end;
 	int namelen;
@@ -74,7 +81,7 @@ rename_retry:
 		rcu_read_unlock();
 		goto rename_retry;
 	}
-	if (*end != '/') {
+	if ((flags & NFS_PATH_CANONICAL) && *end != '/') {
 		if (--buflen < 0) {
 			spin_unlock(&dentry->d_lock);
 			rcu_read_unlock();
@@ -91,9 +98,11 @@ rename_retry:
 		return end;
 	}
 	namelen = strlen(base);
-	/* Strip off excess slashes in base string */
-	while (namelen > 0 && base[namelen - 1] == '/')
-		namelen--;
+	if (flags & NFS_PATH_CANONICAL) {
+		/* Strip off excess slashes in base string */
+		while (namelen > 0 && base[namelen - 1] == '/')
+			namelen--;
+	}
 	buflen -= namelen;
 	if (buflen < 0) {
 		spin_unlock(&dentry->d_lock);
diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
index 79fbb61ce202..1e09eb78543b 100644
--- a/fs/nfs/nfs4namespace.c
+++ b/fs/nfs/nfs4namespace.c
@@ -81,7 +81,8 @@ static char *nfs_path_component(const char *nfspath, const char *end)
 static char *nfs4_path(struct dentry *dentry, char *buffer, ssize_t buflen)
 {
 	char *limit;
-	char *path = nfs_path(&limit, dentry, buffer, buflen);
+	char *path = nfs_path(&limit, dentry, buffer, buflen,
+			      NFS_PATH_CANONICAL);
 	if (!IS_ERR(path)) {
 		char *path_component = nfs_path_component(path, limit);
 		if (path_component)
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index e831bce49766..13c2a5be4765 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -771,7 +771,7 @@ int nfs_show_devname(struct seq_file *m, struct dentry *root)
 	int err = 0;
 	if (!page)
 		return -ENOMEM;
-	devname = nfs_path(&dummy, root, page, PAGE_SIZE);
+	devname = nfs_path(&dummy, root, page, PAGE_SIZE, 0);
 	if (IS_ERR(devname))
 		err = PTR_ERR(devname);
 	else
-- 
1.8.0

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html