Created attachment 497297 [details]
Patch - fix nfs4 check

Further investigation shows that this patch resolves the
problem for me and allows autofs to expire these mounts
again.

Comments please Steve.

Reassigning to comonent nfs-utils.

Note, I see the same bug in upstream nfs-utils tree.

Yeah, looks like upstream probably has the same issue. I wonder... does backing out this patch fix it too? That might be the safest thing for 6.1, and would allow us to get Ian's patch upstream and into RHEL6.2:

commit dc08c702a6c7f824f317af561f491635ee898a71
Author: Chuck Lever <chuck.lever>
Date:   Wed Oct 13 13:55:10 2010 -0400

    umount.nfs: Distinguish between nfs4 and nfs mounts

Ok, yes...backing out Chuck's patch also fixes this issue. Granted, his patch also fixes a legitimate bug, so I think we need something like Ian's patch ultimately, but dropping Chuck's patch from the code might be the safest route for now.

Created attachment 497339 [details]
Patch - revert distinguish between nfs4 and nfs mounts

This does appear to achieve the same outcome, although I
expect there are cases that the original patch catches
that won't be caught if we do this.

But it should at least give us the original behaviour.

Once everyone has had a chance to comment on the patch, and
any needed changes are made, I can post it upstream.

cc'ing Chuck since his patch introduced the regression...

The patch looks good to me. It seems like we ought to be using the routines that encapsulate this logic (as your patch does).

One very minor style nit in read_procmounts:

+        mfp = nfs_setmntent (fnam, "r");

There should be no space between nfs_setmntent and the '('.

The ultimate fix looks reasonable.  Probably this should be applied to nfsstat as well, which is where that code came from, according to the original patch description.

Reverting the original patch simply means that the RHEL 6 client will send a UMNT request for NFSv4 mounts.  I think that should be relatively harmless, but may result in a short hang in some corner cases.

