On Sun, Jan 29, 2012 at 03:32:53PM -0500, Dave Reisner wrote:
> On Sun, Jan 29, 2012 at 04:22:22AM +0100, Lukas Fleischer wrote:
> > This one adds support for the "/fastboot" file, as well as the
> > "fastboot" kernel command line parameter. If either of them is
> > specified, file system checks are skipped. The only exception is the
> > existence of "/forcefsck" or the "forcefsck" kernel parameter which have
> > precedence over fastboot.
> >
> > Implements FS#26154.
> >
> > Reported-by: Mark <m...@voidzero.net>
> > Signed-off-by: Lukas Fleischer <archli...@cryptocrack.de>
> > ---
> > I ran into this today (wanted to skip file system checks for a couple of
> > reboots), discovered FS#26154 and decided to write a patch...
> >
> > A couple of things that I'd like to mention/ask:
> >
> > * I decided that forcefsck always has precedence over fastboot, even if
> > forcefsck was enabled by `shutdown -F` ("/forcefsck") and "fastboot"
> > was specified on the kernel command line. I'm not sure whether this is
> > the right thing to do here, but this is what systemd does and it seems
> > quite reasonable to me. Tom, let me know if there are any reasons to
> > do this differently (e.g. skip checks if "/forcefsck" exists and
> > "fastboot" is specified on the kernel command line).
> >
> > * I didn't implement the "fsck.mode" kernel parameter that Tom mentioned
> > in FS#26154 yet. I can implement this in a follow-up patch if you
> > want, though. Is "fsck.mode" intended to be an alternative for
> > "forcefsck" and "fastboot" or should we drop them in favor of that new
> > option?
> >
> > * The "sysinit_prefsck" and "sysinit_postfsck" hooks won't be run if
> > file system checks are skipped. Will that cause any problems?
>
> This all sounds good to me. I had previously added 'skipfsck' to
> mkinitcpio, but it was never documented anywhere. I'll change this to
> 'fastboot'.
Sounds good!
>
> > rc.sysinit | 27 +++++++++++++++------------
> > tmpfiles.conf | 1 +
> > 2 files changed, 16 insertions(+), 12 deletions(-)
> >
> > diff --git a/rc.sysinit b/rc.sysinit
> > index b70ceaa..6661d0e 100755
> > --- a/rc.sysinit
> > +++ b/rc.sysinit
> > @@ -175,19 +175,22 @@ fi
> > # Check filesystems
> > [[ -f /forcefsck ]] || in_array forcefsck $(< /proc/cmdline) &&
> > FORCEFSCK="-- -f"
> > declare -r FORCEFSCK
> > -run_hook sysinit_prefsck
> > -if [[ -x $(type -P fsck) ]]; then
> > - stat_busy "Checking Filesystems"
> > - fsck_all >|"${FSCK_OUT:-/dev/stdout}"
> > 2>|"${FSCK_ERR:-/dev/stdout}"
> > - declare -r fsckret=$?
> > - (( fsckret <= 1 )) && stat_done || stat_fail
> > -else
> > - declare -r fsckret=0
> > -fi
> > -run_hook sysinit_postfsck
> >
> > -# Single-user login and/or automatic reboot if needed
> > -fsck_reboot $fsckret
> > +if [[ -n $FORCEFSCK ]] || ([[ ! -f /fastboot ]] && ! in_array fastboot $(<
> > /proc/cmdline)); then
>
> command grouping here please, not a subshell.
>
> { [[ ! -f /fastboot ]] && ! in_array fastboot $(< /proc/cmdline); }
Agreed. Boolean expressions like this one make me remember why we're not
using bash syntax everywhere... I will fix this and resubmit the patch
as soon as I get a go-ahead from Tom (just to be sure there's nothing
else to fix).
>
> > + run_hook sysinit_prefsck
> > + if [[ -x $(type -P fsck) ]]; then
> > + stat_busy "Checking Filesystems"
> > + fsck_all >|"${FSCK_OUT:-/dev/stdout}"
> > 2>|"${FSCK_ERR:-/dev/stdout}"
> > + declare -r fsckret=$?
> > + (( fsckret <= 1 )) && stat_done || stat_fail
> > + else
> > + declare -r fsckret=0
> > + fi
> > + run_hook sysinit_postfsck
> > +
> > + # Single-user login and/or automatic reboot if needed
> > + fsck_reboot $fsckret
> > +fi
> >
> > status "Remounting Root" \
> > mount -o remount /
> > diff --git a/tmpfiles.conf b/tmpfiles.conf
> > index 8668116..a63ca07 100644
> > --- a/tmpfiles.conf
> > +++ b/tmpfiles.conf
> > @@ -17,4 +17,5 @@ r /tmp/.X[0-9]-lock
> > r /etc/nologin
> > r /etc/shutdownpid
> > r /forcefsck
> > +r /fastboot
> >
> > --
> > 1.7.8.4
> >
