From owner-freebsd-current@FreeBSD.ORG Mon Jun 28 16:59:00 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 C3CE1106566C for ; Mon, 28 Jun 2010 16:59:00 +0000 (UTC) (envelope-from yanefbsd@gmail.com) Received: from mail-vw0-f54.google.com (mail-vw0-f54.google.com [209.85.212.54]) by mx1.freebsd.org (Postfix) with ESMTP id 7246C8FC13 for ; Mon, 28 Jun 2010 16:58:59 +0000 (UTC) Received: by vws13 with SMTP id 13so7904961vws.13 for ; Mon, 28 Jun 2010 09:58:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=9V7xAVJkfD6/cYPrYx6PvKe1vfwLmMhI+Baf4N6CDLc=; b=msQ0z/D6qnSXBWacg6EplVutPwxU8+TZOR4bNI41NVzLY9VvfsRk7ySjCbPnTVPKnt /JL4J1INnaxqXpkrDWr8DNgsbsvd8ajjMxJtJhZiBCHbsXxgkgSmcI7QZurj5YEk+e14 HbeLS3DdJec3wkWCh4+fFf2vZ4nliHPAWuBIk= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=OEJcxLatdB4d2nUbBkRefsoIFO3l5rPqo5c3Djh3wo46y3WZ79ZEqMXucJbzOHZpEF kFmUnjfE+ZAIzfr/YVKpnVvq2EiGZIJM4fF3DoSUq/g4h3ao5R6KtUmk8RcVy/HI3N8x wmNk/x6p26V2AQhK/xcluxjHn73tzSnuNdjYU= MIME-Version: 1.0 Received: by 10.224.126.219 with SMTP id d27mr3553341qas.155.1277744333154; Mon, 28 Jun 2010 09:58:53 -0700 (PDT) Received: by 10.229.80.75 with HTTP; Mon, 28 Jun 2010 09:58:53 -0700 (PDT) In-Reply-To: <20100627.160845.256787458594170652.imp@bsdimp.com> References: <201006262229.09747.hselasky@c2i.net> <20100627.160845.256787458594170652.imp@bsdimp.com> Date: Mon, 28 Jun 2010 09:58:53 -0700 Message-ID: From: Garrett Cooper To: "M. Warner Losh" 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: Mon, 28 Jun 2010 16:59:00 -0000 On Sun, Jun 27, 2010 at 3:08 PM, M. Warner Losh wrote: > In message: > =A0 =A0 =A0 =A0 =A0 =A0Garrett Cooper writes: > : On Sat, Jun 26, 2010 at 1:45 PM, Garrett Cooper wr= ote: > : > On Sat, Jun 26, 2010 at 1:29 PM, Hans Petter Selasky 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