From 903e92bde6508486482c6d2d18e7ac2087fc06c4 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 6 Nov 2009 15:23:16 +0000 Subject: [PATCH] * Use Upstart's "export fork" feature to properly detect when a daemon is "ready". This prevents ugly race conditions, e.g. HAL failing to start because dbus hasn't finished starting yet. * Support post-start scripts. These are executed after the job's main process has started but before the job's "started" event is emitted. For instance, the udev job uses this to perform "udevadm trigger / settle" to create all devices. Previously this had to be done in the pre-start script, so the daemon had to started in the pre-start script as well. svn path=/nixos/branches/upstart-0.6/; revision=18211 --- modules/services/hardware/hal.nix | 19 ++++------- modules/services/hardware/udev.nix | 27 +++++---------- modules/services/system/dbus.nix | 19 ++++------- modules/system/upstart/upstart.nix | 53 ++++++++++++++++++++++++++---- 4 files changed, 67 insertions(+), 51 deletions(-) diff --git a/modules/services/hardware/hal.nix b/modules/services/hardware/hal.nix index 56bcf1fb4d8..be51968317f 100644 --- a/modules/services/hardware/hal.nix +++ b/modules/services/hardware/hal.nix @@ -99,21 +99,14 @@ in mkdir -m 0755 -p /var/run/hald rm -f /var/cache/hald/fdi-cache - - # !!! Hack: start the daemon here to make sure it's - # running when the Upstart job reaches the "running" - # state. Should be fixable in Upstart 0.6. - # The `PATH=' works around a bug in HAL: it concatenates - # its libexec directory to $PATH, but using a 512-byte - # buffer. So if $PATH is too long it fails. - PATH= ${hal}/sbin/hald --use-syslog # --verbose=yes ''; - postStop = - '' - pid=$(cat /var/run/hald/pid || true) - test -n "$pid" && kill "$pid" - ''; + daemonType = "fork"; + + # The `PATH=' works around a bug in HAL: it concatenates + # its libexec directory to $PATH, but using a 512-byte + # buffer. So if $PATH is too long it fails. + script = "PATH= exec ${hal}/sbin/hald --use-syslog"; }; services.udev.packages = [hal]; diff --git a/modules/services/hardware/udev.nix b/modules/services/hardware/udev.nix index 105f8a2b54f..2baa87e9547 100644 --- a/modules/services/hardware/udev.nix +++ b/modules/services/hardware/udev.nix @@ -169,9 +169,6 @@ in mkdir -p /var/lib/udev/rules.d - # Get rid of possible old udev processes. - ${procps}/bin/pkill -u root "^udevd$" || true - # Do the loading of additional stage 2 kernel modules. # Maybe this isn't the best place... for i in ${toString config.boot.kernelModules}; do @@ -179,32 +176,24 @@ in ${modprobe}/sbin/modprobe $i || true done - # Start udev. mkdir -p /dev/.udev # !!! bug in udev? - ${udev}/sbin/udevd --daemon + ''; + daemonType = "fork"; + + exec = "${udev}/sbin/udevd --daemon"; + + postStart = + '' # Let udev create device nodes for all modules that have already # been loaded into the kernel (or for which support is built into # the kernel). ${udev}/sbin/udevadm trigger ${udev}/sbin/udevadm settle # wait for udev to finish - # Kill udev, let Upstart restart and monitor it. (This is nasty, - # but we have to run `udevadm trigger' first. Maybe we can use - # Upstart's `binary' keyword, but it isn't implemented yet.) - if ! ${procps}/bin/pkill -u root "^udevd$"; then - echo "couldn't stop udevd" - fi - - while ${procps}/bin/pgrep -u root "^udevd$"; do - sleep 1 - done - initctl emit new-devices ''; - - exec = "${udev}/sbin/udevd"; - + }; }; diff --git a/modules/services/system/dbus.nix b/modules/services/system/dbus.nix index d91be4eb88b..b0c8dc66ae5 100644 --- a/modules/services/system/dbus.nix +++ b/modules/services/system/dbus.nix @@ -124,24 +124,19 @@ in ${dbus.tools}/bin/dbus-uuidgen --ensure rm -f ${homeDir}/pid - # !!! hack - dbus should be running once this job is - # considered "running"; should be fixable once we have - # Upstart 0.6. - ${dbus}/bin/dbus-daemon --config-file=${configDir}/system.conf ''; + daemonType = "fork"; + + exec = "${dbus}/bin/dbus-daemon --config-file=${configDir}/system.conf"; + postStop = '' - pid=$(cat ${homeDir}/pid) - if test -n "$pid"; then - kill $pid - fi - # !!! Hack: doesn't belong here. - pid=$(cat /var/run/ConsoleKit/pid) + pid=$(cat /var/run/ConsoleKit/pid || true) if test -n "$pid"; then - kill $pid - rm /var/run/ConsoleKit/pid + kill $pid || true + rm -f /var/run/ConsoleKit/pid fi ''; }; diff --git a/modules/system/upstart/upstart.nix b/modules/system/upstart/upstart.nix index 0919fc0d75f..fea8f9e880c 100644 --- a/modules/system/upstart/upstart.nix +++ b/modules/system/upstart/upstart.nix @@ -38,16 +38,16 @@ let else "" } - ${if job.stopOn != "" then "stop on ${job.stopOn}" else ""} + ${optionalString (job.stopOn != "") "stop on ${job.stopOn}"} env PATH=${makeSearchPath "bin" upstartPath}:${makeSearchPath "sbin" upstartPath} ${concatMapStrings (n: "env ${n}=${getAttr n job.environment}\n") (attrNames job.environment)} - ${if job.preStart != "" then '' + ${optionalString (job.preStart != "") '' pre-start script ${job.preStart} end script - '' else ""} + ''} ${if job.script != "" && job.exec != "" then abort "Job ${job.name} has both a `script' and `exec' attribute." @@ -64,13 +64,28 @@ let else "" } - ${if job.respawn && !job.task then "respawn" else ""} + ${optionalString (job.postStart != "") '' + post-start script + ${job.postStart} + end script + ''} + + ${optionalString job.task "task"} + ${optionalString job.respawn "respawn"} - ${if job.postStop != "" then '' + ${optionalString (job.postStop != "") '' post-stop script ${job.postStop} end script - '' else ""} + ''} + + ${optionalString (!job.task) ( + if job.daemonType == "fork" then "expect fork" else + if job.daemonType == "daemon" then "expect daemon" else + if job.daemonType == "stop" then "expect stop" else + if job.daemonType == "none" then "" else + throw "invalid daemon type `${job.daemonType}'" + )} ${job.extraConfig} ''; @@ -141,6 +156,16 @@ let ''; }; + postStart = mkOption { + type = types.string; + default = ""; + description = '' + Shell commands executed after the job is started (i.e. after + the job's main process is started), but before the job is + considered “running”. + ''; + }; + postStop = mkOption { type = types.string; default = ""; @@ -156,7 +181,7 @@ let description = '' Command to start the job's main process. If empty, the job has no main process, but can still have pre/post-start - and pre/post-stop scripts, and is considered "running" + and pre/post-stop scripts, and is considered “running” until it is stopped. ''; }; @@ -198,6 +223,20 @@ let ''; }; + daemonType = mkOption { + type = types.string; + default = "none"; + description = '' + Determines how Upstart detects when a daemon should be + considered “running”. The value none means + that the daemon is considered ready immediately. The value + fork means that the daemon will fork once. + The value daemon means that the daemon will + fork twice. The value stop means that the + daemon will raise the SIGSTOP signal to indicate readiness. + ''; + }; + extraConfig = mkOption { type = types.string; default = "";