On Wed, Dec 17, 2008 at 03:13:40PM +0100, Pavel Filipensky wrote:
> 1)
> I am wondering if I should add this comment as a warning for  future
> code changes which would go OTW and cause regression of  "6675447
> NFSv4 client hangs on shutdown if server is down beforehand."
> 
> 
> # we cannot us 'df -F nfs $mountp' to test if the given mountpoint has 
> NFS on top of it
> # since df could trigger NFS over-the-wire request and cause regression of 
> # "6675447 NFSv4 client hangs  on shutdown if server is down beforehand."

Sure.

> 2)
> The more I am looking to umountall(1M) the more I think that
> umountall(1M) should get a new syscall and all the work apart from
> parsing the command line options should be done in the kernel.
> 
> Now, the kernel exports its state via a special file system in
> /etc/mnttab.  Information from /etc/mnttab is parsed as a text in
> umountall(1M) - parsing is error prone - space can do lot of harm -
> see 6786256. After the information is parsed it is processed with low
> efficiency just to get a list of mount points to be unmounted.  This
> list goes back to the kernel via umount(1).  Current shell
> implementation is slow and hard to maintain.

I wouldn't worry about this too much.  If you use the shell built-in
read function, double quote expansions of variables containing mount
point paths, and the kernel backslash-quotes whitespace in mount point
paths in /etc/mnttab then you're OK.

> There are two pending bugs which would profit from the new umounatll
> syscall:
> 
> 6733798 manpage umountall(1M) - specify what is a local/remote 
> filesystem + Interface stability
> 6786256 umountall(1M) handles incorrectly mounpoints with a space 
> character in the path

umountall(1M) uses read, but not correctly:

 - first it reads a line at a time from mnttab to reverse the order of
   mnttab

 - then it echos the reversed list to a pipe to a shell function that
   parses each line

The problem with that is that any backslash quoting in the original will
be lost in the first read.  The Korn shell has a -r argument to read
that prevents this, but you can't use the Korn shell.

You could move /bin/tac to /usr/sbin and then use tac(1) to do the
reversing.  THEN the read in domounts() will see any backslashes and
work correctly.

To make it easier for you I wrote you the attached function to
backslash-quote white-space in strings.  That way you can change the
main body of unmountall to:

 249  246          while read dev mountp fstype mode dummy

if [ ! -x /usr/bin/tail ]; then
        exec < $MNTTAB
        REVERSED=
        while read dev mountp rest; do
                bquote mountp
                if [ -n "$REVERSED" ]; then
                        REVERSED="$dev $bquoted $rest\n$REVERSED"
                else
                        REVERSED="$dev $bquoted $rest"
                fi
        done

        error=`echo $REVERSED | doumounts`
else
        error=`tail -r $MNTTAB | doumounts`
fi

Yes, I'm assuming that mnttab backslash-quotes whitespace.

Nico
-- 

Reply via email to