| Message ID | 1479243592-38858-1-git-send-email-smayhew@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> On Nov 15, 2016, at 15:59, Scott Mayhew <smayhew@redhat.com> wrote: > > The nfs_server->mountd_protocol field doesn't get set when a v3 submount > is created, causing /proc/mounts to show 'mountproto=' without a netid. > This in turn causes umount.nfs to emit the error message "Failed to find > '' protocol" if you manually unmount the submount. > > Signed-off-by: Scott Mayhew <smayhew@redhat.com> > --- > fs/nfs/super.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c > index 001796b..b60946d 100644 > --- a/fs/nfs/super.c > +++ b/fs/nfs/super.c > @@ -532,7 +532,8 @@ static void nfs_show_mountd_netid(struct seq_file *m, struct nfs_server *nfss, > { > struct sockaddr *sap = (struct sockaddr *) &nfss->mountd_address; > > - seq_printf(m, ",mountproto="); > + if (nfss->mountd_protocol || showdefaults) > + seq_printf(m, ",mountproto="); > switch (sap->sa_family) { > case AF_INET: > switch (nfss->mountd_protocol) { Does it make sense to call nfs_show_mountd_options() at all for the case of a v3 submount? -- 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
On Tue, 15 Nov 2016, Trond Myklebust wrote: > > > On Nov 15, 2016, at 15:59, Scott Mayhew <smayhew@redhat.com> wrote: > > > > The nfs_server->mountd_protocol field doesn't get set when a v3 submount > > is created, causing /proc/mounts to show 'mountproto=' without a netid. > > This in turn causes umount.nfs to emit the error message "Failed to find > > '' protocol" if you manually unmount the submount. > > > > Signed-off-by: Scott Mayhew <smayhew@redhat.com> > > --- > > fs/nfs/super.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c > > index 001796b..b60946d 100644 > > --- a/fs/nfs/super.c > > +++ b/fs/nfs/super.c > > @@ -532,7 +532,8 @@ static void nfs_show_mountd_netid(struct seq_file *m, struct nfs_server *nfss, > > { > > struct sockaddr *sap = (struct sockaddr *) &nfss->mountd_address; > > > > - seq_printf(m, ",mountproto="); > > + if (nfss->mountd_protocol || showdefaults) > > + seq_printf(m, ",mountproto="); > > switch (sap->sa_family) { > > case AF_INET: > > switch (nfss->mountd_protocol) { > > Does it make sense to call nfs_show_mountd_options() at all for the case of a v3 submount? > Probably not, but I don't see a way to positively identify that it's a submount if all you have is the nfs_server struct (I don't see a way to backtrack to the mount struct). Or are you suggesting to just check one of the mountd-related fields (say mountd_addrlen) before calling nfs_show_mount_options() and skip it altogether if appropriate? -Scott -- 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
On Wed, 16 Nov 2016, Scott Mayhew wrote: > On Tue, 15 Nov 2016, Trond Myklebust wrote: > > > > > > On Nov 15, 2016, at 15:59, Scott Mayhew <smayhew@redhat.com> wrote: > > > > > > The nfs_server->mountd_protocol field doesn't get set when a v3 submount > > > is created, causing /proc/mounts to show 'mountproto=' without a netid. > > > This in turn causes umount.nfs to emit the error message "Failed to find > > > '' protocol" if you manually unmount the submount. > > > > > > Signed-off-by: Scott Mayhew <smayhew@redhat.com> > > > --- > > > fs/nfs/super.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c > > > index 001796b..b60946d 100644 > > > --- a/fs/nfs/super.c > > > +++ b/fs/nfs/super.c > > > @@ -532,7 +532,8 @@ static void nfs_show_mountd_netid(struct seq_file *m, struct nfs_server *nfss, > > > { > > > struct sockaddr *sap = (struct sockaddr *) &nfss->mountd_address; > > > > > > - seq_printf(m, ",mountproto="); > > > + if (nfss->mountd_protocol || showdefaults) > > > + seq_printf(m, ",mountproto="); > > > switch (sap->sa_family) { > > > case AF_INET: > > > switch (nfss->mountd_protocol) { > > > > Does it make sense to call nfs_show_mountd_options() at all for the case of a v3 submount? > > > Probably not, but I don't see a way to positively identify that it's a > submount if all you have is the nfs_server struct (I don't see a way to > backtrack to the mount struct). Or are you suggesting to just check one > of the mountd-related fields (say mountd_addrlen) before calling > nfs_show_mount_options() and skip it altogether if appropriate? > Having looked at it a little more, I do see a few ways but none of them are particularly good. One way would be to dereference nfss->super and walk the s_mounts list looking for any mount whose mnt_expire list is nonempty... but comment beside the s_mounts declaration says '_not_ for fs use', plus it doesn't look like the mount_lock is meant to be used outside of the vfs layer either, so scratch that. Another way would be to walk the nfs_automount_list, comparing nfss to mnt->mnt.mnt_sb->s_fs_info... I made a patch to do that but it works but it adds the overhead of calling a function just so we can (maybe) avoid calling nfs_show_mount_options() which is inlined anyways. A third way would be to add a flag to the nfs_server->flags field to indicate that it's a submount... I don't see any use for said flag aside from checking it in nfs_show_mount_options(), so that seems like wasting a flag. I still prefer the original patch since it pretty much restores the behavior that was there before commit ee671b016, and all I'm trying to do is squelch a harmless error message (since the umount still succeeds). -Scott -- 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
diff --git a/fs/nfs/super.c b/fs/nfs/super.c index 001796b..b60946d 100644 --- a/fs/nfs/super.c +++ b/fs/nfs/super.c @@ -532,7 +532,8 @@ static void nfs_show_mountd_netid(struct seq_file *m, struct nfs_server *nfss, { struct sockaddr *sap = (struct sockaddr *) &nfss->mountd_address; - seq_printf(m, ",mountproto="); + if (nfss->mountd_protocol || showdefaults) + seq_printf(m, ",mountproto="); switch (sap->sa_family) { case AF_INET: switch (nfss->mountd_protocol) {
The nfs_server->mountd_protocol field doesn't get set when a v3 submount is created, causing /proc/mounts to show 'mountproto=' without a netid. This in turn causes umount.nfs to emit the error message "Failed to find '' protocol" if you manually unmount the submount. Signed-off-by: Scott Mayhew <smayhew@redhat.com> --- fs/nfs/super.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)