Date: Mon, 15 Oct 2007 16:18:14 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Eugene Grosbein <eugen@grosbein.pp.ru> Cc: freebsd-bugs@freebsd.org Subject: Re: kern/116608: [panic] [patch] [msdosfs] msdosfs fails to check mount options Message-ID: <20071015154453.Y1164@besplex.bde.org> In-Reply-To: <200710141450.l9EEo4eI008836@freefall.freebsd.org> References: <200710141450.l9EEo4eI008836@freefall.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 14 Oct 2007, Eugene Grosbein wrote:
> Subject: Re: kern/116608: [panic] [patch] [msdosfs] msdosfs fails to check mount options
>
> Here is a version of last patch adjusted for 7.0-PRERELEASE:
Is the bug in -current just that "mount -o rw" doesn't give rw when fstab
says ro?
ffs seems to have the same bug -- rw just doesn't work. noro works.
This may be affected by my mount utilities being out of date (~5.2 mount,
~5.2 mount_ffs, old -current mount_msdosfs -- so old or broken that it
can't handle noauto in fstab).
Starting with a file system mounted ro, "mount -u -o fstab,rw" and
"mount -u -o current,rw" show the same bug (noro works). Even
"mount -u" and "mount -u -o rw" don't work (noro works) for msdosfs,
but work for ffs. This might be because my mount_msdosfs is old enough
to be missing fixes and my mount_ffs is old enough to be missing bugs
(it doesn't use nmount(), whose existence is the cause of most of these
bugs). Upgrading to the ~5.2 mount_msdosfs fixes the bugs for -u but
not for initial mounts. (I downgraded to the old -current mount_msdosfs
to test what it does. This works because at least the compatibility cruft
parts of nmount() work OK.)
> --- sys/fs/msdosfs/msdosfs_vfsops.c.orig 2007-08-16 01:40:09.000000000 +0800
> +++ sys/fs/msdosfs/msdosfs_vfsops.c 2007-10-14 17:58:20.000000000 +0800
> @@ -265,6 +265,7 @@
> }
> }
> if (!(pmp->pm_flags & MSDOSFSMNT_RONLY) &&
> + !vfs_flagopt(mp->mnt_optnew, "noro", NULL, 0) &&
> vfs_flagopt(mp->mnt_optnew, "ro", NULL, 0)) {
> error = VFS_SYNC(mp, MNT_WAIT, td);
> if (error)
> @@ -314,10 +315,12 @@
>
> ro_to_rw = 1;
> }
> + if(!vfs_flagopt(mp->mnt_optnew, "noro", NULL, 0)) {
> vfs_flagopt(mp->mnt_optnew, "ro",
> &pmp->pm_flags, MSDOSFSMNT_RONLY);
> vfs_flagopt(mp->mnt_optnew, "ro",
> &mp->mnt_flag, MNT_RDONLY);
> + }
>
> if (ro_to_rw) {
> /* Now that the volume is modifiable, mark it dirty. */
I think the patch belongs at a higher level. Individual file systems
already have a lot of bloat to parse options. Where mount(1) had
centralized table-driven option parsing for at least generic options,
ffs has a 4-line if statement for each supported option, and msdosfs
apparently has missing code -- why does ffs need more than a table to
indicate its support for the atime option, and how does atime work in
msdosfs where this support is missing; why does ffs have mounds of
code to parse both async and noasync, and if this is needed than why
isn't it needed for almost all options?
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20071015154453.Y1164>