[arch-projects] [initscripts] a bit of cleanup
Hi guys, I'm about to push thefollowing patches, so feedback welcome. In particular: 1) this might affect hooks. any known cases? imho they should be fixed 2) stole this from dave even though he did not submit it. the nerve! 3) this changes behavior, please raise any objections now 4) nothing interesting, move along -t
Avoid global variables, and make things clearer. Signed-off-by: Tom Gundersen <teg@jklm.no> --- functions | 6 ++++++ rc.sysinit | 29 ++++++++++++----------------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/functions b/functions index 0b7fd8d..747e42d 100644 --- a/functions +++ b/functions @@ -428,6 +428,12 @@ NETFS="nfs,nfs4,smbfs,cifs,codafs,ncpfs,shfs,fuse,fuseblk,glusterfs,davfs,fuse.g # Check local filesystems fsck_all() { + [[ -f /forcefsck ]] || in_array forcefsck $(< /proc/cmdline) && FORCEFSCK="-f" + + if [[ ! -n $FORCEFSCK ]] && { [[ -f /fastboot ]] || in_array fastboot $(< /proc/cmdline); }; then + return + fi + if [[ -e /run/initramfs/root-fsck ]]; then IGNORE_MOUNTED="-M" fi diff --git a/rc.sysinit b/rc.sysinit index 8fa4270..0876d05 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -176,24 +176,19 @@ if [[ -f /etc/crypttab && $CS ]] && grep -q ^[^#] /etc/crypttab; then fi # Check filesystems -[[ -f /forcefsck ]] || in_array forcefsck $(< /proc/cmdline) && FORCEFSCK="-f" -declare -r FORCEFSCK - -if [[ -n $FORCEFSCK ]] || { [[ ! -f /fastboot ]] && ! in_array fastboot $(< /proc/cmdline); }; then - 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 +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 status "Remounting Root" \ mount -o remount / -- 1.7.9.4
On Sat, Mar 17, 2012 at 11:44:27AM +0100, Tom Gundersen wrote:
Avoid global variables, and make things clearer.
Signed-off-by: Tom Gundersen <teg@jklm.no> --- functions | 6 ++++++ rc.sysinit | 29 ++++++++++++----------------- 2 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/functions b/functions index 0b7fd8d..747e42d 100644 --- a/functions +++ b/functions @@ -428,6 +428,12 @@ NETFS="nfs,nfs4,smbfs,cifs,codafs,ncpfs,shfs,fuse,fuseblk,glusterfs,davfs,fuse.g
# Check local filesystems fsck_all() { + [[ -f /forcefsck ]] || in_array forcefsck $(< /proc/cmdline) && FORCEFSCK="-f"
If you're trying to make things clearer, I'd probably convert this one into an if statement as well. The use of "||" and "&&" might be a bit confusing if you don't have a closer look here.
+ + if [[ ! -n $FORCEFSCK ]] && { [[ -f /fastboot ]] || in_array fastboot $(< /proc/cmdline); }; then + return
No return value? :) We use the return value of fsck_all() later so it might be worthwhile to explicitly return 0/1 here. Note that moving these checks into fsck_all() will also result in the "Checking Filesystems" being displayed even if the fastboot option is used (just saying, even though you were probably aware of that when writing this patch).
+ fi + if [[ -e /run/initramfs/root-fsck ]]; then IGNORE_MOUNTED="-M" fi diff --git a/rc.sysinit b/rc.sysinit index 8fa4270..0876d05 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -176,24 +176,19 @@ if [[ -f /etc/crypttab && $CS ]] && grep -q ^[^#] /etc/crypttab; then fi
# Check filesystems -[[ -f /forcefsck ]] || in_array forcefsck $(< /proc/cmdline) && FORCEFSCK="-f" -declare -r FORCEFSCK - -if [[ -n $FORCEFSCK ]] || { [[ ! -f /fastboot ]] && ! in_array fastboot $(< /proc/cmdline); }; then - 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 +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
status "Remounting Root" \ mount -o remount / -- 1.7.9.4
On Sat, Mar 17, 2012 at 6:01 PM, Lukas Fleischer <archlinux@cryptocrack.de> wrote:
On Sat, Mar 17, 2012 at 11:44:27AM +0100, Tom Gundersen wrote:
# Check local filesystems fsck_all() { + [[ -f /forcefsck ]] || in_array forcefsck $(< /proc/cmdline) && FORCEFSCK="-f"
If you're trying to make things clearer, I'd probably convert this one into an if statement as well. The use of "||" and "&&" might be a bit confusing if you don't have a closer look here.
I'm not too bothered about this, but next time someone touches that line we could change it into an "if".
+ + if [[ ! -n $FORCEFSCK ]] && { [[ -f /fastboot ]] || in_array fastboot $(< /proc/cmdline); }; then + return
No return value? :) We use the return value of fsck_all() later so it might be worthwhile to explicitly return 0/1 here.
Thanks! Fixed.
Note that moving these checks into fsck_all() will also result in the "Checking Filesystems" being displayed even if the fastboot option is used (just saying, even though you were probably aware of that when writing this patch).
Yeah, I thought that was ok. We might add some message saying that it is being skipped, but let's keep it like it is for now. -t
From: Dave Reisner <dreisner@archlinux.org> We haven't had the static binary in nearly 2 years, so simply call this without a PATH lookup. Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- functions | 11 ----------- rc.sysinit | 14 +++++++------- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/functions b/functions index 747e42d..129b3d4 100644 --- a/functions +++ b/functions @@ -379,17 +379,6 @@ activate_vgs() { (( $? == 0 )) && stat_done || stat_fail } -# Arch cryptsetup packages traditionally contained the binaries -# /usr/sbin/cryptsetup -# /sbin/cryptsetup.static -# By default, initscripts used the /sbin/cryptsetup.static. -# Newer packages will only have /sbin/cryptsetup and no static binary -# This ensures maximal compatibility with the old and new layout -for CS in /sbin/cryptsetup /usr/sbin/cryptsetup \ - /sbin/cryptsetup.static ''; do - [[ -x $CS ]] && break -done - read_crypttab() { # $1 = function to call with the split out line from the crypttab local line nspo failed=0 diff --git a/rc.sysinit b/rc.sysinit index 0876d05..ddac829 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -89,7 +89,7 @@ udevd_modprobe sysinit activate_vgs # Set up non-root encrypted partition mappings -if [[ -f /etc/crypttab && $CS ]] && grep -q ^[^#] /etc/crypttab; then +if [[ -f /etc/crypttab ]] && type -p cryptsetup >/dev/null; then stat_busy "Unlocking encrypted volumes:" modprobe -q dm-crypt 2>/dev/null do_unlock() { @@ -102,7 +102,7 @@ if [[ -f /etc/crypttab && $CS ]] && grep -q ^[^#] /etc/crypttab; then # Ordering of options is different if you are using LUKS vs. not. # Use ugly swizzling to deal with it. # isLuks only gives an exit code but no output to stdout or stderr. - if $CS isLuks "$2" 2>/dev/null; then + if cryptsetup isLuks "$2" 2>/dev/null; then open=luksOpen a=$2 b=$1 @@ -125,13 +125,13 @@ if [[ -f /etc/crypttab && $CS ]] && grep -q ^[^#] /etc/crypttab; then fi if (( _overwriteokay == 0 )); then false - elif $CS -d /dev/urandom $4 $open "$a" "$b" >/dev/null; then + elif cryptsetup -d /dev/urandom $4 $open "$a" "$b" >/dev/null; then stat_append "creating swapspace.." mkswap -f -L $1 /dev/mapper/$1 >/dev/null fi;; ASK) printf "\nOpening '$1' volume:\n" - $CS $4 $open "$a" "$b" < /dev/console;; + cryptsetup $4 $open "$a" "$b" < /dev/console;; /dev*) local ckdev=${3%%:*} local cka=${3#*:} @@ -153,13 +153,13 @@ if [[ -f /etc/crypttab && $CS ]] && grep -q ^[^#] /etc/crypttab; then # cka is numeric: cka=offset, ckb=length dd if=${ckdev} of=${ckfile} bs=1 skip=${cka} count=${ckb} >/dev/null 2>&1;; esac - $CS -d ${ckfile} $4 $open "$a" "$b" >/dev/null + cryptsetup -d ${ckfile} $4 $open "$a" "$b" >/dev/null dd if=/dev/urandom of=${ckfile} bs=1 count=$(stat -c %s ${ckfile}) conv=notrunc >/dev/null 2>&1 rm ${ckfile};; /*) - $CS -d "$3" $4 $open "$a" "$b" >/dev/null;; + cryptsetup -d "$3" $4 $open "$a" "$b" >/dev/null;; *) - echo "$3" | $CS $4 $open "$a" "$b" >/dev/null;; + echo "$3" | cryptsetup $4 $open "$a" "$b" >/dev/null;; esac if (( $? )); then failed=1 -- 1.7.9.4
dm-mod and dm-crypt will be autoloaded when needed (at least on kernels newer than 2.6.36). The rtc modules should be compiled in, as they have been in the official kernels for some time. Signed-off-by: Tom Gundersen <teg@jklm.no> --- functions | 2 -- rc.sysinit | 3 --- 2 files changed, 5 deletions(-) diff --git a/functions b/functions index 129b3d4..f5da93c 100644 --- a/functions +++ b/functions @@ -372,9 +372,7 @@ udevd_modprobe() { activate_vgs() { [[ $USELVM = [yY][eE][sS] && -x $(type -P lvm) && -d /sys/block ]] || return 0 - # Kernel 2.6.x, LVM2 groups stat_busy "Activating LVM2 groups" - modprobe -q dm-mod 2>/dev/null vgchange --sysinit -a y >/dev/null (( $? == 0 )) && stat_done || stat_fail } diff --git a/rc.sysinit b/rc.sysinit index ddac829..06b4bda 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -54,8 +54,6 @@ esac if [[ $HWCLOCK_PARAMS ]]; then stat_busy "Adjusting system time and setting kernel timezone" - # enable rtc access - modprobe -q -a rtc-cmos rtc genrtc # Adjust the system time for timezone offset if rtc is not in UTC # 1. Make creation time on device nodes sane (FS#8665) @@ -91,7 +89,6 @@ activate_vgs # Set up non-root encrypted partition mappings if [[ -f /etc/crypttab ]] && type -p cryptsetup >/dev/null; then stat_busy "Unlocking encrypted volumes:" - modprobe -q dm-crypt 2>/dev/null do_unlock() { # $1 = requested name # $2 = source device -- 1.7.9.4
No functional change, just improve readability. Signed-off-by: Tom Gundersen <teg@jklm.no> --- functions | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ rc.sysinit | 77 ----------------------------------------------------------- 2 files changed, 78 insertions(+), 77 deletions(-) diff --git a/functions b/functions index f5da93c..11ab11f 100644 --- a/functions +++ b/functions @@ -377,6 +377,84 @@ activate_vgs() { (( $? == 0 )) && stat_done || stat_fail } +do_unlock() { + # $1 = requested name + # $2 = source device + # $3 = password + # $4 = options + stat_append "${1}.." + local open=create a=$1 b=$2 failed=0 + # Ordering of options is different if you are using LUKS vs. not. + # Use ugly swizzling to deal with it. + # isLuks only gives an exit code but no output to stdout or stderr. + if cryptsetup isLuks "$2" 2>/dev/null; then + open=luksOpen + a=$2 + b=$1 + fi + case $3 in + SWAP) + local _overwriteokay=0 + if [[ -b $2 && -r $2 ]]; then + # This is DANGEROUS! If there is any known file system, + # partition table, RAID or LVM volume on the device + # we don't overwrite it. + # + # 'blkid' returns 2 if no valid signature has been found. + # Only in this case we should allow overwriting the device. + # + # This sanity check _should_ be sufficient, but it might not. + # This may cause dataloss if it is not used carefully. + blkid -p "$2" &>/dev/null + (( $? == 2 )) && _overwriteokay=1 + fi + if (( _overwriteokay == 0 )); then + false + elif cryptsetup -d /dev/urandom $4 $open "$a" "$b" >/dev/null; then + stat_append "creating swapspace.." + mkswap -f -L $1 /dev/mapper/$1 >/dev/null + fi;; + ASK) + printf "\nOpening '$1' volume:\n" + cryptsetup $4 $open "$a" "$b" < /dev/console;; + /dev*) + local ckdev=${3%%:*} + local cka=${3#*:} + local ckb=${cka#*:} + local cka=${cka%:*} + local ckfile=/dev/ckfile + local ckdir=/dev/ckdir + case ${cka} in + *[!0-9]*) + # Use a file on the device + # cka is not numeric: cka=filesystem, ckb=path + mkdir ${ckdir} + mount -r -t ${cka} ${ckdev} ${ckdir} + dd if=${ckdir}/${ckb} of=${ckfile} >/dev/null 2>&1 + umount ${ckdir} + rmdir ${ckdir};; + *) + # Read raw data from the block device + # cka is numeric: cka=offset, ckb=length + dd if=${ckdev} of=${ckfile} bs=1 skip=${cka} count=${ckb} >/dev/null 2>&1;; + esac + cryptsetup -d ${ckfile} $4 $open "$a" "$b" >/dev/null + dd if=/dev/urandom of=${ckfile} bs=1 count=$(stat -c %s ${ckfile}) conv=notrunc >/dev/null 2>&1 + rm ${ckfile};; + /*) + cryptsetup -d "$3" $4 $open "$a" "$b" >/dev/null;; + *) + echo "$3" | cryptsetup $4 $open "$a" "$b" >/dev/null;; + esac + if (( $? )); then + failed=1 + stat_append "failed " + else + stat_append "ok " + fi + return $failed +} + read_crypttab() { # $1 = function to call with the split out line from the crypttab local line nspo failed=0 diff --git a/rc.sysinit b/rc.sysinit index 06b4bda..3528bb2 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -89,83 +89,6 @@ activate_vgs # Set up non-root encrypted partition mappings if [[ -f /etc/crypttab ]] && type -p cryptsetup >/dev/null; then stat_busy "Unlocking encrypted volumes:" - do_unlock() { - # $1 = requested name - # $2 = source device - # $3 = password - # $4 = options - stat_append "${1}.." - local open=create a=$1 b=$2 failed=0 - # Ordering of options is different if you are using LUKS vs. not. - # Use ugly swizzling to deal with it. - # isLuks only gives an exit code but no output to stdout or stderr. - if cryptsetup isLuks "$2" 2>/dev/null; then - open=luksOpen - a=$2 - b=$1 - fi - case $3 in - SWAP) - local _overwriteokay=0 - if [[ -b $2 && -r $2 ]]; then - # This is DANGEROUS! If there is any known file system, - # partition table, RAID or LVM volume on the device - # we don't overwrite it. - # - # 'blkid' returns 2 if no valid signature has been found. - # Only in this case we should allow overwriting the device. - # - # This sanity check _should_ be sufficient, but it might not. - # This may cause dataloss if it is not used carefully. - blkid -p "$2" &>/dev/null - (( $? == 2 )) && _overwriteokay=1 - fi - if (( _overwriteokay == 0 )); then - false - elif cryptsetup -d /dev/urandom $4 $open "$a" "$b" >/dev/null; then - stat_append "creating swapspace.." - mkswap -f -L $1 /dev/mapper/$1 >/dev/null - fi;; - ASK) - printf "\nOpening '$1' volume:\n" - cryptsetup $4 $open "$a" "$b" < /dev/console;; - /dev*) - local ckdev=${3%%:*} - local cka=${3#*:} - local ckb=${cka#*:} - local cka=${cka%:*} - local ckfile=/dev/ckfile - local ckdir=/dev/ckdir - case ${cka} in - *[!0-9]*) - # Use a file on the device - # cka is not numeric: cka=filesystem, ckb=path - mkdir ${ckdir} - mount -r -t ${cka} ${ckdev} ${ckdir} - dd if=${ckdir}/${ckb} of=${ckfile} >/dev/null 2>&1 - umount ${ckdir} - rmdir ${ckdir};; - *) - # Read raw data from the block device - # cka is numeric: cka=offset, ckb=length - dd if=${ckdev} of=${ckfile} bs=1 skip=${cka} count=${ckb} >/dev/null 2>&1;; - esac - cryptsetup -d ${ckfile} $4 $open "$a" "$b" >/dev/null - dd if=/dev/urandom of=${ckfile} bs=1 count=$(stat -c %s ${ckfile}) conv=notrunc >/dev/null 2>&1 - rm ${ckfile};; - /*) - cryptsetup -d "$3" $4 $open "$a" "$b" >/dev/null;; - *) - echo "$3" | cryptsetup $4 $open "$a" "$b" >/dev/null;; - esac - if (( $? )); then - failed=1 - stat_append "failed " - else - stat_append "ok " - fi - return $failed - } crypto_unlocked=0 read_crypttab do_unlock && stat_done || stat_fail # Maybe someone has LVM on an encrypted block device -- 1.7.9.4
Am 17.03.2012 11:44, schrieb Tom Gundersen:
In particular: 1) this might affect hooks. any known cases? imho they should be fixed
Main users of hooks are the *splash people. And archlive/larch might use it too (I think the fsck hooks were for archlive originally).
2) stole this from dave even though he did not submit it. the nerve!
Good idea - we used to have quite a funny switcheroo with cryptsetup was that two years ago already?
participants (3)
-
Lukas Fleischer
-
Thomas Bächler
-
Tom Gundersen