Discussion:
start-stop-daemon incorrect pid
Rory Vieira
2006-08-28 14:43:42 UTC
Permalink
Hi,

now that I've built the basic initramfs (from buildroot), I'm writing
scripts to start/stop services.

I'm trying to use start-stop-daemon to do this:

start-stop-daemon -S -x /usr/sbin/telnetd -m /var/run/telnetd.pid -p
/var/run/telnetd.pid

When checking the pid file, I noticed that the wrong pid is entered
(usually two less that it should be).
How can I come around this?
--
Rory Vieira
rory dot vieira at gmail dot com
Natanael Copa
2006-08-28 15:00:41 UTC
Permalink
Post by Rory Vieira
Hi,
now that I've built the basic initramfs (from buildroot), I'm writing
scripts to start/stop services.
start-stop-daemon -S -x /usr/sbin/telnetd -m /var/run/telnetd.pid -p
/var/run/telnetd.pid
When checking the pid file, I noticed that the wrong pid is entered
(usually two less that it should be).
How can I come around this?
This is probably because telnetd forks and returns the pid of the
parent. You can't use the pid (unless you can make telnetd create it for
you). Use --exec and --name instead.

btw... the "start-stop-daemon --stop --pidfile ...." is noisy. I'm not
sure how the debian start-stop-daemon is but in gentoo its more quiet by
default. (start-stop-daemon in gentoo is based on the debian one)

/var/home/ncopa $ sudo /etc/init.d/rsyncd stop
* Stopping rsyncd: stopped process in pidfile `/var/run/rsyncd.pid' (pid 26877).
ok.

It should look like:
* Stopping rsyncd: ok.

I have to either
1. patch busybox to behave. (or just be lazy and file a bug of it)
2. patch every init.d script (there are too many)
3. live with the ugly output and endusers future complains

--
Natanael Copa
Bernhard Fischer
2006-08-28 16:54:17 UTC
Permalink
Post by Natanael Copa
btw... the "start-stop-daemon --stop --pidfile ...." is noisy. I'm not
sure how the debian start-stop-daemon is but in gentoo its more quiet by
default. (start-stop-daemon in gentoo is based on the debian one)
/var/home/ncopa $ sudo /etc/init.d/rsyncd stop
* Stopping rsyncd: stopped process in pidfile `/var/run/rsyncd.pid' (pid 26877).
ok.
* Stopping rsyncd: ok.
I have to either
1. patch busybox to behave. (or just be lazy and file a bug of it)
Patches are welcome. IIRC there is a bug-entry for ssd not (?) working
when no pidfile was given. Would be awesome if you could take care of
this while you're at it :)

