From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:39133) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RjG73-0000k6-Cf for qemu-devel@nongnu.org; Fri, 06 Jan 2012 15:10:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RjG6s-000846-4r for qemu-devel@nongnu.org; Fri, 06 Jan 2012 15:10:05 -0500 Received: from e33.co.us.ibm.com ([32.97.110.151]:43897) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RjG6r-000840-O8 for qemu-devel@nongnu.org; Fri, 06 Jan 2012 15:09:53 -0500 Received: from /spool/local by e33.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 6 Jan 2012 13:09:52 -0700 Received: from d03av06.boulder.ibm.com (d03av06.boulder.ibm.com [9.17.195.245]) by d03relay05.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q06K9gVn063944 for ; Fri, 6 Jan 2012 13:09:44 -0700 Received: from d03av06.boulder.ibm.com (loopback [127.0.0.1]) by d03av06.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q06K9gpK010892 for ; Fri, 6 Jan 2012 13:09:42 -0700 Message-ID: <4F075505.3000100@linux.vnet.ibm.com> Date: Fri, 06 Jan 2012 14:09:41 -0600 From: Michael Roth MIME-Version: 1.0 References: <4F062390.6070007@intellilink.co.jp> <4F063DD2.3080104@linux.vnet.ibm.com> <20120106105619.GG14293@redhat.com> <4F0728BA.6020106@linux.vnet.ibm.com> <20120106170553.GA15223@redhat.com> <20120106170637.6f4b25fd@doriath> In-Reply-To: <20120106170637.6f4b25fd@doriath> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] GuestAgent: PIDFILE remains when daemon start fails List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: Kazuo Tanaka , "MATSUDA, Daiki" , qemu-devel@nongnu.org On 01/06/2012 01:06 PM, Luiz Capitulino wrote: > On Fri, 6 Jan 2012 17:05:53 +0000 > "Daniel P. Berrange" wrote: > >> On Fri, Jan 06, 2012 at 11:00:42AM -0600, Michael Roth wrote: >>> On 01/06/2012 04:56 AM, Daniel P. Berrange wrote: >>>> On Thu, Jan 05, 2012 at 06:18:26PM -0600, Michael Roth wrote: >>>>> On 01/05/2012 04:26 PM, MATSUDA, Daiki wrote: >>>>>> Hi, all. >>>>>> >>>>>> I am trying QEMU Guest Agent and encountered a small bug. It is that the >>>>>> PIDFILE remains when daemon start fails. And maybe forgotton to g_free(). >>>>>> >>>>>> MATSUDA, Daiki >>>>>> >>>>> >>>>> Thanks for the patch. There was some contention in the past about >>>>> whether or not to clean up pidfiles when there was abnormal >>>>> termination, but personally I like this approach better. > > Ok, but can't we use atexit() instead then? I guess I prefer it to this patch, but I don't believe that covers segfaults and the like, so maybe a combination of atexit() and F_SETLK would be best (as F_SETLK can still leave stale PID files, they just wouldn't obstruct subsequent instances, but we should still clean them up whenever we can) > >>>> >>>> Yep, this still leaves open the problem of pidfile cleanup when the >>>> daemon crashes. For libvirtd we recently switched over to a crash-safe >>>> pidfile acquisition design, that uses fcntl(F_SETLK) to maintain >>>> exclusive access over the pidfile. With this you don't need to worry >>>> about forgetting to unlink() on termination, since the POSIX lock is >>>> automatically released when process exits (or crashes). >>> >>> Yup, we did the same at some point via lockf(). An argument was made >>> that stale PID files from unresolved crashes should stick around, so >>> we dropped it. I think we should re-evaluate that decision...libvirt >>> taking the same approach is pretty good precedence for me. I don't >>> expect to have state from crashed programs interrupting attempts to >>> restart them, it's more an unpleasant surprise than a feature, I >>> think. > > Ok, I'll agree with you this time. Let's do it. > >> >> Yeah, I think that is rather unpleasant, particularly for something >> like qemu guest agent, which we want to try to ensure is reliably >> running. In any case, if qemu guest agent is being launched by >> something like SystemD, then you can configure whether systemd >> will auto-restart it when it dies with non-zero exit status, so >> I don't think we should delibrately leave stale pidfiles for that >> scenario. >> >> Regards, >> Daniel >