Date: Sun, 27 Jun 2010 16:08:45 -0600 (MDT) From: "M. Warner Losh" <imp@bsdimp.com> To: yanefbsd@gmail.com Cc: freebsd-current@freebsd.org, hselasky@c2i.net Subject: Re: Patch for rc.d/devd on FreeBSD 9-current Message-ID: <20100627.160845.256787458594170652.imp@bsdimp.com> In-Reply-To: <AANLkTilnYGNz7V6z6AkeKsqUvOMN8yLvO57GM1gOIsTD@mail.gmail.com> References: <201006262229.09747.hselasky@c2i.net> <AANLkTilVB_E-BiOtC-gENBQ7FdPTLcmu8qpmdwU1GyXd@mail.gmail.com> <AANLkTilnYGNz7V6z6AkeKsqUvOMN8yLvO57GM1gOIsTD@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
In message: <AANLkTilnYGNz7V6z6AkeKsqUvOMN8yLvO57GM1gOIsTD@mail.gmail.c=
om>
Garrett Cooper <yanefbsd@gmail.com> writes:
: On Sat, Jun 26, 2010 at 1:45 PM, Garrett Cooper <yanefbsd@gmail.com> =
wrote:
: > On Sat, Jun 26, 2010 at 1:29 PM, Hans Petter Selasky <hselasky@c2i.=
net> wrote:
: >> Hi,
: >>
: >> Sometimes utilities that are started by devd require libraries out=
side
: >> /usr/lib. One example is the webcamd utility which is started by d=
evd upon USB
: >> device insertion. What do you think about the following patch:
: >>
: >> --- devd =A0 =A0 =A0 =A0(revision 209463)
: >> +++ devd =A0 =A0 =A0 =A0(local)
: >> @@ -4,7 +4,7 @@
: >> =A0#
: >>
: >> =A0# PROVIDE: devd
: >> -# REQUIRE: netif
: >> +# REQUIRE: netif ldconfig
: >> =A0# BEFORE: NETWORKING mountcritremote
: >> =A0# KEYWORD: nojail shutdown
: >
: > Funny since I was just monkeying around with this. This doesn't app=
ear
: > to be the only issue with devd:
: >
: > # devd
: > devd: devd already running, pid: 430
: > # pgrep 430; echo $?
: > 1
: > #
: >
: > Looks like /etc/rc.d/devd restart is broken :(...
: =
: This seems to fix that.
: Thanks,
: -Garrett
: =
: Index: devd.cc
: =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
: --- devd.cc (revision 209530)
: +++ devd.cc (working copy)
: @@ -739,6 +739,7 @@
: static void
: event_loop(void)
: {
: + bool warn_user_about_cleanup;
: int rv;
: int fd;
: char buffer[DEVCTL_MAXBUF];
: @@ -804,6 +805,17 @@
: new_client(server_fd);
: }
: close(fd);
: + close(server_fd);
: +
: + if (unlink(PIPE) =3D=3D 0) {
: + cfg.remove_pidfile();
: + warn_user_about_cleanup =3D false;
: + } else
: + warn_user_about_cleanup =3D true;
: +
: + if (warn_user_about_cleanup =3D=3D true)
: + warn("cleanup failed");
: +
: }
: =0C
: /*
This patch is wrong. The problems with it:
(1) You don't need to unlink the pipe. If you can't unlink it, then
you won't remove the pid file.
(2) If devd dies suddenly (kill -9, segv, etc), then the pid file will
remain around, and devd will fail to start.
(3) i_really_do_not_like_names_this_long_esp_when_they_are_not_needed.
The following works around the bug in pidfile_open, and allows me to
restart devd both in a nice shutdown mode, as well as preventing devd
from running multiple times. I'm not 100% sure the error handling is
right, I'm still tracing through that path. This seems to work.
Warner
Index: devd.cc
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- devd.cc (revision 209492)
+++ devd.cc (working copy)
@@ -376,11 +376,18 @@
if (_pidfile =3D=3D "")
return;
pfh =3D pidfile_open(_pidfile.c_str(), 0600, &otherpid);
- if (pfh =3D=3D NULL) {
- if (errno =3D=3D EEXIST)
+ if (pfh !=3D NULL)
+ return;
+ if (errno =3D=3D EEXIST) {
+ /*
+ * Work around a bug in libutil where it will return this
+ * condition when the other process does not, in fact,
+ * actually exist. Use kill(2) to see if it is there or not.
+ */
+ if (kill(otherpid, 0) =3D=3D 0)
errx(1, "devd already running, pid: %d", (int)otherpid);
+ } else
warn("cannot open pid file");
- }
}
=
void
@@ -804,6 +811,8 @@
new_client(server_fd);
}
close(fd);
+ close(server_fd);
+ cfg.remove_pidfile();
}
=0C
/*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100627.160845.256787458594170652.imp>