cheers,
Bernhard
Natanael Copa
2006-08-28 18:21:39 UTC
Permalink
Post by Bernhard Fischer
Post by Natanael Copa
btw... the "start-stop-daemon --stop --pidfile ...." is noisy. I'm not
sure how the debian start-stop-daemon is but in gentoo its more quiet by
default. (start-stop-daemon in gentoo is based on the debian one)
/var/home/ncopa $ sudo /etc/init.d/rsyncd stop
* Stopping rsyncd: stopped process in pidfile `/var/run/rsyncd.pid' (pid 26877).
ok.
* Stopping rsyncd: ok.
I have to either
1. patch busybox to behave. (or just be lazy and file a bug of it)
Patches are welcome. IIRC there is a bug-entry for ssd not (?) working
when no pidfile was given. Would be awesome if you could take care of
this while you're at it :)
you are right... I just need to find out if debian ssd does the same (I
don't know if its a real bug otherwise?) But this week I have to work
for 3, since I'm the only one left in the department. And I am tired...

checking on ubuntu... looks like all scripts has --stop --quiet...

Yes... ssd in ubuntu don't print the pid unless -v/--verbose. It *is* a
bug in busybox. I'll take a closer look later.
Post by Bernhard Fischer
cheers,
Bernhard
Bernhard Fischer
2006-08-28 18:31:45 UTC
Permalink
Post by Natanael Copa
you are right... I just need to find out if debian ssd does the same (I
don't know if its a real bug otherwise?) But this week I have to work
for 3, since I'm the only one left in the department. And I am tired...
heh. I have that fortunate position since about two weeks now.
sleepdeprived and annoyed to no end..
Post by Natanael Copa
checking on ubuntu... looks like all scripts has --stop --quiet...
Yes... ssd in ubuntu don't print the pid unless -v/--verbose. It *is* a
bug in busybox. I'll take a closer look later.
Missing feature/update. Ultimatively our incarnation is a stripped down
version of that very same beast.

Fore!
Bernhard Fischer
2006-08-28 18:31:45 UTC
Permalink
Post by Natanael Copa
you are right... I just need to find out if debian ssd does the same (I
don't know if its a real bug otherwise?) But this week I have to work
for 3, since I'm the only one left in the department. And I am tired...
heh. I have that fortunate position since about two weeks now.
sleepdeprived and annoyed to no end..
Post by Natanael Copa
checking on ubuntu... looks like all scripts has --stop --quiet...
Yes... ssd in ubuntu don't print the pid unless -v/--verbose. It *is* a
bug in busybox. I'll take a closer look later.
Missing feature/update. Ultimatively our incarnation is a stripped down
version of that very same beast.

Fore!
Natanael Copa
2006-08-28 18:21:39 UTC
Permalink
Post by Bernhard Fischer
Post by Natanael Copa
btw... the "start-stop-daemon --stop --pidfile ...." is noisy. I'm not
sure how the debian start-stop-daemon is but in gentoo its more quiet by
default. (start-stop-daemon in gentoo is based on the debian one)
/var/home/ncopa $ sudo /etc/init.d/rsyncd stop
* Stopping rsyncd: stopped process in pidfile `/var/run/rsyncd.pid' (pid 26877).
ok.
* Stopping rsyncd: ok.
I have to either
1. patch busybox to behave. (or just be lazy and file a bug of it)
Patches are welcome. IIRC there is a bug-entry for ssd not (?) working
when no pidfile was given. Would be awesome if you could take care of
this while you're at it :)
you are right... I just need to find out if debian ssd does the same (I
don't know if its a real bug otherwise?) But this week I have to work
for 3, since I'm the only one left in the department. And I am tired...

checking on ubuntu... looks like all scripts has --stop --quiet...

Yes... ssd in ubuntu don't print the pid unless -v/--verbose. It *is* a
bug in busybox. I'll take a closer look later.
Post by Bernhard Fischer
cheers,
Bernhard
Bernhard Fischer
2006-08-28 16:54:17 UTC
Permalink
Post by Natanael Copa
btw... the "start-stop-daemon --stop --pidfile ...." is noisy. I'm not
sure how the debian start-stop-daemon is but in gentoo its more quiet by
default. (start-stop-daemon in gentoo is based on the debian one)
/var/home/ncopa $ sudo /etc/init.d/rsyncd stop
* Stopping rsyncd: stopped process in pidfile `/var/run/rsyncd.pid' (pid 26877).
ok.
* Stopping rsyncd: ok.
I have to either
1. patch busybox to behave. (or just be lazy and file a bug of it)
Patches are welcome. IIRC there is a bug-entry for ssd not (?) working
when no pidfile was given. Would be awesome if you could take care of
this while you're at it :)

cheers,
Bernhard
Rory Vieira
2006-09-06 09:04:08 UTC
Permalink
Natanael,
Post by Natanael Copa
This is probably because telnetd forks and returns the pid of the
parent. You can't use the pid (unless you can make telnetd create it for
you). Use --exec and --name instead.
Ok, for some reason this is still not working for me....
Another issue on my side, is that almost all bb daemons don't create a
/var/run/*.pid file... (inetd does though)...
And start-stop-daemon isn't doing the trick I want :(

I'm currently working on some patches to have bb daemons create their
respective pid files anyway. Obviously this is gonna cost me some
bytes, but I have them to spare :)

I started of with dnsd, as the code looks simple enough for me to edit (ugh)...
I copied the stuff from inetd to begin with, and it's currently
creating the pidfile as expected (huray)....
Now I need to 'remove' the file when killed (AH)...
Not sure where that last unlink statement should go :(

Here's the patch (for now)...
diff -Naur ../busybox-1.2.1-orig/networking/dnsd.c ./networking/dnsd.c
--- ../busybox-1.2.1-orig/networking/dnsd.c 2006-07-01 00:42:02.000000000 +0200
+++ ./networking/dnsd.c 2006-09-06 10:47:37.000000000 +0200
@@ -28,6 +28,7 @@
static char *fileconf = "/etc/dnsd.conf";
#define LOCK_FILE "/var/run/dnsd.lock"
#define LOG_FILE "/var/log/dnsd.log"
+#define PID_FILE "/var/run/dnsd.pid"

enum {
MAX_HOST_LEN = 16, // longest host name allowed is 15
@@ -359,6 +360,7 @@
*/
static void interrupt(int x)
{
+ unlink(PID_FILE);
unlink(LOCK_FILE);
write(2, "interrupt exiting\n", 18);
exit(2);
@@ -392,7 +394,7 @@
fprintf(stderr,"fileconf: %s\n", fileconf);
}

- if(is_daemon())
+ if(is_daemon()) {
#ifdef BB_NOMMU
/* reexec for vfork() do continue parent */
vfork_daemon_rexec(1, 0, argc, argv, "-d");
@@ -400,6 +402,16 @@
bb_xdaemon(1, 0);
#endif

+ {
+ FILE *fp;
+
+ if ((fp = fopen (PID_FILE, "w")) != NULL) {
+ fprintf (fp, "%u\n", getpid ());
+ (void) fclose (fp);
+ }
+ }
+ }
+
dnsentryinit(is_verbose());

signal(SIGINT, interrupt);
@@ -453,6 +465,7 @@
}
} // end if
} // end while
+ unlink(PID_FILE);
return 0;
}

If I can get this to work, I will try and create patches for the other
daemons I use (syslogd,klogd etc)...

Thanks for any assistance
--
Rory Vieira
rory dot vieira at gmail dot com
Natanael Copa
2006-09-06 09:11:45 UTC
Permalink
Post by Rory Vieira
Natanael,
Post by Natanael Copa
This is probably because telnetd forks and returns the pid of the
parent. You can't use the pid (unless you can make telnetd create it for
you). Use --exec and --name instead.
Ok, for some reason this is still not working for me....
Another issue on my side, is that almost all bb daemons don't create a
/var/run/*.pid file... (inetd does though)...
And start-stop-daemon isn't doing the trick I want :(
I'm currently working on some patches to have bb daemons create their
respective pid files anyway. Obviously this is gonna cost me some
bytes, but I have them to spare :)
Rob commited a patch the other day, that should let you use --exec with
busybox applests. Try to checkout latest svn and test it.

Creating pids in several applets sounds like bloat.
Rory Vieira
2006-09-06 09:37:54 UTC
Permalink
Natanael,
Post by Natanael Copa
Rob commited a patch the other day, that should let you use --exec with
busybox applests. Try to checkout latest svn and test it.
I tried to apply that patch to my clean 1.2.1...
Only thing gcc was moning about was xstrdup not being available...
Changed that into bb_xstrdup, and at least it compiles :)
Haven't tested it's functionality though...
Post by Natanael Copa
Creating pids in several applets sounds like bloat.
I know... It is... (hehe LOL)
And I've been following the thread about this issue, and understand
(partially) what the problem is all about... (symlinks screwing us up,
and /proc is following them I believe)...

I already figured out why my patch isn't working to the full extent...
dnsd doesn't catch SIGTERM to any function... Changed that and all is
well (huray)...

As I was messing about with start-stop-daemon before, I though about
leaving ssd for what it is, and change my approach...
This comes down to just killing the pid of a given daemon... Now it's
pretty handy if they write them away ;)
The trouble I have, is that my start/stop scripts have the same name
as the daemon in question. So pidof isn't going to work, unless I
exclude the $! in the script. This works to some extent, but is ugly
script (in my point of view)...

I will go and try Robs patch again anyway, and see if ssd can now do
what it's supposed to...
So AFAIU I need to run the daemon with
start-stop-daemon -S -x /sbin/klogd
and kill it with
start-stop-daemon -K -x /sbin/klogd

Thanks and cheers,
--
Rory Vieira
rory dot vieira at gmail dot com
Natanael Copa
2006-09-06 09:46:32 UTC
Permalink
Post by Rory Vieira
Natanael,
I will go and try Robs patch again anyway, and see if ssd can now do
what it's supposed to...
Its alot easier than patching all applets and it should work. It worked
when I tested.
Post by Rory Vieira
So AFAIU I need to run the daemon with
start-stop-daemon -S -x /sbin/klogd
and kill it with
start-stop-daemon -K -x /sbin/klogd
exactly.

If it works maybe you could post a patch here against 1.2.1 for
inclusion in 1.2.2.

Thanks!
Post by Rory Vieira
Thanks and cheers,
--
Rory Vieira
rory dot vieira at gmail dot com
Rory Vieira
2006-09-06 09:55:42 UTC
Permalink
Natanael,
Post by Natanael Copa
Its alot easier than patching all applets and it should work. It worked
when I tested.
True...
Post by Natanael Copa
Post by Rory Vieira
So AFAIU I need to run the daemon with
start-stop-daemon -S -x /sbin/klogd
and kill it with
start-stop-daemon -K -x /sbin/klogd
exactly.
Ok, here's the results:

start-stop-daemon -S -x /sbin/klogd -- -c 4
/sbin/klogd already running
703

ps|grep 703
703 root SW< [kpsmoused]

So this isn't right....
Again, I ONLY changed xstrdup to bb_xstrdup
Post by Natanael Copa
If it works maybe you could post a patch here against 1.2.1 for
inclusion in 1.2.2.
It's the exact patch from Rob with the above change


Just a side note:
I'm using buildroot to create my system. I added Rob's patch to
/srv/buildroot/package/busybox as busybox02-ssd-fixup.patch, and did a
make clean in the /srv/buildroot/build_i386/busybox-1.2.1 folder.
During buildroot make, I can see that the patch is applied...
After build I checked the actual file to make sure :D

Cheers,
--
Rory Vieira
rory dot vieira at gmail dot com
Natanael Copa
2006-09-06 11:19:05 UTC
Permalink
Post by Rory Vieira
Natanael,
I finally fixed my problem by applying your patch instead...
So the patch in svn is not working?
Post by Rory Vieira
when using ssd -K it's a bit noisy... Solved that by using -q
The noisy ness is a problem for me where i run lots of gentoo init.d
scripts natively with busybox.

http://busybox.net/lists/busybox/2006-August/023952.html

But the above patch is a little too minimalistic. If you create a
bugreport for -v and -q options to ssd, I'll create a better patch for
it when I get time.
Post by Rory Vieira
Post by Natanael Copa
If it works maybe you could post a patch here against 1.2.1 for
inclusion in 1.2.2.
I recreated it and attached it (buildroot was monig about it)...
I'd prefer to fix the svn version if its broken. The patch you used uses
cmdline which is ugly.

--
Natanael Copa
Rory Vieira
2006-09-06 09:55:42 UTC
Permalink
Natanael,
Post by Natanael Copa
Its alot easier than patching all applets and it should work. It worked
when I tested.
True...
Post by Natanael Copa
Post by Rory Vieira
So AFAIU I need to run the daemon with
start-stop-daemon -S -x /sbin/klogd
and kill it with
start-stop-daemon -K -x /sbin/klogd
exactly.
Ok, here's the results:

start-stop-daemon -S -x /sbin/klogd -- -c 4
/sbin/klogd already running
703

ps|grep 703
703 root SW< [kpsmoused]

So this isn't right....
Again, I ONLY changed xstrdup to bb_xstrdup
Post by Natanael Copa
If it works maybe you could post a patch here against 1.2.1 for
inclusion in 1.2.2.
It's the exact patch from Rob with the above change


Just a side note:
I'm using buildroot to create my system. I added Rob's patch to
/srv/buildroot/package/busybox as busybox02-ssd-fixup.patch, and did a
make clean in the /srv/buildroot/build_i386/busybox-1.2.1 folder.
During buildroot make, I can see that the patch is applied...
After build I checked the actual file to make sure :D

Cheers,
--
Rory Vieira
rory dot vieira at gmail dot com
Natanael Copa
2006-09-06 11:19:05 UTC
Permalink
Post by Rory Vieira
Natanael,
I finally fixed my problem by applying your patch instead...
So the patch in svn is not working?
Post by Rory Vieira
when using ssd -K it's a bit noisy... Solved that by using -q
The noisy ness is a problem for me where i run lots of gentoo init.d
scripts natively with busybox.

http://busybox.net/lists/busybox/2006-August/023952.html

But the above patch is a little too minimalistic. If you create a
bugreport for -v and -q options to ssd, I'll create a better patch for
it when I get time.
Post by Rory Vieira
Post by Natanael Copa
If it works maybe you could post a patch here against 1.2.1 for
inclusion in 1.2.2.
I recreated it and attached it (buildroot was monig about it)...
I'd prefer to fix the svn version if its broken. The patch you used uses
cmdline which is ugly.

--
Natanael Copa
Rob Landley
2006-09-06 17:12:19 UTC
Permalink
Post by Rory Vieira
Natanael,
Post by Natanael Copa
Rob commited a patch the other day, that should let you use --exec with
busybox applests. Try to checkout latest svn and test it.
I tried to apply that patch to my clean 1.2.1...
Only thing gcc was moning about was xstrdup not being available...
Changed that into bb_xstrdup, and at least it compiles :)
Haven't tested it's functionality though...
Sigh. It's past time to start work on 1.2.2. (or a 1.2.1.fixes.patch,
anyway). Right, time to carve out a couple hours from the schedule and do
this...

Rob
--
Never bet against the cheap plastic solution.
Natanael Copa
2006-09-06 09:46:32 UTC
Permalink
Post by Rory Vieira
Natanael,
I will go and try Robs patch again anyway, and see if ssd can now do
what it's supposed to...
Its alot easier than patching all applets and it should work. It worked
when I tested.
Post by Rory Vieira
So AFAIU I need to run the daemon with
start-stop-daemon -S -x /sbin/klogd
and kill it with
start-stop-daemon -K -x /sbin/klogd
exactly.

If it works maybe you could post a patch here against 1.2.1 for
inclusion in 1.2.2.

Thanks!
Post by Rory Vieira
Thanks and cheers,
--
Rory Vieira
rory dot vieira at gmail dot com
Rob Landley
2006-09-06 17:12:19 UTC
Permalink
Post by Rory Vieira
Natanael,
Post by Natanael Copa
Rob commited a patch the other day, that should let you use --exec with
busybox applests. Try to checkout latest svn and test it.
I tried to apply that patch to my clean 1.2.1...
Only thing gcc was moning about was xstrdup not being available...
Changed that into bb_xstrdup, and at least it compiles :)
Haven't tested it's functionality though...
Sigh. It's past time to start work on 1.2.2. (or a 1.2.1.fixes.patch,
anyway). Right, time to carve out a couple hours from the schedule and do
this...

Rob
--
Never bet against the cheap plastic solution.
Rob Landley
2006-09-06 17:11:10 UTC
Permalink
Post by Natanael Copa
Creating pids in several applets sounds like bloat.
I mind the duplication, but if you add some kind of global "check_pidfile()"
option to libbb (with one big config option to globally enable/disable it
that chops it out when it's not wanted), that sounds reasonable...

Rob
--
Never bet against the cheap plastic solution.
Stephane Billiart
2006-09-06 19:46:36 UTC
Permalink
Post by Natanael Copa
Creating pids in several applets sounds like bloat.
I've already posted this patch I've been using for months on my system
to create pidfiles for monitoring purposes but it never made it to the
SVN tree.

I split the patch in two parts, the first adds writepidfile/removepidfile
to libbb and the second modifies inetd, syslogd and crond to use it.
fakeidentd is another daemon that could use this but it also does a
fchown on the pidfile so I did not modify it here (I don't use it anyway)
--
St?phane Billiart http://perso.orange.fr/billiart/
-------------- next part --------------
Index: libbb/Makefile.in
=================================--- libbb/Makefile.in (revision 16059)
+++ libbb/Makefile.in (working copy)
@@ -22,7 +22,7 @@
kernel_version.c last_char_is.c login.c \
make_directory.c md5.c mode_string.c mtab_file.c \
obscure.c parse_mode.c parse_number.c perror_msg.c \
- perror_msg_and_die.c get_console.c \
+ perror_msg_and_die.c pidfile.c get_console.c \
process_escape_sequence.c procps.c \
recursive_action.c remove_file.c \
restricted_shell.c run_parts.c run_shell.c safe_read.c safe_write.c \
Index: libbb/pidfile.c
=================================--- libbb/pidfile.c (revision 0)
+++ libbb/pidfile.c (revision 0)
@@ -0,0 +1,47 @@
+/* vi: set sw=4 ts=4: */
+/*
+ * Utility routines.
+ *
+ * Copyright (C) 1999-2004 by Erik Andersen <andersen at codepoet.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <stdio.h>
+#include <errno.h>
+#include "libbb.h"
+
+#ifdef CONFIG_FEATURE_PIDFILE
+extern int writepidfile(const char *path)
+{
+ FILE *pidf;
+
+ if ((pidf + fprintf(pidf, "%u\n", getpid());
+ fclose(pidf);
+ return 0;
+ }
+ return 1;
+}
+#endif
+
+/* END CODE */
+/*
+Local Variables:
+c-file-style: "linux"
+c-basic-offset: 4
+tab-width: 4
+End:
+*/
Index: include/libbb.h
=================================--- include/libbb.h (revision 16059)
+++ include/libbb.h (working copy)
@@ -120,6 +120,13 @@
};
extern int logmode;

+#ifdef CONFIG_FEATURE_PIDFILE
+extern int writepidfile(const char *path);
+#define removepidfile(f) remove_file(f, FILEUTILS_FORCE)
+#else
+#define writepidfile(f)
+#define removepidfile(f)
+#endif
extern void bb_show_usage(void) ATTRIBUTE_NORETURN ATTRIBUTE_EXTERNALLY_VISIBLE;
extern void bb_error_msg(const char *s, ...) __attribute__ ((format (printf, 1, 2)));
extern void bb_error_msg_and_die(const char *s, ...) __attribute__ ((noreturn, format (printf, 1, 2)));
Index: Config.in
=================================--- Config.in (revision 16059)
+++ Config.in (working copy)
@@ -135,6 +135,12 @@
Don't enable this unless you have a really good reason to clean
things up manually.

+config CONFIG_FEATURE_PIDFILE
+ bool "Support for pidfile writing"
+ default n
+ help
+ Enable code to write pidfiles (crond and syslogd)
+
config CONFIG_FEATURE_SUID
bool "Support for SUID/SGID handling"
default n
-------------- next part --------------
Index: networking/inetd.c
=================================--- networking/inetd.c (revision 16059)
+++ networking/inetd.c (working copy)
@@ -1191,7 +1191,7 @@
}
(void) close (sep->se_fd);
}
- (void) unlink (_PATH_INETDPID);
+ removepidfile (_PATH_INETDPID);
exit (0);
}

@@ -1294,14 +1294,7 @@
/* If run by hand, ensure groups vector gets trashed */
setgroups (1, &gid);
}
-
- {
- FILE *fp;
-
- if ((fp - fprintf (fp, "%u\n", getpid ());
- (void) fclose (fp);
- }
+ writepidfile (_PATH_INETDPID);
}

if (getrlimit (RLIMIT_NOFILE, &rlim_ofile) < 0) {
Index: sysklogd/syslogd.c
=================================--- sysklogd/syslogd.c (revision 16059)
+++ sysklogd/syslogd.c (working copy)
@@ -425,6 +425,7 @@
unlink(lfile);
if (ENABLE_FEATURE_IPC_SYSLOG)
ipcsyslog_cleanup();
+ removepidfile("/var/run/syslogd.pid");

exit(TRUE);
}
@@ -641,6 +642,7 @@
xdaemon(0, 1);
#endif
}
+ writepidfile("/var/run/syslogd.pid");
doSyslogd();

return EXIT_SUCCESS;
Index: miscutils/crond.c
=================================--- miscutils/crond.c (revision 16059)
+++ miscutils/crond.c (working copy)
@@ -216,6 +216,7 @@
int rescan short sleep_time
+ writepidfile("/var/run/cron.pid");
for (;;) {
sleep((sleep_time + 1) - (short) (time(NULL) % sleep_time));
Rob Landley
2006-09-07 01:02:36 UTC
Permalink
Post by Stephane Billiart
Post by Natanael Copa
Creating pids in several applets sounds like bloat.
I've already posted this patch I've been using for months on my system
to create pidfiles for monitoring purposes but it never made it to the
SVN tree.
I split the patch in two parts, the first adds writepidfile/removepidfile
to libbb and the second modifies inetd, syslogd and crond to use it.
fakeidentd is another daemon that could use this but it also does a
fchown on the pidfile so I did not modify it here (I don't use it anyway)
Looks reasonable, except:

1) Please use your own copyright notice. We're not the FSF, you don't assign
copyright to us. And legally assigning copyright isn't trivial, which is one
of the issues in the SCO vs Novell lawsuit. A decent summary is here:

http://www.groklaw.net/article.php?story=20040326223857634

Putting Erik's copyright notice on it is not an "instrument of conveyance",
and if we go to enforce copyrights on a joint work it would SUCK for the
other side to be able to point out examples showing that who owns what is
unclear.

(Note that we don't always list every copyright in the files, especially when
we add a small chunk to a large existing file. Instead we mention the
contributor in the SVN checkin comment for that commit. I just don't want
there to be _false_ information in there.)

2) We use the short GPL boilerplate now.

3) So what happens if the thing's already running? The libbb code doesn't
check for an existing applet, which I thought was the point of a pidfile...?

Rob
--
Never bet against the cheap plastic solution.
Rory Vieira
2006-09-07 08:10:54 UTC
Permalink
Rob,
Post by Rob Landley
3) So what happens if the thing's already running? The libbb code doesn't
check for an existing applet, which I thought was the point of a pidfile...?
Does libbb need to do that or the init.d script that started the
daemon in the first place? (I use the latter, so do many other
distro's)

Without a pid, I can only check wether a service is running or not...
With a pidfile I gain an additional check:
service has died (and left the pid file intact...) which to me is
quite different to just "not running".
Especially my suse distro makes a clear distinction between the two
(although I have to admit there's no real physical difference... the
proces is just not there :) )

Personally I thing Nataneal is quite right in "creating pids sounds
like bloat...)
But you have to admit that Stephane is going in the right direction
and leaving that choice up to the end-user... (like me LOL)
And again, why does inetd write it's pid? (or udhcp)

Side note:
In my current buildroot system I added the killproc package
(containing startproc,killproc,checkproc). It doesn't really work with
bb applets UNLESS:
I use start-stop-deamon to start the applet in the foreground :( (I do
this with klogd/syslogd now). Checkproc is now capable of making a
distinction between the service running, not running, or stale (died,
whatever)...

Cheers,
--
Rory Vieira
rory dot vieira at gmail dot com
Rob Landley
2006-09-08 01:32:35 UTC
Permalink
Post by Rory Vieira
Rob,
Post by Rob Landley
3) So what happens if the thing's already running? The libbb code doesn't
check for an existing applet, which I thought was the point of a pidfile...?
Does libbb need to do that or the init.d script that started the
daemon in the first place? (I use the latter, so do many other
distro's)
Sigh. I don't suppose there's a reference on this somewhere that I can read?
Post by Rory Vieira
And again, why does inetd write it's pid? (or udhcp)
Because inetd and udhcp aren't really BusyBox applets. They're external
applets that were nailed to the side of the tree and had "BusyBox" painted on
them.

The easy way to find examples of these is to look at the largest files in the
tree: find . -name "*.c" | xargs ls -l | sort -k5,5n

The top three are all _classic_ examples of this:

e2fsck.c: the nails are bent and the paint isn't even dry. This whole
directory needs to get a from-scratch rewrite, which I'm working on in
parallel with everything else...

ash.c: used to be the largest file in the tree. I'm working on bbsh which
should also replace msh.c and hush.c (which are still in the top 10 and don't
share any code with ash.c)...

fdisk.c: Check the list for my rants on this, it's a todo item too.

Rob
--
Never bet against the cheap plastic solution.
Rob Landley
2006-09-08 01:32:35 UTC
Permalink
Post by Rory Vieira
Rob,
Post by Rob Landley
3) So what happens if the thing's already running? The libbb code doesn't
check for an existing applet, which I thought was the point of a pidfile...?
Does libbb need to do that or the init.d script that started the
daemon in the first place? (I use the latter, so do many other
distro's)
Sigh. I don't suppose there's a reference on this somewhere that I can read?
Post by Rory Vieira
And again, why does inetd write it's pid? (or udhcp)
Because inetd and udhcp aren't really BusyBox applets. They're external
applets that were nailed to the side of the tree and had "BusyBox" painted on
them.

The easy way to find examples of these is to look at the largest files in the
tree: find . -name "*.c" | xargs ls -l | sort -k5,5n

The top three are all _classic_ examples of this:

e2fsck.c: the nails are bent and the paint isn't even dry. This whole
directory needs to get a from-scratch rewrite, which I'm working on in
parallel with everything else...

ash.c: used to be the largest file in the tree. I'm working on bbsh which
should also replace msh.c and hush.c (which are still in the top 10 and don't
share any code with ash.c)...

fdisk.c: Check the list for my rants on this, it's a todo item too.

Rob
--
Never bet against the cheap plastic solution.
Stephane Billiart
2006-09-07 13:46:27 UTC
Permalink
Post by Rob Landley
Post by Stephane Billiart
Post by Natanael Copa
Creating pids in several applets sounds like bloat.
I've already posted this patch I've been using for months on my system
to create pidfiles for monitoring purposes but it never made it to the
SVN tree.
I split the patch in two parts, the first adds writepidfile/removepidfile
to libbb and the second modifies inetd, syslogd and crond to use it.
fakeidentd is another daemon that could use this but it also does a
fchown on the pidfile so I did not modify it here (I don't use it anyway)
1) Please use your own copyright notice. We're not the FSF, you don't assign
copyright to us. And legally assigning copyright isn't trivial, which is one
http://www.groklaw.net/article.php?story 040326223857634
Putting Erik's copyright notice on it is not an "instrument of conveyance",
and if we go to enforce copyrights on a joint work it would SUCK for the
other side to be able to point out examples showing that who owns what is
unclear.
(Note that we don't always list every copyright in the files, especially when
we add a small chunk to a large existing file. Instead we mention the
contributor in the SVN checkin comment for that commit. I just don't want
there to be _false_ information in there.)
2) We use the short GPL boilerplate now.
OK, I corrected these.
I also added support for fakeidentd, now only udhcp uses its own pidfile
routines (with locking).
Post by Rob Landley
3) So what happens if the thing's already running? The libbb code doesn't
check for an existing applet, which I thought was the point of a pidfile...?
pidfiles just make checking for a process presence/absence/death easier
in monitoring programs and startup/shutdown scripts.
Preventing a service to be launched several times is usually done in
those scripts, not in the daemon code itself.

I originally added the feature because I use monit
http://www.tildeslash.com/monit/
and it requires pidfiles to do its job.

New patches attached.
--
St?phane Billiart http://perso.orange.fr/billiart/
-------------- next part --------------
Index: libbb/Makefile.in
=================================--- libbb/Makefile.in (revision 16063)
+++ libbb/Makefile.in (working copy)
@@ -22,7 +22,7 @@
kernel_version.c last_char_is.c login.c \
make_directory.c md5.c mode_string.c mtab_file.c \
obscure.c parse_mode.c parse_number.c perror_msg.c \
- perror_msg_and_die.c get_console.c \
+ perror_msg_and_die.c pidfile.c get_console.c \
process_escape_sequence.c procps.c \
recursive_action.c remove_file.c \
restricted_shell.c run_parts.c run_shell.c safe_read.c safe_write.c \
Index: libbb/pidfile.c
=================================--- libbb/pidfile.c (revision 0)
+++ libbb/pidfile.c (revision 0)
@@ -0,0 +1,25 @@
+/* vi: set sw=4 ts=4: */
+/*
+ * pid file routines
+ *
+ * Copyright (C) 2006 by Stephane Billiart <stephane.billiart at gmail.com>
+ *
+ * Licensed under GPLv2 or later, see file LICENSE in this tarball for details.
+ */
+#include <stdio.h>
+#include <errno.h>
+#include "libbb.h"
+
+#ifdef CONFIG_FEATURE_PIDFILE
+extern int writepidfile(const char *path)
+{
+ FILE *pidf;
+
+ if ((pidf + fprintf(pidf, "%u\n", getpid());
+ fclose(pidf);
+ return 0;
+ }
+ return 1;
+}
+#endif
Index: include/libbb.h
=================================--- include/libbb.h (revision 16063)
+++ include/libbb.h (working copy)
@@ -120,6 +120,13 @@
};
extern int logmode;

+#ifdef CONFIG_FEATURE_PIDFILE
+extern int writepidfile(const char *path);
+#define removepidfile(f) remove_file(f, FILEUTILS_FORCE)
+#else
+#define writepidfile(f) ((void)0)
+#define removepidfile(f) ((void)0)
+#endif
extern void bb_show_usage(void) ATTRIBUTE_NORETURN ATTRIBUTE_EXTERNALLY_VISIBLE;
extern void bb_error_msg(const char *s, ...) __attribute__ ((format (printf, 1, 2)));
extern void bb_error_msg_and_die(const char *s, ...) __attribute__ ((noreturn, format (printf, 1, 2)));
Index: Config.in
=================================--- Config.in (revision 16063)
+++ Config.in (working copy)
@@ -135,6 +135,12 @@
Don't enable this unless you have a really good reason to clean
things up manually.

+config CONFIG_FEATURE_PIDFILE
+ bool "Support for pidfile writing"
+ default n
+ help
+ Enable code to write pidfiles (crond and syslogd)
+
config CONFIG_FEATURE_SUID
bool "Support for SUID/SGID handling"
default n
-------------- next part --------------
Index: networking/fakeidentd.c
=================================--- networking/fakeidentd.c (revision 16063)
+++ networking/fakeidentd.c (working copy)
@@ -102,31 +102,10 @@

static void handlexitsigs(int signum)
{
- if (unlink(PIDFILE) < 0)
- close(open(PIDFILE, O_WRONLY|O_CREAT|O_TRUNC, 0644));
+ removepidfile(PIDFILE);
exit(0);
}

-/* May succeed. If not, won't care. */
-static void writepid(uid_t nobody, uid_t nogrp)
-{
- char buf[24];
- int fd -
- if (fd < 0)
- return;
-
- snprintf(buf, 23, "%d\n", getpid());
- write(fd, buf, strlen(buf));
- fchown(fd, nobody, nogrp);
- close(fd);
-
- /* should this handle ILL, ... (see signal(7)) */
- signal(SIGTERM, handlexitsigs);
- signal(SIGINT, handlexitsigs);
- signal(SIGQUIT, handlexitsigs);
-}
-
/* return 0 as parent, 1 as child */
static int godaemon(void)
{
@@ -143,8 +122,15 @@
bb_error_msg_and_die("Cannot find uid/gid of user '%s'", nobodystr);
nobody nogrp - writepid(nobody, nogrp);
+ if (writepidfile(PIDFILE)) {
+ chown(PIDFILE, nobody, nogrp);

+ /* should this handle ILL, ... (see signal(7)) */
+ signal(SIGTERM, handlexitsigs);
+ signal(SIGINT, handlexitsigs);
+ signal(SIGQUIT, handlexitsigs);
+ }
+
close(0);
inetbind();
xsetgid(nogrp);
Index: networking/inetd.c
=================================--- networking/inetd.c (revision 16063)
+++ networking/inetd.c (working copy)
@@ -1191,7 +1191,7 @@
}
(void) close (sep->se_fd);
}
- (void) unlink (_PATH_INETDPID);
+ removepidfile (_PATH_INETDPID);
exit (0);
}

@@ -1294,14 +1294,7 @@
/* If run by hand, ensure groups vector gets trashed */
setgroups (1, &gid);
}
-
- {
- FILE *fp;
-
- if ((fp - fprintf (fp, "%u\n", getpid ());
- (void) fclose (fp);
- }
+ writepidfile (_PATH_INETDPID);
}

if (getrlimit (RLIMIT_NOFILE, &rlim_ofile) < 0) {
Index: sysklogd/syslogd.c
=================================--- sysklogd/syslogd.c (revision 16063)
+++ sysklogd/syslogd.c (working copy)
@@ -425,6 +425,7 @@
unlink(lfile);
if (ENABLE_FEATURE_IPC_SYSLOG)
ipcsyslog_cleanup();
+ removepidfile("/var/run/syslogd.pid");

exit(TRUE);
}
@@ -641,6 +642,7 @@
xdaemon(0, 1);
#endif
}
+ writepidfile("/var/run/syslogd.pid");
doSyslogd();

return EXIT_SUCCESS;
Index: miscutils/crond.c
=================================--- miscutils/crond.c (revision 16063)
+++ miscutils/crond.c (working copy)
@@ -216,6 +216,7 @@
int rescan short sleep_time
+ writepidfile("/var/run/cron.pid");
for (;;) {
sleep((sleep_time + 1) - (short) (time(NULL) % sleep_time));
Rory Vieira
2006-09-07 08:10:54 UTC
Permalink
Rob,
Post by Rob Landley
3) So what happens if the thing's already running? The libbb code doesn't
check for an existing applet, which I thought was the point of a pidfile...?
Does libbb need to do that or the init.d script that started the
daemon in the first place? (I use the latter, so do many other
distro's)

Without a pid, I can only check wether a service is running or not...
With a pidfile I gain an additional check:
service has died (and left the pid file intact...) which to me is
quite different to just "not running".
Especially my suse distro makes a clear distinction between the two
(although I have to admit there's no real physical difference... the
proces is just not there :) )

Personally I thing Nataneal is quite right in "creating pids sounds
like bloat...)
But you have to admit that Stephane is going in the right direction
and leaving that choice up to the end-user... (like me LOL)
And again, why does inetd write it's pid? (or udhcp)

Side note:
In my current buildroot system I added the killproc package
(containing startproc,killproc,checkproc). It doesn't really work with
bb applets UNLESS:
I use start-stop-deamon to start the applet in the foreground :( (I do
this with klogd/syslogd now). Checkproc is now capable of making a
distinction between the service running, not running, or stale (died,
whatever)...

Cheers,
--
Rory Vieira
rory dot vieira at gmail dot com
Stephane Billiart
2006-09-07 13:46:27 UTC
Permalink
Post by Rob Landley
Post by Stephane Billiart
Post by Natanael Copa
Creating pids in several applets sounds like bloat.
I've already posted this patch I've been using for months on my system
to create pidfiles for monitoring purposes but it never made it to the
SVN tree.
I split the patch in two parts, the first adds writepidfile/removepidfile
to libbb and the second modifies inetd, syslogd and crond to use it.
fakeidentd is another daemon that could use this but it also does a
fchown on the pidfile so I did not modify it here (I don't use it anyway)
1) Please use your own copyright notice. We're not the FSF, you don't assign
copyright to us. And legally assigning copyright isn't trivial, which is one
http://www.groklaw.net/article.php?story=20040326223857634
Putting Erik's copyright notice on it is not an "instrument of conveyance",
and if we go to enforce copyrights on a joint work it would SUCK for the
other side to be able to point out examples showing that who owns what is
unclear.
(Note that we don't always list every copyright in the files, especially when
we add a small chunk to a large existing file. Instead we mention the
contributor in the SVN checkin comment for that commit. I just don't want
there to be _false_ information in there.)
2) We use the short GPL boilerplate now.
OK, I corrected these.
I also added support for fakeidentd, now only udhcp uses its own pidfile
routines (with locking).
Post by Rob Landley
3) So what happens if the thing's already running? The libbb code doesn't
check for an existing applet, which I thought was the point of a pidfile...?
pidfiles just make checking for a process presence/absence/death easier
in monitoring programs and startup/shutdown scripts.
Preventing a service to be launched several times is usually done in
those scripts, not in the daemon code itself.

I originally added the feature because I use monit
http://www.tildeslash.com/monit/
and it requires pidfiles to do its job.

New patches attached.
--
St?phane Billiart http://perso.orange.fr/billiart/
-------------- next part --------------
Index: libbb/Makefile.in
===================================================================
--- libbb/Makefile.in (revision 16063)
+++ libbb/Makefile.in (working copy)
@@ -22,7 +22,7 @@
kernel_version.c last_char_is.c login.c \
make_directory.c md5.c mode_string.c mtab_file.c \
obscure.c parse_mode.c parse_number.c perror_msg.c \
- perror_msg_and_die.c get_console.c \
+ perror_msg_and_die.c pidfile.c get_console.c \
process_escape_sequence.c procps.c \
recursive_action.c remove_file.c \
restricted_shell.c run_parts.c run_shell.c safe_read.c safe_write.c \
Index: libbb/pidfile.c
===================================================================
--- libbb/pidfile.c (revision 0)
+++ libbb/pidfile.c (revision 0)
@@ -0,0 +1,25 @@
+/* vi: set sw=4 ts=4: */
+/*
+ * pid file routines
+ *
+ * Copyright (C) 2006 by Stephane Billiart <stephane.billiart at gmail.com>
+ *
+ * Licensed under GPLv2 or later, see file LICENSE in this tarball for details.
+ */
+#include <stdio.h>
+#include <errno.h>
+#include "libbb.h"
+
+#ifdef CONFIG_FEATURE_PIDFILE
+extern int writepidfile(const char *path)
+{
+ FILE *pidf;
+
+ if ((pidf = bb_wfopen(path, "w")) != NULL) {
+ fprintf(pidf, "%u\n", getpid());
+ fclose(pidf);
+ return 0;
+ }
+ return 1;
+}
+#endif
Index: include/libbb.h
===================================================================
--- include/libbb.h (revision 16063)
+++ include/libbb.h (working copy)
@@ -120,6 +120,13 @@
};
extern int logmode;

+#ifdef CONFIG_FEATURE_PIDFILE
+extern int writepidfile(const char *path);
+#define removepidfile(f) remove_file(f, FILEUTILS_FORCE)
+#else
+#define writepidfile(f) ((void)0)
+#define removepidfile(f) ((void)0)
+#endif
extern void bb_show_usage(void) ATTRIBUTE_NORETURN ATTRIBUTE_EXTERNALLY_VISIBLE;
extern void bb_error_msg(const char *s, ...) __attribute__ ((format (printf, 1, 2)));
extern void bb_error_msg_and_die(const char *s, ...) __attribute__ ((noreturn, format (printf, 1, 2)));
Index: Config.in
===================================================================
--- Config.in (revision 16063)
+++ Config.in (working copy)
@@ -135,6 +135,12 @@
Don't enable this unless you have a really good reason to clean
things up manually.

+config CONFIG_FEATURE_PIDFILE
+ bool "Support for pidfile writing"
+ default n
+ help
+ Enable code to write pidfiles (crond and syslogd)
+
config CONFIG_FEATURE_SUID
bool "Support for SUID/SGID handling"
default n
-------------- next part --------------
Index: networking/fakeidentd.c
===================================================================
--- networking/fakeidentd.c (revision 16063)
+++ networking/fakeidentd.c (working copy)
@@ -102,31 +102,10 @@

static void handlexitsigs(int signum)
{
- if (unlink(PIDFILE) < 0)
- close(open(PIDFILE, O_WRONLY|O_CREAT|O_TRUNC, 0644));
+ removepidfile(PIDFILE);
exit(0);
}

-/* May succeed. If not, won't care. */
-static void writepid(uid_t nobody, uid_t nogrp)
-{
- char buf[24];
- int fd = open(PIDFILE, O_WRONLY|O_CREAT|O_TRUNC, 0664);
-
- if (fd < 0)
- return;
-
- snprintf(buf, 23, "%d\n", getpid());
- write(fd, buf, strlen(buf));
- fchown(fd, nobody, nogrp);
- close(fd);
-
- /* should this handle ILL, ... (see signal(7)) */
- signal(SIGTERM, handlexitsigs);
- signal(SIGINT, handlexitsigs);
- signal(SIGQUIT, handlexitsigs);
-}
-
/* return 0 as parent, 1 as child */
static int godaemon(void)
{
@@ -143,8 +122,15 @@
bb_error_msg_and_die("Cannot find uid/gid of user '%s'", nobodystr);
nobody = pw->pw_uid;
nogrp = pw->pw_gid;
- writepid(nobody, nogrp);
+ if (writepidfile(PIDFILE)) {
+ chown(PIDFILE, nobody, nogrp);

+ /* should this handle ILL, ... (see signal(7)) */
+ signal(SIGTERM, handlexitsigs);
+ signal(SIGINT, handlexitsigs);
+ signal(SIGQUIT, handlexitsigs);
+ }
+
close(0);
inetbind();
xsetgid(nogrp);
Index: networking/inetd.c
===================================================================
--- networking/inetd.c (revision 16063)
+++ networking/inetd.c (working copy)
@@ -1191,7 +1191,7 @@
}
(void) close (sep->se_fd);
}
- (void) unlink (_PATH_INETDPID);
+ removepidfile (_PATH_INETDPID);
exit (0);
}

@@ -1294,14 +1294,7 @@
/* If run by hand, ensure groups vector gets trashed */
setgroups (1, &gid);
}
-
- {
- FILE *fp;
-
- if ((fp = fopen (_PATH_INETDPID, "w")) != NULL) {
- fprintf (fp, "%u\n", getpid ());
- (void) fclose (fp);
- }
+ writepidfile (_PATH_INETDPID);
}

if (getrlimit (RLIMIT_NOFILE, &rlim_ofile) < 0) {
Index: sysklogd/syslogd.c
===================================================================
--- sysklogd/syslogd.c (revision 16063)
+++ sysklogd/syslogd.c (working copy)
@@ -425,6 +425,7 @@
unlink(lfile);
if (ENABLE_FEATURE_IPC_SYSLOG)
ipcsyslog_cleanup();
+ removepidfile("/var/run/syslogd.pid");

exit(TRUE);
}
@@ -641,6 +642,7 @@
xdaemon(0, 1);
#endif
}
+ writepidfile("/var/run/syslogd.pid");
doSyslogd();

return EXIT_SUCCESS;
Index: miscutils/crond.c
===================================================================
--- miscutils/crond.c (revision 16063)
+++ miscutils/crond.c (working copy)
@@ -216,6 +216,7 @@
int rescan = 60;
short sleep_time = 60;

+ writepidfile("/var/run/cron.pid");
for (;;) {
sleep((sleep_time + 1) - (short) (time(NULL) % sleep_time));
Rob Landley
2006-09-07 01:02:36 UTC
Permalink
Post by Stephane Billiart
Post by Natanael Copa
Creating pids in several applets sounds like bloat.
I've already posted this patch I've been using for months on my system
to create pidfiles for monitoring purposes but it never made it to the
SVN tree.
I split the patch in two parts, the first adds writepidfile/removepidfile
to libbb and the second modifies inetd, syslogd and crond to use it.
fakeidentd is another daemon that could use this but it also does a
fchown on the pidfile so I did not modify it here (I don't use it anyway)
Looks reasonable, except:

1) Please use your own copyright notice. We're not the FSF, you don't assign
copyright to us. And legally assigning copyright isn't trivial, which is one
of the issues in the SCO vs Novell lawsuit. A decent summary is here:

http://www.groklaw.net/article.php?story=20040326223857634

Putting Erik's copyright notice on it is not an "instrument of conveyance",
and if we go to enforce copyrights on a joint work it would SUCK for the
other side to be able to point out examples showing that who owns what is
unclear.

(Note that we don't always list every copyright in the files, especially when
we add a small chunk to a large existing file. Instead we mention the
contributor in the SVN checkin comment for that commit. I just don't want
there to be _false_ information in there.)

2) We use the short GPL boilerplate now.

3) So what happens if the thing's already running? The libbb code doesn't
check for an existing applet, which I thought was the point of a pidfile...?

Rob
--
Never bet against the cheap plastic solution.
Rory Vieira
2006-09-06 09:37:54 UTC
Permalink
Natanael,
Post by Natanael Copa
Rob commited a patch the other day, that should let you use --exec with
busybox applests. Try to checkout latest svn and test it.
I tried to apply that patch to my clean 1.2.1...
Only thing gcc was moning about was xstrdup not being available...
Changed that into bb_xstrdup, and at least it compiles :)
Haven't tested it's functionality though...
Post by Natanael Copa
Creating pids in several applets sounds like bloat.
I know... It is... (hehe LOL)
And I've been following the thread about this issue, and understand
(partially) what the problem is all about... (symlinks screwing us up,
and /proc is following them I believe)...

I already figured out why my patch isn't working to the full extent...
dnsd doesn't catch SIGTERM to any function... Changed that and all is
well (huray)...

As I was messing about with start-stop-daemon before, I though about
leaving ssd for what it is, and change my approach...
This comes down to just killing the pid of a given daemon... Now it's
pretty handy if they write them away ;)
The trouble I have, is that my start/stop scripts have the same name
as the daemon in question. So pidof isn't going to work, unless I
exclude the $! in the script. This works to some extent, but is ugly
script (in my point of view)...

I will go and try Robs patch again anyway, and see if ssd can now do
what it's supposed to...
So AFAIU I need to run the daemon with
start-stop-daemon -S -x /sbin/klogd
and kill it with
start-stop-daemon -K -x /sbin/klogd

Thanks and cheers,
--
Rory Vieira
rory dot vieira at gmail dot com
Rob Landley
2006-09-06 17:11:10 UTC
Permalink
Post by Natanael Copa
Creating pids in several applets sounds like bloat.
I mind the duplication, but if you add some kind of global "check_pidfile()"
option to libbb (with one big config option to globally enable/disable it
that chops it out when it's not wanted), that sounds reasonable...

Rob
--
Never bet against the cheap plastic solution.
Stephane Billiart
2006-09-06 19:46:36 UTC
Permalink
Post by Natanael Copa
Creating pids in several applets sounds like bloat.
I've already posted this patch I've been using for months on my system
to create pidfiles for monitoring purposes but it never made it to the
SVN tree.

I split the patch in two parts, the first adds writepidfile/removepidfile
to libbb and the second modifies inetd, syslogd and crond to use it.
fakeidentd is another daemon that could use this but it also does a
fchown on the pidfile so I did not modify it here (I don't use it anyway)
--
St?phane Billiart http://perso.orange.fr/billiart/
-------------- next part --------------
Index: libbb/Makefile.in
===================================================================
--- libbb/Makefile.in (revision 16059)
+++ libbb/Makefile.in (working copy)
@@ -22,7 +22,7 @@
kernel_version.c last_char_is.c login.c \
make_directory.c md5.c mode_string.c mtab_file.c \
obscure.c parse_mode.c parse_number.c perror_msg.c \
- perror_msg_and_die.c get_console.c \
+ perror_msg_and_die.c pidfile.c get_console.c \
process_escape_sequence.c procps.c \
recursive_action.c remove_file.c \
restricted_shell.c run_parts.c run_shell.c safe_read.c safe_write.c \
Index: libbb/pidfile.c
===================================================================
--- libbb/pidfile.c (revision 0)
+++ libbb/pidfile.c (revision 0)
@@ -0,0 +1,47 @@
+/* vi: set sw=4 ts=4: */
+/*
+ * Utility routines.
+ *
+ * Copyright (C) 1999-2004 by Erik Andersen <andersen at codepoet.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <stdio.h>
+#include <errno.h>
+#include "libbb.h"
+
+#ifdef CONFIG_FEATURE_PIDFILE
+extern int writepidfile(const char *path)
+{
+ FILE *pidf;
+
+ if ((pidf = bb_wfopen(path, "w")) != NULL) {
+ fprintf(pidf, "%u\n", getpid());
+ fclose(pidf);
+ return 0;
+ }
+ return 1;
+}
+#endif
+
+/* END CODE */
+/*
+Local Variables:
+c-file-style: "linux"
+c-basic-offset: 4
+tab-width: 4
+End:
+*/
Index: include/libbb.h
===================================================================
--- include/libbb.h (revision 16059)
+++ include/libbb.h (working copy)
@@ -120,6 +120,13 @@
};
extern int logmode;

+#ifdef CONFIG_FEATURE_PIDFILE
+extern int writepidfile(const char *path);
+#define removepidfile(f) remove_file(f, FILEUTILS_FORCE)
+#else
+#define writepidfile(f)
+#define removepidfile(f)
+#endif
extern void bb_show_usage(void) ATTRIBUTE_NORETURN ATTRIBUTE_EXTERNALLY_VISIBLE;
extern void bb_error_msg(const char *s, ...) __attribute__ ((format (printf, 1, 2)));
extern void bb_error_msg_and_die(const char *s, ...) __attribute__ ((noreturn, format (printf, 1, 2)));
Index: Config.in
===================================================================
--- Config.in (revision 16059)
+++ Config.in (working copy)
@@ -135,6 +135,12 @@
Don't enable this unless you have a really good reason to clean
things up manually.

+config CONFIG_FEATURE_PIDFILE
+ bool "Support for pidfile writing"
+ default n
+ help
+ Enable code to write pidfiles (crond and syslogd)
+
config CONFIG_FEATURE_SUID
bool "Support for SUID/SGID handling"
default n
-------------- next part --------------
Index: networking/inetd.c
===================================================================
--- networking/inetd.c (revision 16059)
+++ networking/inetd.c (working copy)
@@ -1191,7 +1191,7 @@
}
(void) close (sep->se_fd);
}
- (void) unlink (_PATH_INETDPID);
+ removepidfile (_PATH_INETDPID);
exit (0);
}

@@ -1294,14 +1294,7 @@
/* If run by hand, ensure groups vector gets trashed */
setgroups (1, &gid);
}
-
- {
- FILE *fp;
-
- if ((fp = fopen (_PATH_INETDPID, "w")) != NULL) {
- fprintf (fp, "%u\n", getpid ());
- (void) fclose (fp);
- }
+ writepidfile (_PATH_INETDPID);
}

if (getrlimit (RLIMIT_NOFILE, &rlim_ofile) < 0) {
Index: sysklogd/syslogd.c
===================================================================
--- sysklogd/syslogd.c (revision 16059)
+++ sysklogd/syslogd.c (working copy)
@@ -425,6 +425,7 @@
unlink(lfile);
if (ENABLE_FEATURE_IPC_SYSLOG)
ipcsyslog_cleanup();
+ removepidfile("/var/run/syslogd.pid");

exit(TRUE);
}
@@ -641,6 +642,7 @@
xdaemon(0, 1);
#endif
}
+ writepidfile("/var/run/syslogd.pid");
doSyslogd();

return EXIT_SUCCESS;
Index: miscutils/crond.c
===================================================================
--- miscutils/crond.c (revision 16059)
+++ miscutils/crond.c (working copy)
@@ -216,6 +216,7 @@
int rescan = 60;
short sleep_time = 60;

+ writepidfile("/var/run/cron.pid");
for (;;) {
sleep((sleep_time + 1) - (short) (time(NULL) % sleep_time));
Natanael Copa
2006-09-06 09:11:45 UTC
Permalink
Post by Rory Vieira
Natanael,
Post by Natanael Copa
This is probably because telnetd forks and returns the pid of the
parent. You can't use the pid (unless you can make telnetd create it for
you). Use --exec and --name instead.
Ok, for some reason this is still not working for me....
Another issue on my side, is that almost all bb daemons don't create a
/var/run/*.pid file... (inetd does though)...
And start-stop-daemon isn't doing the trick I want :(
I'm currently working on some patches to have bb daemons create their
respective pid files anyway. Obviously this is gonna cost me some
bytes, but I have them to spare :)
Rob commited a patch the other day, that should let you use --exec with
busybox applests. Try to checkout latest svn and test it.

Creating pids in several applets sounds like bloat.
Rory Vieira
2006-09-06 09:04:08 UTC
Permalink
Natanael,
Post by Natanael Copa
This is probably because telnetd forks and returns the pid of the
parent. You can't use the pid (unless you can make telnetd create it for
you). Use --exec and --name instead.
Ok, for some reason this is still not working for me....
Another issue on my side, is that almost all bb daemons don't create a
/var/run/*.pid file... (inetd does though)...
And start-stop-daemon isn't doing the trick I want :(

I'm currently working on some patches to have bb daemons create their
respective pid files anyway. Obviously this is gonna cost me some
bytes, but I have them to spare :)

I started of with dnsd, as the code looks simple enough for me to edit (ugh)...
I copied the stuff from inetd to begin with, and it's currently
creating the pidfile as expected (huray)....
Now I need to 'remove' the file when killed (AH)...
Not sure where that last unlink statement should go :(

Here's the patch (for now)...
diff -Naur ../busybox-1.2.1-orig/networking/dnsd.c ./networking/dnsd.c
--- ../busybox-1.2.1-orig/networking/dnsd.c 2006-07-01 00:42:02.000000000 +0200
+++ ./networking/dnsd.c 2006-09-06 10:47:37.000000000 +0200
@@ -28,6 +28,7 @@
static char *fileconf = "/etc/dnsd.conf";
#define LOCK_FILE "/var/run/dnsd.lock"
#define LOG_FILE "/var/log/dnsd.log"
+#define PID_FILE "/var/run/dnsd.pid"

enum {
MAX_HOST_LEN = 16, // longest host name allowed is 15
@@ -359,6 +360,7 @@
*/
static void interrupt(int x)
{
+ unlink(PID_FILE);
unlink(LOCK_FILE);
write(2, "interrupt exiting\n", 18);
exit(2);
@@ -392,7 +394,7 @@
fprintf(stderr,"fileconf: %s\n", fileconf);
}

- if(is_daemon())
+ if(is_daemon()) {
#ifdef BB_NOMMU
/* reexec for vfork() do continue parent */
vfork_daemon_rexec(1, 0, argc, argv, "-d");
@@ -400,6 +402,16 @@
bb_xdaemon(1, 0);
#endif

+ {
+ FILE *fp;
+
+ if ((fp = fopen (PID_FILE, "w")) != NULL) {
+ fprintf (fp, "%u\n", getpid ());
+ (void) fclose (fp);
+ }
+ }
+ }
+
dnsentryinit(is_verbose());

signal(SIGINT, interrupt);
@@ -453,6 +465,7 @@
}
} // end if
} // end while
+ unlink(PID_FILE);
return 0;
}

If I can get this to work, I will try and create patches for the other
daemons I use (syslogd,klogd etc)...

Thanks for any assistance
--
Rory Vieira
rory dot vieira at gmail dot com
Rory Vieira
2006-08-28 14:43:42 UTC
Permalink
Hi,

now that I've built the basic initramfs (from buildroot), I'm writing
scripts to start/stop services.

I'm trying to use start-stop-daemon to do this:

start-stop-daemon -S -x /usr/sbin/telnetd -m /var/run/telnetd.pid -p
/var/run/telnetd.pid

When checking the pid file, I noticed that the wrong pid is entered
(usually two less that it should be).
How can I come around this?
--
Rory Vieira
rory dot vieira at gmail dot com
Natanael Copa
2006-08-28 15:00:41 UTC
Permalink
Post by Rory Vieira
Hi,
now that I've built the basic initramfs (from buildroot), I'm writing
scripts to start/stop services.
start-stop-daemon -S -x /usr/sbin/telnetd -m /var/run/telnetd.pid -p
/var/run/telnetd.pid
When checking the pid file, I noticed that the wrong pid is entered
(usually two less that it should be).
How can I come around this?
This is probably because telnetd forks and returns the pid of the
parent. You can't use the pid (unless you can make telnetd create it for
you). Use --exec and --name instead.

btw... the "start-stop-daemon --stop --pidfile ...." is noisy. I'm not
sure how the debian start-stop-daemon is but in gentoo its more quiet by
default. (start-stop-daemon in gentoo is based on the debian one)

/var/home/ncopa $ sudo /etc/init.d/rsyncd stop
* Stopping rsyncd: stopped process in pidfile `/var/run/rsyncd.pid' (pid 26877).
ok.

It should look like:
* Stopping rsyncd: ok.

I have to either
1. patch busybox to behave. (or just be lazy and file a bug of it)
2. patch every init.d script (there are too many)
3. live with the ugly output and endusers future complains

--
Natanael Copa
Loading...