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>
index | next in thread | previous in thread | raw e-mail
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
home |
help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20071015154453.Y1164>