On Monday 27 October 2008 09:53, Vladimir Dronnikov wrote:
> Hello, Denys!
>
> I've refreshed the patches at http://drvv.ru/busybox
> Please, take a look
Looking at http://drvv.ru/busybox/runsvdir.patch
On the "nitpicks" level, I propose having less
#if ENABLE_FEATURE_RUNSVDIR_INIT
statements, by using constant-zero is_init if it is not.
See attached patch.
Second, also easy thing - please use /* */ comments
as the rest of this file does.
Harder problems:
+ // let services exit cleanly
+ sync(); sleep(5);
sync() might block for long. With some stupid
situations (runaway process creating more and more
dirty data + kernel buglet), it may never return.
I prefer syncing without waiting, say,
by execing sync() from a child.
+ // kill survived services
+ killall(SIGKILL);
+ sync(); sleep(1);
+ // umount -a
+ // ???
+ // reboot
+ reboot(
+ (SIGUSR1 == bb_got_signal) ? RB_HALT_SYSTEM :
+ (SIGUSR2 == bb_got_signal) ? RB_POWER_OFF :
+ RB_AUTOBOOT
+ );
Do we really want to hard-reboot without remounting RO
or unmounting filesystems?
What if user disagrees with our reboot sequence (needs bigger delays,
wants to free lop devices, etc?). Yes, he can code up "different reboot"
himself, as a shell script. But this exact *our* sequence can be set up
exactly the same way as a shell script, no need to patch runsvdir! Right?
--
vda
diff -d -urpN busybox.5/runit/Config.in busybox.6/runit/Config.in
--- busybox.5/runit/Config.in 2008-10-26 02:02:35.000000000 +0200
+++ busybox.6/runit/Config.in 2008-10-29 03:22:06.000000000 +0100
@@ -20,6 +20,13 @@ config RUNSVDIR
a directory, in the services directory dir, up to a limit of 1000
subdirectories, and restarts a runsv process if it terminates.
+config FEATURE_RUNSVDIR_INIT
+ bool "Run as init"
+ depends on RUNSVDIR
+ default n
+ help
+ Act as a simple init replacement if run as PID 1.
+
config FEATURE_RUNSVDIR_LOG
bool "Enable scrolling argument log"
depends on RUNSVDIR
diff -d -urpN busybox.5/runit/runsvdir.c busybox.6/runit/runsvdir.c
--- busybox.5/runit/runsvdir.c 2008-10-26 02:02:35.000000000 +0200
+++ busybox.6/runit/runsvdir.c 2008-10-29 03:37:49.000000000 +0100
@@ -28,11 +28,13 @@ ADVISED OF THE POSSIBILITY OF SUCH DAMAG
/* Busyboxed by Denys Vlasenko <[EMAIL PROTECTED]> */
/* TODO: depends on runit_lib.c - review and reduce/eliminate */
-#include <sys/poll.h>
-#include <sys/file.h>
#include "libbb.h"
#include "runit_lib.h"
+#if ENABLE_FEATURE_RUNSVDIR_INIT
+#include <sys/reboot.h>
+#endif
+
#define MAXSERVICES 1000
/* Should be not needed - all dirs are on same FS, right? */
@@ -96,6 +98,14 @@ static void warnx(const char *m1)
}
#endif
+static void killall(int signo)
+{
+ int i;
+ for (i = 0; i < svnum; i++)
+ if (sv[i].pid)
+ kill(sv[i].pid, signo);
+}
+
static void runsv(int no, const char *name)
{
pid_t pid;
@@ -116,8 +126,12 @@ static void runsv(int no, const char *na
if (set_pgrp)
setsid();
bb_signals(0
- + (1 << SIGHUP)
- + (1 << SIGTERM)
+ | (1 << SIGHUP)
+ | (1 << SIGTERM)
+#if ENABLE_FEATURE_RUNSVDIR_INIT
+ | (1 << SIGUSR1)
+ | (1 << SIGUSR2)
+#endif
, SIG_DFL);
execvp(prog[0], prog);
fatal2_cannot("start runsv ", name);
@@ -220,6 +234,11 @@ int runsvdir_main(int argc UNUSED_PARAM,
unsigned now;
unsigned stampcheck;
int i;
+#if ENABLE_FEATURE_RUNSVDIR_INIT
+ bool is_init = (1 == getpid());
+#else
+ enum { is_init = 0 };
+#endif
INIT_G();
@@ -227,7 +246,12 @@ int runsvdir_main(int argc UNUSED_PARAM,
set_pgrp = getopt32(argv, "P");
argv += optind;
- bb_signals_recursive((1 << SIGTERM) | (1 << SIGHUP), record_signo);
+ bb_signals_recursive(0
+ | (1 << SIGTERM)
+ | (1 << SIGHUP)
+ | (is_init ? ((1 << SIGUSR1) | (1 << SIGUSR2)) : 0)
+ , record_signo);
+
svdir = *argv++;
#if ENABLE_FEATURE_RUNSVDIR_LOG
@@ -341,16 +365,43 @@ run:
}
}
#endif
- switch (bb_got_signal) {
- case SIGHUP:
- for (i = 0; i < svnum; i++)
- if (sv[i].pid)
- kill(sv[i].pid, SIGTERM);
- // N.B. fall through
- case SIGTERM:
- _exit((SIGHUP == bb_got_signal) ? 111 : EXIT_SUCCESS);
+
+ if (bb_got_signal) {
+ // init signaled: kill all services
+ // runsvdir SIGHUP signaled: kill all services
+ if (SIGHUP == bb_got_signal || is_init) {
+ killall(SIGTERM);
+ }
+ // runsvdir: any signal causes exit
+ if (!is_init)
+ break;
+#if ENABLE_FEATURE_RUNSVDIR_INIT
+ // N.B. we are here if we are init
+
+ // SIGHUP means go on serving
+ if (SIGHUP == bb_got_signal)
+ bb_got_signal = 0;
+
+ // SIGUSR1 means halt
+ // SIGUSR2 means poweroff
+ // SIGTERM means reboot
+ // let services exit cleanly
+ sync(); sleep(5);
+ // kill survived services
+ killall(SIGKILL);
+ sync(); sleep(1);
+ // umount -a
+ // ???
+ // reboot
+ reboot(
+ (SIGUSR1 == bb_got_signal) ? RB_HALT_SYSTEM :
+ (SIGUSR2 == bb_got_signal) ? RB_POWER_OFF :
+ RB_AUTOBOOT
+ );
+#endif
}
}
- /* not reached */
- return 0;
+
+ /* N.B. never get here if is init */
+ return (SIGHUP == bb_got_signal) ? 111 : EXIT_SUCCESS;
}
_______________________________________________
busybox mailing list
[email protected]
http://busybox.net/cgi-bin/mailman/listinfo/busybox
