umount and remount readonly code path: linvfs_remount

["Amit D. Chaudhary" <amitc@xxxxxxxxxxx>]

Hi,

After doing some debugging into a particular problem we have been facing 
with xfs and mount and remount / as readonly, I might have found a 
defect which is triggered once in a while.
We have 2 XFS filesystems, during a reboot first /mnt is umounted, then 
/ is remounted readonly. This is xfs 1.0.1 on powerpc. Once in a while 
after some writes being done on /, there would be junk that is printed 
during reboot. An example,
mount: unknown mount option "mà".
umount2: No space left on device
...

This was traced to a prink in fs/xfs/linux/xfs_super.c in 
mountargs_xfs() (now called xfs_parseargs() ). What is happening is that 
the options string is null during some of the calls to this function, 
most of the time strtok works fine and returns nothing with a NULL 
parameter as probably the internal static string is fine. Sometimes, it 
returns a junk string which is then attempted to match against valid 
remount options and fails. This also skips the rest of processing in 
linvfs_remount.
This problem also exists in the latest version in cvs,
http://oss.sgi.com/cgi-bin/cvsweb.cgi/~checkout~/linux-2.4-xfs/linux/fs/xfs/linux/xfs_super.c?rev=1.148&content-type=text/plain

There are many ways this can be fixed, including not calling 
xfs_parseargs from linvfs_remount when options is null or the simpler 
skip the loop for options if null. A patch for later is attached.

Another minor point, maybe the error -EINVAL is better than -ENOSPC on 
line 720 of xfs_super.c, linvfs_remount()

Comments or advise if I am doing things wrong are welcome.

Regards
Amit

--- xfs_super.c Mon Dec 17 17:59:37 2001
+++ xfs_super-remountfix.c      Mon Dec 17 18:34:31 2001
@@ -137,6 +137,9 @@ xfs_parseargs(
 
        args->flags |= XFSMNT_32BITINODES;
        
+       if(!options)
+           goto skip_options;
+
        for (this_char = strtok (options, ",");
             this_char != NULL;
             this_char = strtok (NULL, ",")) {
@@ -234,6 +237,7 @@ xfs_parseargs(
                 }
        }
 
+ skip_options:
        if (args->flags & XFSMNT_NORECOVERY) {
                if ((args->flags & MS_RDONLY) == 0) {
                        printk("XFS: no-recovery mounts must be read-only.\n");