Date: Mon, 28 Jun 2010 09:58:53 -0700 From: Garrett Cooper <yanefbsd@gmail.com> To: "M. Warner Losh" <imp@bsdimp.com> Cc: freebsd-current@freebsd.org, hselasky@c2i.net Subject: Re: Patch for rc.d/devd on FreeBSD 9-current Message-ID: <AANLkTimpbAzY2gu8Fsly6wqDYZVg7C0ID-vzeH3A3N8s@mail.gmail.com> In-Reply-To: <20100627.160845.256787458594170652.imp@bsdimp.com> References: <201006262229.09747.hselasky@c2i.net> <AANLkTilVB_E-BiOtC-gENBQ7FdPTLcmu8qpmdwU1GyXd@mail.gmail.com> <AANLkTilnYGNz7V6z6AkeKsqUvOMN8yLvO57GM1gOIsTD@mail.gmail.com> <20100627.160845.256787458594170652.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Jun 27, 2010 at 3:08 PM, M. Warner Losh <imp@bsdimp.com> wrote:
> In message: <AANLkTilnYGNz7V6z6AkeKsqUvOMN8yLvO57GM1gOIsTD@mail.gmail.com=
>
> =A0 =A0 =A0 =A0 =A0 =A0Garrett Cooper <yanefbsd@gmail.com> writes:
> : On Sat, Jun 26, 2010 at 1:45 PM, Garrett Cooper <yanefbsd@gmail.com> wr=
ote:
> : > On Sat, Jun 26, 2010 at 1:29 PM, Hans Petter Selasky <hselasky@c2i.ne=
t> wrote:
> : >> Hi,
> : >>
> : >> Sometimes utilities that are started by devd require libraries outsi=
de
> : >> /usr/lib. One example is the webcamd utility which is started by dev=
d 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 appea=
r
> : > 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 =A0 (revision 209530)
> : +++ devd.cc =A0 (working copy)
> : @@ -739,6 +739,7 @@
> : =A0static void
> : =A0event_loop(void)
> : =A0{
> : + =A0 =A0 bool warn_user_about_cleanup;
> : =A0 =A0 =A0 int rv;
> : =A0 =A0 =A0 int fd;
> : =A0 =A0 =A0 char buffer[DEVCTL_MAXBUF];
> : @@ -804,6 +805,17 @@
> : =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 new_client(server_fd);
> : =A0 =A0 =A0 }
> : =A0 =A0 =A0 close(fd);
> : + =A0 =A0 close(server_fd);
> : +
> : + =A0 =A0 if (unlink(PIPE) =3D=3D 0) {
> : + =A0 =A0 =A0 =A0 =A0 =A0 cfg.remove_pidfile();
> : + =A0 =A0 =A0 =A0 =A0 =A0 warn_user_about_cleanup =3D false;
> : + =A0 =A0 } else
> : + =A0 =A0 =A0 =A0 =A0 =A0 warn_user_about_cleanup =3D true;
> : +
> : + =A0 =A0 if (warn_user_about_cleanup =3D=3D true)
> : + =A0 =A0 =A0 =A0 =A0 =A0 warn("cleanup failed");
> : +
> : =A0}
> :
> : =A0/*
>
> This patch is wrong. =A0The problems with it:
>
> (1) You don't need to unlink the pipe. =A0If you can't unlink it, then
> =A0 =A0you won't remove the pid file.
The reason why my previous patch deleted `PIPE' file was because it's
not a true pipe, but instead a named file in the filesystem, as that
never gets cleaned up otherwise (please see my latest post).
> (2) If devd dies suddenly (kill -9, segv, etc), then the pid file will
> =A0 =A0remain around, and devd will fail to start.
Correct.
> (3) i_really_do_not_like_names_this_long_esp_when_they_are_not_needed.
Fair enough :D.
> 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. =A0I'm not 100% sure the error handling is
> right, I'm still tracing through that path. =A0This seems to work.
Thanks!
-Garrett
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTimpbAzY2gu8Fsly6wqDYZVg7C0ID-vzeH3A3N8s>