All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] NFS: Don't hard-code the fs_type when submounting
@ 2020-02-20 13:06 Scott Mayhew
  2020-02-20 13:46 ` Patrick Steinhardt
  0 siblings, 1 reply; 4+ messages in thread
From: Scott Mayhew @ 2020-02-20 13:06 UTC (permalink / raw)
  To: trond.myklebust, anna.schumaker; +Cc: ps, dhowells, linux-nfs

Hard-coding the fstype causes "nfs4" mounts to appear as "nfs",
which breaks scripts that do "umount -at nfs4".

Reported-by: Patrick Steinhardt <ps@pks.im>
Fixes: f2aedb713c28 ("NFS: Add fs_context support.")
Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 fs/nfs/namespace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
index ad6077404947..f3ece8ed3203 100644
--- a/fs/nfs/namespace.c
+++ b/fs/nfs/namespace.c
@@ -153,7 +153,7 @@ struct vfsmount *nfs_d_automount(struct path *path)
 	/* Open a new filesystem context, transferring parameters from the
 	 * parent superblock, including the network namespace.
 	 */
-	fc = fs_context_for_submount(&nfs_fs_type, path->dentry);
+	fc = fs_context_for_submount(path->mnt->mnt_sb->s_type, path->dentry);
 	if (IS_ERR(fc))
 		return ERR_CAST(fc);
 
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] NFS: Don't hard-code the fs_type when submounting
  2020-02-20 13:06 [PATCH] NFS: Don't hard-code the fs_type when submounting Scott Mayhew
@ 2020-02-20 13:46 ` Patrick Steinhardt
  2020-02-21 20:21   ` Scott Mayhew
  0 siblings, 1 reply; 4+ messages in thread
From: Patrick Steinhardt @ 2020-02-20 13:46 UTC (permalink / raw)
  To: Scott Mayhew; +Cc: trond.myklebust, anna.schumaker, dhowells, linux-nfs

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

On Thu, Feb 20, 2020 at 08:06:20AM -0500, Scott Mayhew wrote:
> Hard-coding the fstype causes "nfs4" mounts to appear as "nfs",
> which breaks scripts that do "umount -at nfs4".
> 
> Reported-by: Patrick Steinhardt <ps@pks.im>
> Fixes: f2aedb713c28 ("NFS: Add fs_context support.")
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> ---
>  fs/nfs/namespace.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
> index ad6077404947..f3ece8ed3203 100644
> --- a/fs/nfs/namespace.c
> +++ b/fs/nfs/namespace.c
> @@ -153,7 +153,7 @@ struct vfsmount *nfs_d_automount(struct path *path)
>  	/* Open a new filesystem context, transferring parameters from the
>  	 * parent superblock, including the network namespace.
>  	 */
> -	fc = fs_context_for_submount(&nfs_fs_type, path->dentry);
> +	fc = fs_context_for_submount(path->mnt->mnt_sb->s_type, path->dentry);
>  	if (IS_ERR(fc))
>  		return ERR_CAST(fc);

Thanks for your fix! While this fixes the fstype with mount.nfs4(8),
it still doesn't work when using mount(8):

     $ sudo mount server:/mnt /mnt && findmnt -n -ofstype /mnt
     nfs
     $ sudo umount /mnt
     $ sudo mount.nfs4 server:/mnt /mnt && findmnt -n -ofstype /mnt
     nfs4

I guess the issue is that the kernel doesn't yet know which NFS version
the server provides at the point where `fs_context_for_submount()` is
called.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] NFS: Don't hard-code the fs_type when submounting
  2020-02-20 13:46 ` Patrick Steinhardt
@ 2020-02-21 20:21   ` Scott Mayhew
  2020-02-22  7:25     ` Patrick Steinhardt
  0 siblings, 1 reply; 4+ messages in thread
From: Scott Mayhew @ 2020-02-21 20:21 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: trond.myklebust, anna.schumaker, dhowells, linux-nfs

On Thu, 20 Feb 2020, Patrick Steinhardt wrote:

