From owner-freebsd-current@FreeBSD.ORG Sun Jun 27 22:12:45 2010 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 7DF8C106564A for ; Sun, 27 Jun 2010 22:12:45 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from harmony.bsdimp.com (bsdimp.com [199.45.160.85]) by mx1.freebsd.org (Postfix) with ESMTP id 30C968FC0A for ; Sun, 27 Jun 2010 22:12:45 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by harmony.bsdimp.com (8.14.3/8.14.1) with ESMTP id o5RM8ZMv075320; Sun, 27 Jun 2010 16:08:35 -0600 (MDT) (envelope-from imp@bsdimp.com) Date: Sun, 27 Jun 2010 16:08:45 -0600 (MDT) Message-Id: <20100627.160845.256787458594170652.imp@bsdimp.com> To: yanefbsd@gmail.com From: "M. Warner Losh" In-Reply-To: References: <201006262229.09747.hselasky@c2i.net> X-Mailer: Mew version 6.3 on Emacs 22.3 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable Cc: freebsd-current@freebsd.org, hselasky@c2i.net Subject: Re: Patch for rc.d/devd on FreeBSD 9-current X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 27 Jun 2010 22:12:45 -0000 In message: Garrett Cooper writes: : On Sat, Jun 26, 2010 at 1:45 PM, Garrett Cooper = wrote: : > On Sat, Jun 26, 2010 at 1:29 PM, Hans Petter Selasky 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 /*