Date: Fri, 30 Apr 2010 23:00:45 +0200 From: Jilles Tjoelker <jilles@stack.nl> To: Andriy Gapon <avg@icyb.net.ua> Cc: freebsd-rc@freebsd.org Subject: Re: rc.d/root: handle filesystems with r/o support only Message-ID: <20100430210045.GA47661@stack.nl> In-Reply-To: <4BCA090E.1040403@icyb.net.ua> References: <4BCA090E.1040403@icyb.net.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Apr 17, 2010 at 10:16:30PM +0300, Andriy Gapon wrote: > Could you please review the following patch? > The idea is to not try to remount root R/W if root filesystem supports > only R/O. For example, cd9660. This should make life easier for live > CDs that use standard rc startup. Currently one has to either put a > root fs entry into fstab (and thus guess root device name, e.g. cd0 vs > acd0) or to specify root_rw_mount="NO" in rc.conf. This change > attempts to guess that automatically. This looks useful, although modifying rc.conf doesn't seem too much of a burden for a livecd. > I think that things like ro_fs_list, in_list and is_readonly_fs could > be shared with other rc scripts. > E.g. see http://www.freebsd.org/cgi/query-pr.cgi?pr=conf/116931 > P.S. sorry if the code style and structure differs from conventions. > --- a/etc/rc.d/root > +++ b/etc/rc.d/root > @@ -13,11 +13,54 @@ name="root" > start_cmd="root_start" > stop_cmd=":" > > +ro_fs_list="cd9660 udf" > + > +in_list() > +{ > + local _x _list _i > + > + _x=$1 > + _list=$2 > + for _i in ${_list}; do > + [ "${_x}" = "${_i}" ] && return 0 > + done > + return 1 > +} case " $_list " in *" $_x "*) return 0;; *) return 1;; esac This also works if the list elements contain metacharacters or IFS characters other than space (tab/newline). Although many of the functions in rc.subr break when given such input, I don't think that's an excuse for adding more functions that are broken like that. (You could also fix it using local IFS=' ' - set -f but 'local -' is a non-portable ash feature to save/restore the options; doing this portably needs more code.) Alternatively, the code case "$fstype" in cd9660|udf) return 0;; *) return 1;; esac could be put in a function. > + > +is_readonly_fs() > +{ > + local _arg _ret > + > + _arg="$1" ; shift > + _ret=`mount -p | while read _dev _mp _type _rest; do > + [ $_mp = "$_arg" ] || continue > + echo $_type > + break > + done` > + > + if [ -z "${_ret}" ]; then > + warn "root filesystem not found" Hmm, the function still becomes /-specific in the debug messages. > + return 1 > + fi > + if in_list "${_ret}" "${ro_fs_list}"; then > + info "read-only root filesystem type: ${_ret}" > + return 0 > + else > + info "read-write root filesystem type: ${_ret}" > + return 1 > + fi > +} > + > root_start() > { > # root normally must be read/write, but if this is a BOOTP NFS > # diskless boot it does not have to be. > # > + > + if is_readonly_fs '/' ; then > + root_rw_mount="NO" > + fi > + > case ${root_rw_mount} in > [Nn][Oo] | '') > ;; -- Jilles Tjoelker
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100430210045.GA47661>