(In reply to comment #11)
> 
> One very minor style nit in read_procmounts:
> 
> +        mfp = nfs_setmntent (fnam, "r");
> 
> There should be no space between nfs_setmntent and the '('.

Yeah, me too, that's a cut, paste and modify from other code
in utils/mount/fstab.c, so I left it to be consistent with the
existing coding style. Looking again I see it isn't quite
consistent their either, I'll change it.

(In reply to comment #12)
> The ultimate fix looks reasonable.  Probably this should be applied to nfsstat
> as well, which is where that code came from, according to the original patch
> description.

I don't see nfsstat as a problem. It's the added name comparision (with
the escaped octal space characters) that causes the breakage. I don't
see these comparisons in nfsstat.
 
> 
> Reverting the original patch simply means that the RHEL 6 client will send a
> UMNT request for NFSv4 mounts.  I think that should be relatively harmless, but
> may result in a short hang in some corner cases.

Yes, we've been there before. Unfortunately, those corner cases
seem to come up all too often at sites.

Created attachment 497698 [details]
Patch - fix nfs4 check (updated)

- style, remove space between function call and bracket.
- correct nfs-utils version in description text.

Created attachment 497699 [details]
Patch - fix nfs4 check (updated)

Try again, update nfs-utils version.

I have to wonder about the wisdom of escaping characters in the
mount table like this (although I don't know what else to do,
how does util-linux habdle this?).

With the move to eliminate the use of the straight text based
/etc/mtab, which doesn't appear to use these escapes, any shell
script and anything else that doesn't use the glibc functions
setmntent()/getmntent()/endmntent() will probably break.

All code in util-linux that read or write *tab files always use mangle() and unmangle() functions.

Note that nfs-utils upstream is already possible to link against libmount (this feature is enabled in rawhide/Fedora-16) -- see mount_libmount.c. All such low-level problems should be hidden in the library.

We have findmnt(8) for work with the fstab/mtab/mountinfo files in the shell scripts. It's definitely better than "grep /proc/mounts".

(In reply to comment #18)
> We have findmnt(8) for work with the fstab/mtab/mountinfo files in the shell
> scripts. It's definitely better than "grep /proc/mounts".

The reason the original patch looks in /proc/mounts is because /proc/mounts is authoritative with regard to mount option settings.  The exact effective mount option settings are in /proc/mounts.  They are not reflected in user space log files.  User space can know and remember exactly what the user requested, and /proc/mounts shows exactly what the kernel is providing.  Two different views of the same elephant.

For example, the user may not have specified rsize or wsize options; looking in /proc/mounts, we can see exactly what the server and the client kernel have negotiated.  R/wsize is negotiated by the NFS client in the kernel, thus those settings cannot be recorded by user space at mount time.

Thus, IMO, both user space files and /proc/mounts are important for complete visibility.

Should we instead/also fix the kernel to use mangling and escaping for showing pathnames in /proc/mounts?

(In reply to comment #14)
> (In reply to comment #12)
> > The ultimate fix looks reasonable.  Probably this should be applied to nfsstat
> > as well, which is where that code came from, according to the original patch
> > description.
> 
> I don't see nfsstat as a problem. It's the added name comparision (with
> the escaped octal space characters) that causes the breakage. I don't
> see these comparisons in nfsstat.

Certainly, nfsstat isn't broken.  But the new API in fstab.c is cleaner than open coding.  nfsstat can at least re-use this API.

Created attachment 498279 [details]
Patch - fix nfs4 check (update 2)

There's an additional problem with the mount table handling.

If there is a trailing '/' on the mount location and /etc/mtab
is being maintained independently, then since the mtab entry
won't have the trailing '/' the match in nfs_umount_is_vers4()
will fail. This didn't fail before because the check succeeded
if either the mount location or mount directory matched. I
dropped the mount directory check because in order to get a
passed in mntentchn pointer that has to match (an entry in
/etc/mtab) in the first place.

But this also raises the question of what happens when attempting
to umount nfs internally mounted mounts (that appear only in 
/proc/mounts). I think umount will fail because there is no entry
in /etc/mtab. Is that what we want? Do we need to check this for
CIFS?

There may be more yet, there is now a different unexplained
regression test failure.

(In reply to comment #21)
> But this also raises the question of what happens when attempting
> to umount nfs internally mounted mounts (that appear only in 
> /proc/mounts). I think umount will fail because there is no entry
> in /etc/mtab. Is that what we want? Do we need to check this for
> CIFS?

Already reported, see bug #702664. The trailing '/' is a problem. 

The mount.<type> helpers should be fixed, it's really bad idea to have "/foo" in fstab and "/foo/" in mtab (or /proc/mounts). I don't see reason why the trailing '/' should be required, mount(2) and umount(2) syscalls work as expected without the extra char.

(In reply to comment #22)
> (In reply to comment #21)
> > But this also raises the question of what happens when attempting
> > to umount nfs internally mounted mounts (that appear only in 
> > /proc/mounts). I think umount will fail because there is no entry
> > in /etc/mtab. Is that what we want? Do we need to check this for
> > CIFS?
> 
> Already reported, see bug #702664. The trailing '/' is a problem. 
> 
> The mount.<type> helpers should be fixed, it's really bad idea to have "/foo"
> in fstab and "/foo/" in mtab (or /proc/mounts). I don't see reason why the
> trailing '/' should be required, mount(2) and umount(2) syscalls work as
> expected without the extra char

Using a trailing slash can be a useful convenience but, as you
say, shouldn't be required and should be handled by utilities.

For example, consider the autofs multi-mount map entry in a
direct mount map:

/test/foo / s1:/p1 \
          /m1 s2:/p2
          /m2 s3:/p3

it's convenient to use /test/foo (fstype autofs) for the direct
mount trigger and /test/foo/ (fstype something else, eg. nfs) for
the root of the multi-mount. This gives a visual queue when looking
at largish mount tables trying to work out what's going on.

The autofs connectathon tests check trailing slashes are
handled too but I haven't run through that yet this time
round.

Note that the source field (1st in fstab) could be an arbitrary string. For example network filesystems or fuse use very crazy strings as a source field. For programs like umount(8) is it almost impossible to interpret and compare the FS specific strings (e.g. "//foo.com/abc") in any correct way.

Verified umount is successful on nfs-utils-1.2.3-8.el6 on i386 and x86_64:
[root@hp-dl120g6-01 ~]# mkdir /mnt/dir\ with\ spaces
[root@hp-dl120g6-01 ~]# mount -t nfs dell-per210-01.lab.bos.redhat.com:/tmp /mnt/dir\ with\ spaces
[root@hp-dl120g6-01 ~]# cat /proc/mounts 
...
dell-per210-01.lab.bos.redhat.com:/tmp/ /mnt/dir\040with\040spaces nfs4 rw,relatime,vers=4,rsize=1048576,wsize=1048576,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=10.16.67.0,minorversion=0,local_lock=none,addr=10.16.45.42 0 0

[root@hp-dl120g6-01 ~]# umount /mnt/dir\ with\ spaces
[root@hp-dl120g6-01 ~]# mount
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>  
Verify against the issue done, needinfo shown as below.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 

hi Steve,
I found exportfs give message about "/tmp/dir with spaces does not support NFS export", but in fact the dir containing spaces exports out.
So is it another problem? 

# cat /etc/exports 
"/tmp/dir with spaces" *(rw)

# /etc/init.d/nfs restart
Shutting down NFS mountd: [  OK  ]
Shutting down NFS daemon: [  OK  ]
Shutting down NFS quotas: [  OK  ]
Shutting down NFS services:  [  OK  ]
Starting NFS services:  exportfs: /tmp/dir with spaces does not support NFS export
[  OK  ]
Starting NFS quotas: [  OK  ]
Starting NFS daemon: [  OK  ]
Starting NFS mountd: [  OK  ]

# showmount -e localhost
Export list for localhost:
/tmp/dir with spaces *

(In reply to comment #29)
> Verified umount is successful on nfs-utils-1.2.3-8.el6 on i386 and x86_64:
> [root@hp-dl120g6-01 ~]# mkdir /mnt/dir\ with\ spaces
> [root@hp-dl120g6-01 ~]# mount -t nfs dell-per210-01.lab.bos.redhat.com:/tmp
> /mnt/dir\ with\ spaces
> [root@hp-dl120g6-01 ~]# cat /proc/mounts 
> ...
> dell-per210-01.lab.bos.redhat.com:/tmp/ /mnt/dir\040with\040spaces nfs4
> rw,relatime,vers=4,rsize=1048576,wsize=1048576,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=10.16.67.0,minorversion=0,local_lock=none,addr=10.16.45.42
> 0 0
> 
> [root@hp-dl120g6-01 ~]# umount /mnt/dir\ with\ spaces
> [root@hp-dl120g6-01 ~]# mount
> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>  
> Verify against the issue done, needinfo shown as below.
> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
> 
> hi Steve,
> I found exportfs give message about "/tmp/dir with spaces does not support NFS
> export", but in fact the dir containing spaces exports out.
> So is it another problem? 
> 
Where you able to mount that export from a client?

(In reply to comment #30)
> (In reply to comment #29)
> > Verified umount is successful on nfs-utils-1.2.3-8.el6 on i386 and x86_64:
> > [root@hp-dl120g6-01 ~]# mkdir /mnt/dir\ with\ spaces
> > [root@hp-dl120g6-01 ~]# mount -t nfs dell-per210-01.lab.bos.redhat.com:/tmp
> > /mnt/dir\ with\ spaces
> > [root@hp-dl120g6-01 ~]# cat /proc/mounts 
> > ...
> > dell-per210-01.lab.bos.redhat.com:/tmp/ /mnt/dir\040with\040spaces nfs4
> > rw,relatime,vers=4,rsize=1048576,wsize=1048576,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=10.16.67.0,minorversion=0,local_lock=none,addr=10.16.45.42
> > 0 0
> > 
> > [root@hp-dl120g6-01 ~]# umount /mnt/dir\ with\ spaces
> > [root@hp-dl120g6-01 ~]# mount
> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>  
> > Verify against the issue done, needinfo shown as below.
> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
> > 
> > hi Steve,
> > I found exportfs give message about "/tmp/dir with spaces does not support NFS
> > export", but in fact the dir containing spaces exports out.
> > So is it another problem? 
> > 
> Where you able to mount that export from a client?

hi Steve,
I mount from localhost(nfs server) in my test steps, but seems the problem is about start nfs service showing wrong message if exports dir with spaces. The wrong message is "exportfs: /tmp/dir with spaces does not support NFS export", but in fact, the dir with spaces exports and mount successfully.

# cat /etc/exports 
"/tmp/dir with spaces" *(rw)

# /etc/init.d/nfs restart
Shutting down NFS mountd: [  OK  ]
Shutting down NFS daemon: [  OK  ]
Shutting down NFS quotas: [  OK  ]
Shutting down NFS services:  [  OK  ]
Starting NFS services:  exportfs: /tmp/dir with spaces does not support NFS export
[  OK  ]
Starting NFS quotas: [  OK  ]
Starting NFS daemon: [  OK  ]
Starting NFS mountd: [  OK  ]

# /etc/init.d/nfs status
rpc.svcgssd is stopped
rpc.mountd (pid 16614) is running...
nfsd (pid 16611 16610 16609 16608 16607 16606 16605 16604) is running...
rpc.rquotad (pid 16598) is running...

(In reply to comment #31)
> (In reply to comment #30)
> > (In reply to comment #29)
> > > Verified umount is successful on nfs-utils-1.2.3-8.el6 on i386 and x86_64:
> > > [root@hp-dl120g6-01 ~]# mkdir /mnt/dir\ with\ spaces
> > > [root@hp-dl120g6-01 ~]# mount -t nfs dell-per210-01.lab.bos.redhat.com:/tmp
> > > /mnt/dir\ with\ spaces
> > > [root@hp-dl120g6-01 ~]# cat /proc/mounts 
> > > ...
> > > dell-per210-01.lab.bos.redhat.com:/tmp/ /mnt/dir\040with\040spaces nfs4
> > > rw,relatime,vers=4,rsize=1048576,wsize=1048576,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=10.16.67.0,minorversion=0,local_lock=none,addr=10.16.45.42
> > > 0 0
> > > 
> > > [root@hp-dl120g6-01 ~]# umount /mnt/dir\ with\ spaces
> > > [root@hp-dl120g6-01 ~]# mount
> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>  
> > > Verify against the issue done, needinfo shown as below.
> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
> > > 
> > > hi Steve,
> > > I found exportfs give message about "/tmp/dir with spaces does not support NFS
> > > export", but in fact the dir containing spaces exports out.
> > > So is it another problem? 
> > > 
> > Where you able to mount that export from a client?
> 
> hi Steve,
> I mount from localhost(nfs server) in my test steps, but seems the problem is
> about start nfs service showing wrong message if exports dir with spaces. The
> wrong message is "exportfs: /tmp/dir with spaces does not support NFS export",
> but in fact, the dir with spaces exports and mount successfully.

Hey Bruce,

Would you happen have any history as to why this message exists?

(In reply to comment #31)
> (In reply to comment #30)
> > (In reply to comment #29)
> > > Verified umount is successful on nfs-utils-1.2.3-8.el6 on i386 and x86_64:
> > > [root@hp-dl120g6-01 ~]# mkdir /mnt/dir\ with\ spaces
> > > [root@hp-dl120g6-01 ~]# mount -t nfs dell-per210-01.lab.bos.redhat.com:/tmp
> > > /mnt/dir\ with\ spaces
> > > [root@hp-dl120g6-01 ~]# cat /proc/mounts 
> > > ...
> > > dell-per210-01.lab.bos.redhat.com:/tmp/ /mnt/dir\040with\040spaces nfs4
> > > rw,relatime,vers=4,rsize=1048576,wsize=1048576,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=10.16.67.0,minorversion=0,local_lock=none,addr=10.16.45.42
> > > 0 0
> > > 
> > > [root@hp-dl120g6-01 ~]# umount /mnt/dir\ with\ spaces
> > > [root@hp-dl120g6-01 ~]# mount
> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>  
> > > Verify against the issue done, needinfo shown as below.
> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
> > > 
> > > hi Steve,
> > > I found exportfs give message about "/tmp/dir with spaces does not support NFS
> > > export", but in fact the dir containing spaces exports out.
> > > So is it another problem? 
> > > 
> > Where you able to mount that export from a client?
> 
> hi Steve,
> I mount from localhost(nfs server) in my test steps, but seems the problem is
> about start nfs service showing wrong message if exports dir with spaces. The
> wrong message is "exportfs: /tmp/dir with spaces does not support NFS export",
> but in fact, the dir with spaces exports and mount successfully.
> 
> # cat /etc/exports 
> "/tmp/dir with spaces" *(rw)
> 
> # /etc/init.d/nfs restart
> Shutting down NFS mountd: [  OK  ]
> Shutting down NFS daemon: [  OK  ]
> Shutting down NFS quotas: [  OK  ]
> Shutting down NFS services:  [  OK  ]
> Starting NFS services:  exportfs: /tmp/dir with spaces does not support NFS
> export
> [  OK  ]
Hmm... I'm seeing a different error...

# cat /etc/exports 
"/home/dir\ with\ spaces" *(rw)

# service nfs restart
Shutting down NFS mountd:                                  [  OK  ]
Shutting down NFS daemon:                                  [  OK  ]
Shutting down NFS quotas:                                  [  OK  ]
Shutting down NFS services:                                [  OK  ]
Starting NFS services:  exportfs: Failed to stat /home/dir\ with\ spaces: No such file or directory 

                                                           [  OK  ]
Starting NFS quotas:                                       [  OK  ]
Starting NFS daemon:                                       [  OK  ]
Starting NFS mountd:                                       [  OK  ]

# ls /home/dir\ with\ spaces/
./  ../

(In reply to comment #33)
> (In reply to comment #31)
> > (In reply to comment #30)
> > > (In reply to comment #29)
> > > > Verified umount is successful on nfs-utils-1.2.3-8.el6 on i386 and x86_64:
> > > > [root@hp-dl120g6-01 ~]# mkdir /mnt/dir\ with\ spaces
> > > > [root@hp-dl120g6-01 ~]# mount -t nfs dell-per210-01.lab.bos.redhat.com:/tmp
> > > > /mnt/dir\ with\ spaces
> > > > [root@hp-dl120g6-01 ~]# cat /proc/mounts 
> > > > ...
> > > > dell-per210-01.lab.bos.redhat.com:/tmp/ /mnt/dir\040with\040spaces nfs4
> > > > rw,relatime,vers=4,rsize=1048576,wsize=1048576,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=10.16.67.0,minorversion=0,local_lock=none,addr=10.16.45.42
> > > > 0 0
> > > > 
> > > > [root@hp-dl120g6-01 ~]# umount /mnt/dir\ with\ spaces
> > > > [root@hp-dl120g6-01 ~]# mount
> > > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>  
> > > > Verify against the issue done, needinfo shown as below.
> > > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
> > > > 
> > > > hi Steve,
> > > > I found exportfs give message about "/tmp/dir with spaces does not support NFS
> > > > export", but in fact the dir containing spaces exports out.
> > > > So is it another problem? 
> > > > 
> > > Where you able to mount that export from a client?
> > 
> > hi Steve,
> > I mount from localhost(nfs server) in my test steps, but seems the problem is
> > about start nfs service showing wrong message if exports dir with spaces. The
> > wrong message is "exportfs: /tmp/dir with spaces does not support NFS export",
> > but in fact, the dir with spaces exports and mount successfully.
> > 
> > # cat /etc/exports 
> > "/tmp/dir with spaces" *(rw)
> > 
> > # /etc/init.d/nfs restart
> > Shutting down NFS mountd: [  OK  ]
> > Shutting down NFS daemon: [  OK  ]
> > Shutting down NFS quotas: [  OK  ]
> > Shutting down NFS services:  [  OK  ]
> > Starting NFS services:  exportfs: /tmp/dir with spaces does not support NFS
> > export
> > [  OK  ]
> Hmm... I'm seeing a different error...
> 
> # cat /etc/exports 
> "/home/dir\ with\ spaces" *(rw)
> 
> # service nfs restart
> Shutting down NFS mountd:                                  [  OK  ]
> Shutting down NFS daemon:                                  [  OK  ]
> Shutting down NFS quotas:                                  [  OK  ]
> Shutting down NFS services:                                [  OK  ]
> Starting NFS services:  exportfs: Failed to stat /home/dir\ with\ spaces: No
> such file or directory 
> 
>                                                            [  OK  ]
> Starting NFS quotas:                                       [  OK  ]
> Starting NFS daemon:                                       [  OK  ]
> Starting NFS mountd:                                       [  OK  ]
> 
> # ls /home/dir\ with\ spaces/
> ./  ../

hi Steve,
I think it's a separate bug which may related to exports, so I filed a new bug 735696 to keep tracking the problem, thanks.

finish automation testcase and run on all arches, results passed:
https://beaker.engineering.redhat.com/jobs/128756
https://beaker.engineering.redhat.com/jobs/128754
https://beaker.engineering.redhat.com/jobs/128753
https://beaker.engineering.redhat.com/jobs/128752

    Technical note added. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    New Contents:
Previously, the function responsible for parsing the /proc/mounts file was not able to handle single quote characters in the path name of a mount points entries. As a consequence, an NFS exported file system could not be unmounted if its mount point contained space characters. To fix this problem, the parsing routine has been rewritten so that it now parses entries in the /proc/mounts file properly. All NFS file systems now can be unmounted as expected.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHSA-2011-1534.html

*** Bug 855157 has been marked as a duplicate of this bug. ***