Date: Fri, 14 Jul 2017 19:33:28 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: rgrimes@freebsd.org Cc: John Baldwin <jhb@freebsd.org>, Ian Lepore <ian@freebsd.org>, Edward Tomasz Napierala <trasz@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r320803 - head/sbin/mount Message-ID: <20170714181053.F1041@besplex.bde.org> In-Reply-To: <201707131911.v6DJBTMC090066@pdx.rh.CN85.dnsmgr.net> References: <201707131911.v6DJBTMC090066@pdx.rh.CN85.dnsmgr.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 13 Jul 2017, Rodney W. Grimes wrote: >> On Tuesday, July 11, 2017 02:35:15 PM Ian Lepore wrote: >>> I think the docs on this are pretty clear... under -u it says: >>> >>> The set of options is determined by applying the options specified >>> in the argument to -o and finally applying the -r or -w option. > ... > > To shed some light on all of this I did the dig to find the original > documentaton on mount -u, which actually makes it rather clear what > and how things should be happening. Somehow this has been lost over > the years: > csrg change to mount.c adding -u > https://svnweb.freebsd.org/csrg/sbin/mount/mount.c?r1=39329&r2=39333 > > csrg change to mount.8 documenting -u > https://svnweb.freebsd.org/csrg/sbin/mount/mount.8?r1=39328&r2=39466 > This documenting of -u clearly states that /etc/fstab shall be > consulted during a mount -u, some place we lost that. FreeBSD intentionally removed this in the change that added -o fstab and -o current: X Index: mount.8 X =================================================================== X RCS file: /home/ncvs/src/sbin/mount/mount.8,v X retrieving revision 1.25 X retrieving revision 1.26 X diff -u -1 -r1.25 -r1.26 X --- mount.8 8 Apr 1999 13:59:42 -0000 1.25 X +++ mount.8 7 May 1999 05:22:07 -0000 1.26 X @@ -32,3 +32,3 @@ X .\" @(#)mount.8 8.8 (Berkeley) 6/16/94 X -.\" $Id: mount.8,v 1.24 1998/10/16 00:06:56 des Exp $ X +.\" $Id: mount.8,v 1.25 1999/04/08 13:59:42 ghelmer Exp $ X .\" X ... X @@ -289,9 +302,5 @@ X flag is also specified. X -The set of options is determined by first extracting the options X -for the file system from the X -.Xr fstab 5 X -table, X -then applying any options specified by the X -.Fl o X -argument, X +The set of options is determined by applying the options specified X +in the argument to X +.Fl o X and finally applying the The new behaviour is easier to use. To get the old behaviour, use "fstab" for the first option in the list. To not have this option, just don't use it. If it were forced like it used to be, then canceling it would be difficult. Even boolean options are only easy to cancel if they have positive logic. E.g., the cancelation of "ro" is "noro", but the cancelation of "rw" is itself since "norw" gives the positive option "ro". "fstab" cannot be canceled by "nofstab" since that is not supported. Non-boolean options are even more difficult to cancel. E.g., to cancel rsize=16384 for nfs, you have to know the current value and specify this again. I checked that "ro" is currently broken for -o current as well as for -o fstab. Both ignore the specified setting of "ro" and force "noro". So all of: -u # with "ro" currently set -u -o fstab # with "ro" in fstab -u -o current # with "ro" currently set change the current "ro" setting to "noro" == "ro" == "" = absense of MNT_RDONLY in the mount flags. This is because nmount(2) is too hard to use, and its use for "ro" is still broken despite many attempts to fix it. nmount(2) apparently doesn't understand that the absence of "ro" means "noro". So users or utilities have to suppy "noro" to get the historical default of "rw". mount/mount.c does this. This is a cancelation like the ones that the user would have to do to cancel the old forced "fstab". The order of the flags is important for cancelation. mount is aware of this, but gets it wrong anyway: X $ svn ann mount.c | grep -C1 noro X 1558 rgrimes case 'w': X 76198 dd options = catopt(options, "noro"); X 1558 rgrimes break; X -- X 163671 ru if ((init_flags & MNT_UPDATE) && (ro == 0)) X 163671 ru options = catopt(options, "noro"); X 186505 obrien X $ svn log -r 163671 mount.c X ------------------------------------------------------------------------ X r163671 | ru | 2006-10-24 22:02:29 +0000 (Tue, 24 Oct 2006) | 12 lines X X Revert rev. 1.86 by jmallett@ as it breaks "ro" mounts specified X in /etc/fstab. X X This has been happening due to the priority inversion; options X specified on the command line should take precedence over options X from fstab over default "noro" option, but since both the default X "noro" and options specified on the command line (-w, -r, -o ...) X were put into the same "options" variable, "noro" took precedence X over fstab "ro" (this is easily visible with "mount -d"). X X PR: bin/100164 X X ------------------------------------------------------------------------ Before r163671, the options list was initialized to "noro", so "noro" was specified even without -u unless it is canceled by a later option. I don't know why this didn't work -- "noro" is the correct default, and canceling it it later should work. r163671 moves this to the MNT_UPDATE case. Cancelation still doesn't work for this cases. Manual cancelation does work: "ro" on the command line after "fstab" or "current" works to add the "ro" that should be added for "fstab" or "current". I debugged this only as far as seeing "-u -o fstab" turning into the options list "noatime,noro,???" in the kernel, where /etc/fstab has "ro,noatime". "noro" apparently gets added at the end of the options instead of the start, so it cancels "ro" instead of vice versa. Indeed, r163671 seems to move the addition to precisely this wrong place. So r163671 seems to be perfectly backwards for the -u case, and it is unclear why it was needed for the non-(-u) case. Later cancelation of "ro" by "noro" seems to work to give the bug for -u, so why doesn't later cancelation of "noro" by "ro" work to give correct behaviour before r163671? Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170714181053.F1041>