> On Thu, Feb 20, 2020 at 08:06:20AM -0500, Scott Mayhew wrote:
> > Hard-coding the fstype causes "nfs4" mounts to appear as "nfs",
> > which breaks scripts that do "umount -at nfs4".
> > 
> > Reported-by: Patrick Steinhardt <ps@pks.im>
> > Fixes: f2aedb713c28 ("NFS: Add fs_context support.")
> > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > ---
> >  fs/nfs/namespace.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
> > index ad6077404947..f3ece8ed3203 100644
> > --- a/fs/nfs/namespace.c
> > +++ b/fs/nfs/namespace.c
> > @@ -153,7 +153,7 @@ struct vfsmount *nfs_d_automount(struct path *path)
> >  	/* Open a new filesystem context, transferring parameters from the
> >  	 * parent superblock, including the network namespace.
> >  	 */
> > -	fc = fs_context_for_submount(&nfs_fs_type, path->dentry);
> > +	fc = fs_context_for_submount(path->mnt->mnt_sb->s_type, path->dentry);
> >  	if (IS_ERR(fc))
> >  		return ERR_CAST(fc);
> 
> Thanks for your fix! While this fixes the fstype with mount.nfs4(8),
> it still doesn't work when using mount(8):
> 
>      $ sudo mount server:/mnt /mnt && findmnt -n -ofstype /mnt
>      nfs
>      $ sudo umount /mnt
>      $ sudo mount.nfs4 server:/mnt /mnt && findmnt -n -ofstype /mnt
>      nfs4
> 
> I guess the issue is that the kernel doesn't yet know which NFS version
> the server provides at the point where `fs_context_for_submount()` is
> called.

Thanks for testing.  Actually the problem is that the super_block's
s_type is now based on the fs_context->fs_type field, which is set when
the fs_context is created (way before the mount options are parsed).
When you use mount(8) without specifying '-t nfs4', it defaults to
using the mount.nfs helper, which calls mount(2) with 'nfs' as the
fstype.  I'm sending a second patch that double-checks the
fs_context->fs_type after the mount options have been parsed.  We still
shouldn't be hard-coding the fstype in fs_context_for_submount() though
(i.e. both patches are needed).

-Scott
> 
> Patrick



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] NFS: Don't hard-code the fs_type when submounting
  2020-02-21 20:21   ` Scott Mayhew
@ 2020-02-22  7:25     ` Patrick Steinhardt
  0 siblings, 0 replies; 4+ messages in thread
From: Patrick Steinhardt @ 2020-02-22  7:25 UTC (permalink / raw)
  To: Scott Mayhew; +Cc: trond.myklebust, anna.schumaker, dhowells, linux-nfs

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

On Fri, Feb 21, 2020 at 03:21:05PM -0500, Scott Mayhew wrote:
> On Thu, 20 Feb 2020, Patrick Steinhardt wrote:
> 
> > On Thu, Feb 20, 2020 at 08:06:20AM -0500, Scott Mayhew wrote:
> > > Hard-coding the fstype causes "nfs4" mounts to appear as "nfs",
> > > which breaks scripts that do "umount -at nfs4".
> > > 
> > > Reported-by: Patrick Steinhardt <ps@pks.im>
> > > Fixes: f2aedb713c28 ("NFS: Add fs_context support.")
> > > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > > ---
> > >  fs/nfs/namespace.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
> > > index ad6077404947..f3ece8ed3203 100644
> > > --- a/fs/nfs/namespace.c
> > > +++ b/fs/nfs/namespace.c
> > > @@ -153,7 +153,7 @@ struct vfsmount *nfs_d_automount(struct path *path)
> > >  	/* Open a new filesystem context, transferring parameters from the
> > >  	 * parent superblock, including the network namespace.
> > >  	 */
> > > -	fc = fs_context_for_submount(&nfs_fs_type, path->dentry);
> > > +	fc = fs_context_for_submount(path->mnt->mnt_sb->s_type, path->dentry);
> > >  	if (IS_ERR(fc))
> > >  		return ERR_CAST(fc);
> > 
> > Thanks for your fix! While this fixes the fstype with mount.nfs4(8),
> > it still doesn't work when using mount(8):
> > 
> >      $ sudo mount server:/mnt /mnt && findmnt -n -ofstype /mnt
> >      nfs
> >      $ sudo umount /mnt
> >      $ sudo mount.nfs4 server:/mnt /mnt && findmnt -n -ofstype /mnt
> >      nfs4
> > 
> > I guess the issue is that the kernel doesn't yet know which NFS version
> > the server provides at the point where `fs_context_for_submount()` is
> > called.
> 
> Thanks for testing.  Actually the problem is that the super_block's
> s_type is now based on the fs_context->fs_type field, which is set when
> the fs_context is created (way before the mount options are parsed).
> When you use mount(8) without specifying '-t nfs4', it defaults to
> using the mount.nfs helper, which calls mount(2) with 'nfs' as the
> fstype.  I'm sending a second patch that double-checks the
> fs_context->fs_type after the mount options have been parsed.  We still
> shouldn't be hard-coding the fstype in fs_context_for_submount() though
> (i.e. both patches are needed).

I can confirm that the problem is fixed with both patches applied.
Thanks a lot!

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-02-22  7:25 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-20 13:06 [PATCH] NFS: Don't hard-code the fs_type when submounting Scott Mayhew
2020-02-20 13:46 ` Patrick Steinhardt
2020-02-21 20:21   ` Scott Mayhew
2020-02-22  7:25     ` Patrick Steinhardt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.