On Wed, 2010-06-30 at 23:57 +0200, Thomas Bächler wrote:

Nice patch, but ...

...

-if [ -t 1 ]; then +if [[ -t 1 ]]; then SAVE_POSITION="\033[s" RESTORE_POSITION="\033[u" DEL_TEXT="\033[$(($STAT_COL+4))G" @@ -116,10 +116,8 @@ stat_fail() {

Hmm, this definitely doesn't belong into this patch.

Yeah, I think it got smashed together while I was rebasing things to clean up.

...

}

stat_die() { - retval=1 - [ "$1" = "" ] || retval=$1 stat_fail - exit $retval + exit ${1:-1} }

status() {

Okay, this is certainly nicer than before.

-- Victor Lowther LPIC2 UCP RHCE

Am 30.06.2010 23:47, schrieb Victor Lowther:

Go ahead and declare add_hook and run_hook as readonly functions to prevent hooks from overwriting them. Too bad bash does not have lexically scoped variables -- if it does, we could do the same with the hook_funcs associative array.

Lots of hunks below that have nothing to do with the actual change made here:

diff --git a/functions b/functions index 8bbdfc6..d9f55fa 100644 --- a/functions +++ b/functions @@ -148,38 +148,39 @@ in_array() { # daemons:

add_daemon() { - [ -d /var/run/daemons ] || /bin/mkdir -p /var/run/daemons - /bin/touch /var/run/daemons/$1 + [[ -d /var/run/daemons ]] || /bin/mkdir -p /var/run/daemons + > /var/run/daemons/"$1" }

rm_daemon() { - /bin/rm -f /var/run/daemons/$1 + /bin/rm -f /var/run/daemons/"$1" }

ck_daemon() { - [ -f /var/run/daemons/$1 ] && return 1 - return 0 + [[ ! -f /var/run/daemons/$1 ]] }

-ck_depends() { - for daemon in $@; do - if ck_daemon $daemon; then - /etc/rc.d/$daemon start - fi - done +have_daemon() { + [[ -x /etc/rc.d/"$1" ]] }

start_daemon() { - /etc/rc.d/$1 start + have_daemon "$1" && /etc/rc.d/"$1" start +} + +ck_depends() { + for daemon in "$@"; do + ck_daemon "$daemon" && start_daemon "$daemon" + done }

start_daemon_bkgd() { stat_bkgd "Starting $1" - (/etc/rc.d/$1 start) &>/dev/null & + have_daemon "$1" && (start_daemon "$1") &>/dev/null & }

stop_daemon() { - /etc/rc.d/$1 stop + have_daemon "$1" && /etc/rc.d/"$1" stop }

Here the actual patch starts, the above doesn't belong into this patch. You should _really_ check out git add -p/-i

# Status functions @@ -234,24 +235,26 @@ ck_status() { # single_postkillall: after all processes have been killed in rc.single # shutdown_poweroff: directly before powering off in rc.shutdown # -# Make sure to never override the add_hook and run_hook functions via functions.d +# Declare add_hook and run_hook as read-only to prevent overwriting them. +# Too bad we cannot do the same thing with hook_funcs

declare -A hook_funcs

add_hook() { - [ -z "$1" -o -z "$2" ] && return 1 - hook_funcs["$1"]="${hook_funcs["$1"]} $2" + [[ $1 && $2 ]] || return 1 + hook_funcs["$1"]+=" $2" }

+= in bash? Really? Didn't know that worked.

run_hook() { - local func - - [ -z "$1" ] && return 1 - for func in ${hook_funcs["$1"]}; do - ${func} - done + [[ $1 ]] || return 1 + local func + for func in ${hook_funcs["$1"]}; do + "${func}" + done }

+declare -r add_hook run_hook +

Nice, I didn't know you could do that.

# Function for setting console font if required set_consolefont() { if [ -n "$CONSOLEFONT" ]; then

Am 2010-06-30 23:47, schrieb Victor Lowther:

Move that shared code into functions. --- functions | 29 +++++++++++++++++++++++++++++ rc.shutdown | 32 +------------------------------- rc.single | 27 +-------------------------- 3 files changed, 31 insertions(+), 57 deletions(-)

diff --git a/functions b/functions index d8e8e54..bf6ed45 100644 --- a/functions +++ b/functions @@ -203,6 +203,35 @@ ck_status() { fi }

+kill_everything() { + # Find daemons NOT in the DAEMONS array. Shut these down first + for daemon in /var/run/daemons/*; do + [[ -f $daemon ]] || continue + daemon=${daemon##*/} + in_array "$daemon" "${DAEMONS[@]}" || stop_daemon "$daemon" + done + + # Shutdown daemons in reverse order + for ((i=${#DAEMONS[@]}-1; i>=0; i--)); do + [[ ${DAEMONS[$i]:0:1} = '!' ]]&& continue + ck_daemon ${DAEMONS[$i]#@} || stop_daemon ${DAEMONS[$i]#@} + done + + # Terminate all processes + stat_busy "Sending SIGTERM To Processes" + run_hook single_prekillall

This line should be run_hook "${0##*/rc.}"_prekillall IMHO

+ /sbin/killall5 -15&> /dev/null + /bin/sleep 5 + stat_done + + stat_busy "Sending SIGKILL To Processes" + /sbin/killall5 -9&> /dev/null + /bin/sleep 1 + stat_done + + run_hook single_postkillall

Similar as above.

+} + ############################### # Custom hooks in initscripts # ############################### diff --git a/rc.shutdown b/rc.shutdown index 002a45d..ef9b16d 100755 --- a/rc.shutdown +++ b/rc.shutdown @@ -19,37 +19,7 @@ echo " " [[ -x /bin/domainname ]]&& /bin/domainname "" [[ -x /etc/rc.local.shutdown ]]&& /etc/rc.local.shutdown

- -# Find daemons NOT in the DAEMONS array. Shut these down first -if [ -d /var/run/daemons ]; then - for daemon in $(/bin/ls -1t /var/run/daemons); do - if ! in_array $daemon ${DAEMONS[@]}; then - stop_daemon $daemon - fi - done -fi -# Shutdown daemons in reverse order -let i=${#DAEMONS[@]}-1 -while [ $i -ge 0 ]; do - if [ "${DAEMONS[$i]:0:1}" != '!' ]; then - ck_daemon ${DAEMONS[$i]#@} || stop_daemon ${DAEMONS[$i]#@} - fi - let i=i-1 -done - -# Terminate all processes -stat_busy "Sending SIGTERM To Processes" -run_hook shutdown_prekillall -/sbin/killall5 -15&> /dev/null -/bin/sleep 5 -stat_done - -stat_busy "Sending SIGKILL To Processes" -/sbin/killall5 -9&> /dev/null -/bin/sleep 1 -stat_done - -run_hook shutdown_postkillall +kill_everything

stat_busy "Saving Random Seed" RANDOM_SEED=/var/lib/misc/random-seed diff --git a/rc.single b/rc.single index aa27be0..a84ece8 100755 --- a/rc.single +++ b/rc.single @@ -9,33 +9,8 @@ run_hook single_start

if [[ $PREVLEVEL != N ]]; then - # Find daemons NOT in the DAEMONS array. Shut these down first - for daemon in /var/run/daemons/*; do - [[ -f $daemon ]] || continue - daemon=${daemon##*/} - in_array "$daemon" "${DAEMONS[@]}" || stop_daemon "$daemon" - done - - # Shutdown daemons in reverse order - for ((i=${#DAEMONS[@]}-1; i>=0; i--)); do - [[ ${DAEMONS[$i]:0:1} = '!' ]]&& continue - ck_daemon ${DAEMONS[$i]#@} || stop_daemon ${DAEMONS[$i]#@} - done - - # Terminate all processes - stat_busy "Sending SIGTERM To Processes" - run_hook single_prekillall - /sbin/killall5 -15&> /dev/null - /bin/sleep 5 - stat_done - - stat_busy "Sending SIGKILL To Processes" - /sbin/killall5 -9&> /dev/null - /bin/sleep 1 - stat_done - - run_hook single_postkillall

+ kill_everything stat_busy "Starting UDev Daemon" /sbin/udevd --daemon stat_done

2010-07-02 11:27, Thomas Bächler:

First of all, sorry for not continuing the review yesterday, time is short :(

Still, I'll finish this, as I'd like to have this applied eventually.

Am 02.07.2010 11:21, schrieb Kurt J. Bosch:

...

Am 2010-06-30 23:47, schrieb Victor Lowther:

...

Move that shared code into functions. + run_hook single_prekillall

This line should be run_hook "${0##*/rc.}"_prekillall IMHO

Kurt is right here. We call this code from rc.single and rc.shutdown I think. We use two distinct hooks, you can register functions for these hooks independently!

Hmm, git bashification-redux has now: kill_everything() { # $1 = where we are being called from. # This is used to determine which hooks to run. # Find daemons NOT in the DAEMONS array. Shut these down first for daemon in /var/run/daemons/*; do [[ -f $daemon ]] || continue daemon=${daemon##*/} in_array "$daemon" "${DAEMONS[@]}" || stop_daemon "$daemon" done # Shutdown daemons in reverse order for ((i=${#DAEMONS[@]}-1; i>=0; i--)); do [[ ${DAEMONS[$i]:0:1} = '!' ]] && continue ck_daemon ${DAEMONS[$i]#@} || stop_daemon ${DAEMONS[$i]#@} done # Terminate all processes stat_busy "Sending SIGTERM To Processes" run_hook "$1_prekillall" /sbin/killall5 -15 &> /dev/null /bin/sleep 5 stat_done stat_busy "Sending SIGKILL To Processes" /sbin/killall5 -9 &> /dev/null /bin/sleep 1 stat_done run_hook "$1_postkillall" } I suggest: From b202be97f8dc1c0c68aaea792d4457c674c673f3 Mon Sep 17 00:00:00 2001 From: Kurt J. Bosch Date: Tue, 31 Aug 2010 09:57:47 +0200 Subject: [PATCH 17/17] Correct behaviour of kill_everything() --- functions | 11 +++++------ 1 files changed, 5 insertions(+), 6 deletions(-) diff --git a/functions b/functions index b9ba718..3ca7324 100644 --- a/functions +++ b/functions @@ -205,10 +205,9 @@ ck_status() { kill_everything() { # $1 = where we are being called from. # This is used to determine which hooks to run. - # Find daemons NOT in the DAEMONS array. Shut these down first - for daemon in /var/run/daemons/*; do - [[ -f $daemon ]] || continue - daemon=${daemon##*/} + # Find daemons NOT in the DAEMONS array. + # Shut these down first in reverse order. + for daemon in $( /bin/ls -t /var/run/daemons ); do in_array "$daemon" "${DAEMONS[@]}" || stop_daemon "$daemon" done @@ -220,7 +219,7 @@ kill_everything() { # Terminate all processes stat_busy "Sending SIGTERM To Processes" - run_hook "$1_prekillall" + run_hook "${1}_prekillall" /sbin/killall5 -15 &> /dev/null /bin/sleep 5 stat_done @@ -230,7 +229,7 @@ kill_everything() { /bin/sleep 1 stat_done - run_hook "$1_postkillall" + run_hook "${1}_postkillall" } activate_vgs() { -- 1.7.0.3

2010-08-31 13:16, Dave Reisner:

On Tue, Aug 31, 2010 at 10:07:52AM +0200, Kurt J. Bosch wrote:

...

--snip--

I suggest:

From b202be97f8dc1c0c68aaea792d4457c674c673f3 Mon Sep 17 00:00:00 2001 From: Kurt J. Bosch Date: Tue, 31 Aug 2010 09:57:47 +0200 Subject: [PATCH 17/17] Correct behaviour of kill_everything()

--- functions | 11 +++++------ 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/functions b/functions index b9ba718..3ca7324 100644 --- a/functions +++ b/functions @@ -205,10 +205,9 @@ ck_status() { kill_everything() { # $1 = where we are being called from. # This is used to determine which hooks to run. - # Find daemons NOT in the DAEMONS array. Shut these down first - for daemon in /var/run/daemons/*; do - [[ -f $daemon ]] || continue - daemon=${daemon##*/} + # Find daemons NOT in the DAEMONS array. + # Shut these down first in reverse order. + for daemon in $( /bin/ls -t /var/run/daemons ); do in_array "$daemon" "${DAEMONS[@]}" || stop_daemon "$daemon" done

@@ -220,7 +219,7 @@ kill_everything() {

# Terminate all processes stat_busy "Sending SIGTERM To Processes" - run_hook "$1_prekillall" + run_hook "${1}_prekillall" /sbin/killall5 -15&> /dev/null /bin/sleep 5 stat_done @@ -230,7 +229,7 @@ kill_everything() { /bin/sleep 1 stat_done

- run_hook "$1_postkillall" + run_hook "${1}_postkillall" }

activate_vgs() { -- 1.7.0.3

Parsing the output of ls will never be better than using shell globbing no matter how much simpler it might make the code appear to be. Not only are you avoiding a fork, but the shell glob will properly handle any odd characters thrown into the mix. You'll see breakage on something as simple as a space in your suggestion. While I'm inclined to believe that there will never be a space in the name of a daemon in Arch, if we're going for pure Bash in this rewrite, let's use Bash instead of mindlessly forking.

NAK. The patch just reverts breaking of the current behaviour which is shutdown daemons in reverse order of start. AFAIK bash globbing is unable to sort on timestamps, right? p

On Wed, Sep 1, 2010 at 5:41 AM, Kurt J. Bosch wrote:

2010-08-31 13:16, Dave Reisner:

...

On Tue, Aug 31, 2010 at 10:07:52AM +0200, Kurt J. Bosch wrote:

...

--snip--

I suggest:

 From b202be97f8dc1c0c68aaea792d4457c674c673f3 Mon Sep 17 00:00:00 2001 From: Kurt J. Bosch Date: Tue, 31 Aug 2010 09:57:47 +0200 Subject: [PATCH 17/17] Correct behaviour of kill_everything()

---  functions |   11 +++++------  1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/functions b/functions index b9ba718..3ca7324 100644 --- a/functions +++ b/functions @@ -205,10 +205,9 @@ ck_status() {  kill_everything() {      # $1 = where we are being called from.      # This is used to determine which hooks to run. -    # Find daemons NOT in the DAEMONS array. Shut these down first -    for daemon in /var/run/daemons/*; do -        [[ -f $daemon ]] || continue -        daemon=${daemon##*/} +    # Find daemons NOT in the DAEMONS array. +    # Shut these down first in reverse order. +    for daemon in $( /bin/ls -t /var/run/daemons ); do        in_array "$daemon" "${DAEMONS[@]}" || stop_daemon "$daemon"      done

@@ -220,7 +219,7 @@ kill_everything() {

       # Terminate all processes      stat_busy "Sending SIGTERM To Processes" -    run_hook "$1_prekillall" +    run_hook "${1}_prekillall"      /sbin/killall5 -15&>  /dev/null      /bin/sleep 5      stat_done @@ -230,7 +229,7 @@ kill_everything() {      /bin/sleep 1      stat_done

-    run_hook "$1_postkillall" +    run_hook "${1}_postkillall"  }

 activate_vgs() { -- 1.7.0.3

Parsing the output of ls will never be better than using shell globbing no matter how much simpler it might make the code appear to be. Not only are you avoiding a fork, but the shell glob will properly handle any odd characters thrown into the mix. You'll see breakage on something as simple as a space in your suggestion. While I'm inclined to believe that there will never be a space in the name of a daemon in Arch, if we're going for pure Bash in this rewrite, let's use Bash instead of mindlessly forking.

NAK. The patch just reverts breaking of the current behaviour which is shutdown daemons in reverse order of start. AFAIK bash globbing is unable to sort on timestamps, right?

Bash globbing cannot handle it, but bash knows how to test file timestamps, so we could code something up that handles killing the daemons in reverse timestamp order if we had to. However, there is no reason to think that the order is significant for daemons not in $DAEMONS. If there are any dependencies that this code does not handle gracefully, the offending daemons will be killed when we kill -15 and then kill -9 the rest of the processes. If a daemon does not handle SIGTERM gracefully because of dependency issues, it has other, more significant problems.

I am missing the difference. Diff please? Sent from my Nexus One On Aug 18, 2010 2:41 AM, "Kurt J. Bosch" wrote:

Am 2010-06-30 23:47, schrieb Victor Lowther:

...

Trying to stick with POSIX syntax only just slows things down. --- rc.sysinit | 27 +++++++++++---------------- 1 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/rc.sysinit b/rc.sysinit index 29adeca..f3e60b7 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -44,27 +44,22 @@ fi

HWCLOCK_PARAMS="--hctosys" case $HARDWARECLOCK in - UTC) "$HWCLOCK_PARAMS --utc";; - localtime) HWCLOCK_PARAMS="$HWCLOCK_PARAMS --localtime";; + UTC) HWCLOCK_PARAMS+=" --utc";; + localtime) HWCLOCK_PARAMS+=" --localtime";; *) HWCLOCK_PARAMS="";; esac

-if [ -n "$HWCLOCK_PARAMS" ]; then +if [[ $HWCLOCK_PARAMS ]]; then # enable rtc access - /sbin/modprobe -q rtc-cmos - # some custom kernels use rtc/genrtc, try to load those too - /sbin/modprobe -q rtc - /sbin/modprobe -q genrtc + /sbin/modprobe -q -a rtc-cmos rtc genrtc # If devtmpfs is used, the required RTC device already exists now # Otherwise, create whatever device is available - if [ ! -c /dev/rtc -a ! -c /dev/rtc0 ]; then - if [ -f /sys/class/rtc/rtc0/dev ]; then - IFS=: read -r major minor< /sys/class/rtc/rtc0/dev - /bin/mknod /dev/rtc0 c $major $minor - elif [ -f /sys/class/misc/rtc/dev ]; then - IFS=: read -r major minor< /sys/class/misc/rtc/dev - /bin/mknod /dev/rtc c $major $minor - fi + if ! [[ -c /dev/rtc || -c /dev/rtc0 ]]; then + for dev in /sys/class/rtc/rtc0/dev /sys/class/misc/rtc/dev; do + [[ -e $dev ]] || continue + IFS=: read -r major minor< "$dev" + /bin/mknod /dev/rtc c $major $minor + done fi

Actually this doesn't do the same. What about the following?

if ! [[ -c /dev/rtc || -c /dev/rtc0 ]]; then for dev in /sys/class/rtc/rtc0 /sys/class/misc/rtc; do [[ -e $dev/dev ]] || continue IFS=: read -r major minor< "$dev/dev" /bin/mknod /dev/${dev##*/} c $major $minor done fi

Am 2010-08-19 10:19, schrieb Jan de Groot:

On Thu, 2010-08-19 at 00:56 -0400, Dave Reisner wrote:

...

Couldn't we avoid all this by just flipping a switch in the kernel?

CONFIG_RTC_DRV_CMOS=y

If it's compiled into the kernel, udev picks it up and creates the /dev nodes for us.

Which still locks out the people who use a custom kernel with this driver compiled as module. IMHO the init scripts should work with both module and built-in.

So why not let udev do the job? Patch below. I modified my initcpio to get rid of the devtmpfs. A ls -l /dev/rtc* added between the new /sbin/udevadm settle and the initial clock setting showed /dev/rtc0 and /dev/rtc -> rtc0. So I think this should work. I did *not* move the sysinit_udevlaunched hook together with udev start to avoid insane creation times of dev nodes because this hook is used for some early udev triggering in fbsplash-extras (AUR). Patch against bashification-redux: From 22d410a2566964d58752d443a1312a6eb552660a Mon Sep 17 00:00:00 2001 From: Kurt J. Bosch Date: Thu, 19 Aug 2010 16:46:23 +0200 Subject: [PATCH 17/17] Correct rtc dev nodes creation using udev --- rc.sysinit | 22 +++++++--------------- 1 files changed, 7 insertions(+), 15 deletions(-) diff --git a/rc.sysinit b/rc.sysinit index 4421def..2415967 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -46,6 +46,12 @@ else /bin/dmesg -n 3 fi +echo > /proc/sys/kernel/hotplug + +stat_busy "Starting UDev Daemon" +/sbin/udevd --daemon +stat_done + HWCLOCK_PARAMS="--hctosys" case $HARDWARECLOCK in UTC) HWCLOCK_PARAMS+=" --utc";; @@ -56,15 +62,7 @@ esac if [[ $HWCLOCK_PARAMS ]]; then # enable rtc access /sbin/modprobe -q -a rtc-cmos rtc genrtc - # If devtmpfs is used, the required RTC device already exists now - # Otherwise, create whatever device is available - if ! [[ -c /dev/rtc || -c /dev/rtc0 ]]; then - for dev in /sys/class/rtc/rtc0/dev /sys/class/misc/rtc/dev; do - [[ -e $dev ]] || continue - IFS=: read -r major minor < "$dev" - /bin/mknod /dev/rtc c $major $minor - done - fi + /sbin/udevadm settle # Do a clock set here for a few reasons: # 1. Make creation time on udev nodes sane (FS#8665) @@ -79,12 +77,6 @@ if [[ $HWCLOCK_PARAMS ]]; then fi fi -echo > /proc/sys/kernel/hotplug - -stat_busy "Starting UDev Daemon" -/sbin/udevd --daemon -stat_done - run_hook sysinit_udevlaunched # Trigger udev uevents -- 1.7.2.1

On 2010-08-19 23:12, Victor Lowther wrote:

On Aug 19, 2010 10:08 AM, "Kurt J. Bosch" wrote:

...

On 2010-08-19 10:19, Jan de Groot wrote:

...

On Thu, 2010-08-19 at 00:56 -0400, Dave Reisner wrote:

...

Couldn't we avoid all this by just flipping a switch in the kernel?

CONFIG_RTC_DRV_CMOS=y

If it's compiled into the kernel, udev picks it up and creates the /dev nodes for us.

Which still locks out the people who use a custom kernel with this driver compiled as module. IMHO the init scripts should work with both module and built-in.

So why not let udev do the job? Patch below. I modified my initcpio to get rid of the devtmpfs. A ls -l /dev/rtc* added between the new /sbin/udevadm settle and the initial clock setting showed /dev/rtc0 and /dev/rtc -> rtc0. So I think this should work.

I did *not* move the sysinit_udevlaunched hook together with udev start to avoid insane creation times of dev nodes because this hook is used for some early udev triggering in fbsplash-extras (AUR).

Patch against bashification-redux:

From 22d410a2566964d58752d443a1312a6eb552660a Mon Sep 17 00:00:00 2001 From: Kurt J. Bosch Date: Thu, 19 Aug 2010 16:46:23 +0200 Subject: [PATCH 17/17] Correct rtc dev nodes creation using udev

--- rc.sysinit | 22 +++++++--------------- 1 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/rc.sysinit b/rc.sysinit index 4421def..2415967 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -46,6 +46,12 @@ else /bin/dmesg -n 3 fi

+echo> /proc/sys/kernel/hotplug + +stat_busy "Starting UDev Daemon" +/sbin/udevd --daemon +stat_done + HWCLOCK_PARAMS="--hctosys" case $HARDWARECLOCK in UTC) HWCLOCK_PARAMS+=" --utc";; @@ -56,15 +62,7 @@ esac if [[ $HWCLOCK_PARAMS ]]; then # enable rtc access /sbin/modprobe -q -a rtc-cmos rtc genrtc - # If devtmpfs is used, the required RTC device already exists now - # Otherwise, create whatever device is available - if ! [[ -c /dev/rtc || -c /dev/rtc0 ]]; then - for dev in /sys/class/rtc/rtc0/dev /sys/class/misc/rtc/dev; do - [[ -e $dev ]] || continue - IFS=: read -r major minor< "$dev" - /bin/mknod /dev/rtc c $major $minor - done - fi + /sbin/udevadm settle

# Do a clock set here for a few reasons: # 1. Make creation time on udev nodes sane (FS#8665) @@ -79,12 +77,6 @@ if [[ $HWCLOCK_PARAMS ]]; then fi fi

-echo> /proc/sys/kernel/hotplug - -stat_busy "Starting UDev Daemon" -/sbin/udevd --daemon -stat_done - run_hook sysinit_udevlaunched

# Trigger udev uevents -- 1.7.2.1

I like this one the best.

Sent from my Nexus One. Sorry for top posting.

Corrected Hmm, the patch might break things with custom non modular kernels without devtmpfs though. No modules loading no uevents and no dev nodes. On the other hand reading the hwclock man page, I'm a bit uncertain whether that piece will ever use /dev/rtc0 without a symlink from /dev/rtc. Instead it might actually fall back to direct I/O with the current code and with the patched one too. Moreover I found a couple of interesting things: * The system clock is already set (by the kernel - in spite of CONFIG_RTC_HCTOSYS not set) even without doing anything. Weird, but with UTC hardware clock I even get correct mtimes in early user space. * man hwclock:

−−systz Reset the System Time based on the current timezone. ... This is an alternate option to −−hctosys that does not read the hardware clock, and may be used in system startup scripts for recent 2.6 kernels where you know the System Time contains the Hardware Clock time. I switched my hardware clock to localtime, reverted my patch and modified rc.sysinit to use −systz instead of --hctosys for the first hwclock call and also disabled the entire rtc modprobe and mknod code. A date command inserted for testing spit out the correct time. From the util-linux-ng list: Indeed, we may be running hwclock --systz before /dev is mounted. Getting rid of the pre fsck hwclock --hctosys would also on the average save another half second of boot up time beside removing some ugly code.

AFAIKS we can go one of three ways: * Use udev expecting CONFIG_RTC_DRV_CMOS=m or fall back to direct I/O * Use −−systz, drop the ugly code and expect some help from the kernel * Use an even more ugly code creating the missing symlink: Corrected non udev patch against bashification-redux: From 4e5d6a763af8f45c63ad53aa99b82246f90c0b43 Mon Sep 17 00:00:00 2001 From: Kurt J. Bosch Date: Fri, 20 Aug 2010 09:04:46 +0200 Subject: [PATCH 17/17] Really correct rtc dev nodes creation --- rc.sysinit | 15 +++++++++------ 1 files changed, 9 insertions(+), 6 deletions(-) diff --git a/rc.sysinit b/rc.sysinit index 4421def..1ff37cc 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -58,12 +58,15 @@ if [[ $HWCLOCK_PARAMS ]]; then /sbin/modprobe -q -a rtc-cmos rtc genrtc # If devtmpfs is used, the required RTC device already exists now # Otherwise, create whatever device is available - if ! [[ -c /dev/rtc || -c /dev/rtc0 ]]; then - for dev in /sys/class/rtc/rtc0/dev /sys/class/misc/rtc/dev; do - [[ -e $dev ]] || continue - IFS=: read -r major minor < "$dev" - /bin/mknod /dev/rtc c $major $minor - done + if ! [[ -c /dev/rtc ]]; then + for dev in /sys/class/misc/rtc /sys/class/rtc/rtc0; do + [[ -e $dev/dev ]] || continue + IFS=: read -r major minor < "$dev/dev" + node=${dev##*/} + /bin/mknod /dev/$node c $major $minor + [[ $node = rtc ]] || /bin/ln -s $node /dev/rtc + break + done fi # Do a clock set here for a few reasons: -- 1.7.2.1 -- Kurt

Am 2010-08-19 06:23, schrieb Victor Lowther:

I am missing the difference. Diff please?

/dev/rtc vs. /dev/{rtc,rtc0} as already said in the other replies. Patch against bashification-redux: From 5d3ac218c3e05bf9735bb49826bee0b393418699 Mon Sep 17 00:00:00 2001 From: Kurt J. Bosch Date: Thu, 19 Aug 2010 18:17:01 +0200 Subject: [PATCH 17/17] Correct rtc dev nodes creation --- rc.sysinit | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/rc.sysinit b/rc.sysinit index 4421def..9fb10af 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -59,10 +59,10 @@ if [[ $HWCLOCK_PARAMS ]]; then # If devtmpfs is used, the required RTC device already exists now # Otherwise, create whatever device is available if ! [[ -c /dev/rtc || -c /dev/rtc0 ]]; then - for dev in /sys/class/rtc/rtc0/dev /sys/class/misc/rtc/dev; do - [[ -e $dev ]] || continue - IFS=: read -r major minor < "$dev" - /bin/mknod /dev/rtc c $major $minor + for dev in /sys/class/rtc/rtc0 /sys/class/misc/rtc; do + [[ -e $dev/dev ]] || continue + IFS=: read -r major minor< "$dev/dev" + /bin/mknod /dev/${dev##*/} c $major $minor done fi -- 1.7.2.1

Am 30.06.2010 23:47, schrieb Victor Lowther:

--- rc.sysinit | 9 ++++----- 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/rc.sysinit b/rc.sysinit index 1d16224..31636e5 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -113,13 +113,12 @@ fi run_hook sysinit_udevsettled

# bring up the loopback interface -if [ -d /sys/class/net/lo ]; then +if [[ -d /sys/class/net/lo ]]; then stat_busy "Bringing up loopback interface" - /sbin/ifconfig lo 127.0.0.1 up - if [ $? -ne 0 ]; then - stat_fail + if ! /sbin/ifconfig lo 127.0.0.1 up; then + stat_fail else - stat_done + stat_done fi fi

Hmm, this could be done with the status() function instead, right? No need for stat_{busy,fail,done} then.

This patch should be split into two: Am 30.06.2010 23:47, schrieb Victor Lowther:

diff --git a/rc.sysinit b/rc.sysinit index 5842a57..c6ed35f 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -303,12 +303,12 @@ status "Updating Module Dependencies" /sbin/depmod -A : >| /etc/profile.d/locale.sh /bin/chmod 755 /etc/profile.d/locale.sh # Set user defined locale -[ -z "$LOCALE" ] && LOCALE="en_US" +[[ $LOCALE ]] || LOCALE="en_US" stat_busy "Setting Locale: $LOCALE" echo "export LANG=$LOCALE" >>/etc/profile.d/locale.sh stat_done

-if echo "$LOCALE" | /bin/grep -qi utf ; then +if [[ $LOCALE =~ utf|UTF ]]; then stat_busy "Setting Consoles to UTF-8 mode" # UTF-8 consoles are default since 2.6.24 kernel # this code is needed not only for older kernels,

Simple bashification.

@@ -318,9 +318,11 @@ if echo "$LOCALE" | /bin/grep -qi utf ; then printf "\033%%G" > ${i} done # the $CONSOLE check helps us avoid this when running scripts from cron - echo 'if [ "$CONSOLE" = "" -a "$TERM" = "linux" -a -t 1 ]; then printf "\033%%G"; fi' >>/etc/profile.d/locale.sh + cat <<"EOF" >>/etc/profile.d/locale.sh +if [ "$CONSOLE" = "" -a "$TERM" = "linux" -a -t 1 ]; then printf "\033%%G"; fi +EOF

This has nothing to do with bashification, this is a readability improvement. Same applies below. Either merge the bashifications into the "big one" and apply the readability changes separately, or leave it like this, but adjust the commit message. Same applies below.

stat_done - [ -n "$KEYMAP" ] && status "Loading Keyboard Map: $KEYMAP" /bin/loadkeys -q -u $KEYMAP + [[ $KEYMAP ]] && status "Loading Keyboard Map: $KEYMAP" /bin/loadkeys -q -u "$KEYMAP" else stat_busy "Setting Consoles to legacy mode" # make non-UTF-8 consoles work on 2.6.24 and newer kernels @@ -329,9 +331,11 @@ else printf "\033%%@" > ${i} done # the $CONSOLE check helps us avoid this when running scripts from cron - echo 'if [ "$CONSOLE" = "" -a "$TERM" = "linux" -a -t 1 ]; then printf "\033%%@"; fi' >>/etc/profile.d/locale.sh + cat <<"EOF" >>/etc/profile.d/locale.sh +if [ "$CONSOLE" = "" -a "$TERM" = "linux" -a -t 1 ]; then printf "\033%%@"; fi +EOF stat_done - [ -n "$KEYMAP" ] && status "Loading Keyboard Map: $KEYMAP" /bin/loadkeys -q $KEYMAP + [[ $KEYMAP ]] && status "Loading Keyboard Map: $KEYMAP" /bin/loadkeys -q $KEYMAP fi

# Set console font if required

On Wed, Jun 30, 2010 at 17:47, Victor Lowther wrote:

---  rc.sysinit |    2 +-  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/rc.sysinit b/rc.sysinit index 319ea60..037a9f2 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -277,7 +277,7 @@ stat_busy "Removing Leftover Files"  /bin/rm -f /var/lock/* &>/dev/null  /bin/rm -rf /tmp/* /tmp/.* &>/dev/null  /bin/rm -f /forcefsck &>/dev/null -(cd /var/run && /usr/bin/find . ! -type d -exec /bin/rm -f -- {} \; ) +(cd /var/run && /usr/bin/find . \! -type d -delete  )  : >| /var/run/utmp  /bin/chmod 0664 /var/run/utmp  # Keep {x,k,g}dm happy with xorg -- 1.7.1

I don't think this is portable though.

Am 30.06.2010 23:58, schrieb Daenyth Blank:

On Wed, Jun 30, 2010 at 17:47, Victor Lowther wrote:

...

--- rc.sysinit | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/rc.sysinit b/rc.sysinit index 319ea60..037a9f2 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -277,7 +277,7 @@ stat_busy "Removing Leftover Files" /bin/rm -f /var/lock/* &>/dev/null /bin/rm -rf /tmp/* /tmp/.* &>/dev/null /bin/rm -f /forcefsck &>/dev/null -(cd /var/run && /usr/bin/find . ! -type d -exec /bin/rm -f -- {} \; ) +(cd /var/run && /usr/bin/find . \! -type d -delete ) : >| /var/run/utmp /bin/chmod 0664 /var/run/utmp # Keep {x,k,g}dm happy with xorg -- 1.7.1

I don't think this is portable though.

Portability is not an issue. The beauty about this is that we can simply assume we have GNU tools and we do use Linux ... because that is what Arch is based on. I have no problem having GNU-specific features in there, as we can rely on GNU find (or GNU whatever) being present. The only exception is initramfs - however, no shell script is used in both the system and initramfs.

Am 30.06.2010 23:47, schrieb Victor Lowther:

--- rc.sysinit | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/rc.sysinit b/rc.sysinit index 319ea60..037a9f2 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -277,7 +277,7 @@ stat_busy "Removing Leftover Files" /bin/rm -f /var/lock/* &>/dev/null /bin/rm -rf /tmp/* /tmp/.* &>/dev/null /bin/rm -f /forcefsck &>/dev/null -(cd /var/run && /usr/bin/find . ! -type d -exec /bin/rm -f -- {} \; ) +(cd /var/run && /usr/bin/find . \! -type d -delete ) : >| /var/run/utmp /bin/chmod 0664 /var/run/utmp # Keep {x,k,g}dm happy with xorg

ACKed.

Am 2010-06-30 23:47, schrieb Victor Lowther:

--- functions | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/functions b/functions index 9ec8b5e..f1dce8a 100644 --- a/functions +++ b/functions @@ -4,6 +4,10 @@

# width:

+grep -q initdebug&& { + exec 2>/dev/tty9 +} +

Hmm, won't this cause a read from stdin when starting/stopping daemons manually in a terminal?

STAT_COL=80 if [[ ! -t 1 ]]; then USECOLOR=""

Am 30.06.2010 23:47, schrieb Victor Lowther:

--- functions | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/functions b/functions index 9ec8b5e..f1dce8a 100644 --- a/functions +++ b/functions @@ -4,6 +4,10 @@

# width:

+grep -q initdebug && { + exec 2>/dev/tty9 +} + STAT_COL=80 if [[ ! -t 1 ]]; then USECOLOR=""

Apart from Kurt's comment: Won't this cause errors to be omitted from the console? I'd much rather implement a full boot logging using bootlogd, but last time I tried, it was broken. Redirecting stderr away from the normal output seems unhelpful, and unecessary. I'd much rather omit this in favor of a patch that adds proper boot logging, which is a feature we really need.

On Fri, 2010-07-16 at 14:06 +0200, Thomas Bächler wrote:

Am 30.06.2010 23:47, schrieb Victor Lowther:

...

This builds straight out of a git checkout. --- PKGBUILD | 23 +++++++++++++++++++++++ 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/PKGBUILD b/PKGBUILD new file mode 100644 index 0000000..79b3403 --- /dev/null +++ b/PKGBUILD @@ -0,0 +1,23 @@ +pkgname=initscripts-git +pkgver=$(date +%s) +pkgrel=$(git log --pretty=format:%h |head -n 1) +pkgdesc="System initialization/bootup scripts" +arch=('i686' 'x86_64') +url="http://www.archlinux.org" +license=('GPL') +groups=('base') +conflicts=('initscripts') +provides=('initscripts=9999') +backup=(etc/inittab etc/rc.conf etc/rc.local etc/rc.local.shutdown) +depends=('glibc' 'bash' 'awk' 'grep' 'coreutils' 'sed' 'udev>=139-1' + 'net-tools' 'ncurses' 'kbd' 'findutils' 'sysvinit') +optdepends=('bridge-utils: Network bridging support' + 'dhcpcd: DHCP network configuration' + 'wireless_tools: Wireless networking') +source=() +sha256sums=() + +build() { + cd .. + DESTDIR=${pkgdir} ./install.sh +}

Not necessary IMO, but nice to have. Should come with a .gitignore patch that adds '*pkg.tar.*'.

It is nice to have -- being able to makepkg && sudo pacman -U inits*.tar.* sure does make it easier to test things. Why do you think a .gitignore patch is needed? Unless someone does something silly like adding the tarball to the git repo git will ignore it anyways. -- Victor Lowther LPIC2 UCP RHCE

Am 2010-06-30 23:47, schrieb Victor Lowther:

--- functions | 2 +- rc.shutdown | 3 +++ 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/functions b/functions index f1dce8a..2c80a59 100644 --- a/functions +++ b/functions @@ -4,7 +4,7 @@

# width:

-grep -q initdebug&& { +grep -q initdebug /proc/cmdline&& { exec 2>/dev/tty9 }

OK that fixes the stdin problem :) - but /proc won't be mounted when this is sourced in rc.sysinit if someone doesn't use an initcpio.

diff --git a/rc.shutdown b/rc.shutdown index 1081970..96b10f0 100755 --- a/rc.shutdown +++ b/rc.shutdown @@ -93,6 +93,9 @@ run_hook shutdown_poweroff

# Power off or reboot printsep + +grep -q initdebug /proc/cmdline&& sleep 9999 + if [[ $RUNLEVEL = 0 ]]; then printhl "${C_H2}POWER OFF" /sbin/poweroff -d -f -h -i

Am 30.06.2010 23:47, schrieb Victor Lowther:

--- functions | 2 +- rc.shutdown | 3 +++ 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/functions b/functions index f1dce8a..2c80a59 100644 --- a/functions +++ b/functions @@ -4,7 +4,7 @@

# width:

-grep -q initdebug && { +grep -q initdebug /proc/cmdline && { exec 2>/dev/tty9 }

diff --git a/rc.shutdown b/rc.shutdown index 1081970..96b10f0 100755 --- a/rc.shutdown +++ b/rc.shutdown @@ -93,6 +93,9 @@ run_hook shutdown_poweroff

# Power off or reboot printsep + +grep -q initdebug /proc/cmdline && sleep 9999 + if [[ $RUNLEVEL = 0 ]]; then printhl "${C_H2}POWER OFF" /sbin/poweroff -d -f -h -i

Seems nice. I'll still omit the initdebug patches for now and rethink this, for some reason I don't like this yet.

Am 30.06.2010 23:47, schrieb Victor Lowther:

--- network | 11 +++-------- 1 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/network b/network index 977e81e..242acc3 100755 --- a/network +++ b/network @@ -3,14 +3,9 @@ . /etc/rc.conf . /etc/rc.d/functions

-# wireless settings -[ -f /etc/conf.d/wireless ] && . /etc/conf.d/wireless -# ethernet bonding settings -[ -f /etc/conf.d/bonding ] && . /etc/conf.d/bonding -# bridge settings -[ -f /etc/conf.d/bridges ] && . /etc/conf.d/bridges -# dhcpcd settings -[ -f /etc/conf.d/dhcpcd ] && . /etc/conf.d/dhcpcd +for s in wireless bonding bridges dhcpcd; do + [[ -f /etc/conf.d/$s ]] && . "/etc/conf.d/$s" +done

ifup() {

ACKed.

On Wed, 2010-06-30 at 23:55 +0200, Thomas Bächler wrote:

Am 30.06.2010 23:47, schrieb Victor Lowther:

...

Despite efforts to make the initscripts POSIX, we use bash 4.0 features.

Bashifying this framework should result in about a 30% speedup, assuming no IO latency and that all programs we call also take zero time. :) --- functions | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/functions b/functions index 672eed2..023de35 100644 --- a/functions +++ b/functions @@ -1,4 +1,4 @@ -# +#!/bin/bash # initscripts functions

I don't see the need for this patch. functions is not supposed to be executed standalone, it is only source'd from bash scripts.

It is a habit I have -- including the shebang line at the top makes sure my text editors automatically detect the right shell syntax for syntax highlighting. -- Victor Lowther LPIC2 UCP RHCE

On Tue, Sep 7, 2010 at 3:32 PM, Thomas Bächler wrote:

Am 30.06.2010 23:47, schrieb Victor Lowther:

...

Despite efforts to make the initscripts POSIX, we use bash 4.0 features.

Bashifying this framework should result in about a 30% speedup, assuming no IO latency and that all programs we call also take zero time. :)

I just pushed the patches - I was going to do more review of some of them, but I am apparently too busy. Please post any patches (especially if a correction of patch 21 is needed, I haven't finished reading the discussion) rebased on the current initscripts.git.

Your last patch has a typo (missed close paren): diff --git a/rc.sysinit b/rc.sysinit index b25f7ac..dc916a2 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -275,7 +275,7 @@ stat_busy "Removing Leftover Files" /bin/rm -f /var/lock/* &>/dev/null /bin/rm -rf /tmp/* /tmp/.* &>/dev/null /bin/rm -f /forcefsck &>/dev/null -[[ -d /var/run ]] && /usr/bin/find /var/run/ \! -type d -delete ) +[[ -d /var/run ]] && /usr/bin/find /var/run/ \! -type d -delete : >| /var/run/utmp /bin/chmod 0664 /var/run/utmp # Keep {x,k,g}dm happy with xorg