| Message ID | 20200220130620.3547817-1-smayhew@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | NFS: Don't hard-code the fs_type when submounting | expand |
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
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
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
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);
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(-)