cvs commit: src/sbin/fsck_ffs main.c

Yar Tikhiy yar at
Wed Mar 5 12:20:34 UTC 2008

On Wed, Mar 05, 2008 at 08:25:49AM +0000, Craig Rodrigues wrote:
> rodrigc     2008-03-05 08:25:49 UTC
>   FreeBSD src repository
>   Modified files:
>     sbin/fsck_ffs        main.c 
>   Log:
>   For a mounted file system which is read-only, when
>   doing the MNT_RELOAD, pass in "ro" and "update"
>   string mount options to nmount() instead of MNT_RDONLY and MNT_UPDATE flags.
>   Due to the complexity of the mount parsing code especially
>   with respect to the root file system, passing in MNT_RDONLY and MNT_UPDATE
>   flags would do weird things and would cause fsck to convert the root
>   file system from a read-only mount to read-write.

Your analysis of the problem sounds not quite correct to me.

In fact, the weird things happen because the "ro" string option
isn't set on the root mount from the outset.  After your patch,
fsck just adds the missing "ro" to the root mount options `when it
becomes safe to do so' -- the "ro" string option is dangerous to
set on the root FS from the outset due to a bunch of bugs in the
kernel code, as my experience with this issue showed to the public.
The bugs are all the same: one piece of code passes bit flags and
old-style mount() arguments while its counterpart wants string
options or vice versa.  Our current position in the middle of the
way appears totally unacceptable as it leads to problems like this
one.  Either we should arrive at using string options exlusively,
or abandon them at all.

Also note that, were the string options implemented in full, there
would be no need for fsck to pass "ro" along with "update" to preserve
read-only status: existing string options go away from the mount
point only if explicitly turned off through a "no" prefix, as in
"noro".  This is an evident design point of string options, and it's
implemented already (modulo bugs in vfs_mergeopts().)

Finally, it can be argued that "update" actually belongs to nmount()
flags, not to string options, because essentially it is not a
persistent mount option, it's a modifier for the current operation
requested by this nmount() call.  But this is a topic for a separate
discussion on the desired nmount(2) semantics.

I won't restart our back-out war, but please clearly mark your
change as a temporary workaround so that it doesn't mislead people,
including us when we revisit the code.  Adding "ro" to the root
mount options lately is an obvious hack, and it isn't a job for
fsck at all.

It'll be safe to set the "ro" string option on the root mount from
the beginning as soon as all the root-capable filesystems and their
mount_foo tools are converted to using string options properly.  A
crucial part of the task is your work on nfs_mount(), which I fully


>   To test:
>    - boot into single user mode
>    - show mounted file systems with: mount
>    - root file system should be mounted read-only
>    - fsck /
>    - show mounted file systems with: mount
>    - root file system should still be mounted read-only
>   PR:             120319
>   MFC after:      1 month
>   Reported by:    yar
>   Revision  Changes    Path
>   1.49      +3 -1      src/sbin/fsck_ffs/main.c


More information about the cvs-src mailing list