[SSSD] [PATCH] exit original process after sssd is initialized

Jakub Hrozek jhrozek at redhat.com
Fri Nov 2 10:16:07 UTC 2012


On Thu, Nov 01, 2012 at 05:36:10PM +0100, Pavel Březina wrote:
> On 10/30/2012 01:51 PM, Jakub Hrozek wrote:
> >On Fri, Oct 26, 2012 at 01:10:38PM +0200, Pavel Březina wrote:
> >>On 10/22/2012 01:49 PM, Pavel Březina wrote:
> >>>On 10/19/2012 02:20 PM, Simo Sorce wrote:
> >>>>On Fri, 2012-10-19 at 09:52 +0200, Pavel Březina wrote:
> >>>>>On 10/18/2012 08:43 PM, Stephen Gallagher wrote:
> >>>>>>On 10/18/2012 09:50 AM, Jakub Hrozek wrote:
> >>>>>>>On Thu, Oct 18, 2012 at 11:28:10AM +0200, Pavel Březina wrote:
> >>>>>>>>On 10/18/2012 11:15 AM, Pavel Březina wrote:
> >>>>>>>>>https://fedorahosted.org/sssd/ticket/1357
> >>>>>>>>>
> >>>>>>>>>Neither systemd or our init script use pid file as a notification
> >>>>>>>>>that sssd is finished initializing. They will continue starting up
> >>>>>>>>>next service right after the original process is terminated.
> >>>>>>>>
> >>>>>>>>Oops, I forgot to amend the patch with latest changes. The final
> >>>>>>>>patch is attached.
> >>>>>>>
> >>>>>>>I haven't really had time to read the patch too carefully yet, but my
> >>>>>>>first thought was to always try to use tevent signals if you need
> >>>>>>>signals at all.
> >>>>>>
> >>>>>>Yes, I went through a lot of effort a little over a year ago to get the
> >>>>>>monitor properly using tevent signals instead of managing its own
> >>>>>>directly. Please do not add manual handlers.
> >>>>>>
> >>>>>>Nack.
> >>>>>
> >>>>>Hi guys,
> >>>>>I just want to make sure that we are on the same page here before I
> >>>>>start modifying it to tevent.
> >>>>>
> >>>>>I am not mixing tevent signals with posix signals. There is no existing
> >>>>>tevent context available and there will never be (unless I create it).
> >>>>>
> >>>>>I just need to catch SIGTERM in original process (that forks and become
> >>>>>monitor), so that monitor can signal the original process to quit. Or
> >>>>>wait for the monitor to exit (in case of error etc.) if the signal
> >>>>>doesn't come.
> >>>>>
> >>>>>Using tevent for this case seems like using a sledgehammer to crack a
> >>>>>nut.
> >>>>
> >>>>Pavel if this is before we create the tevent event context than it is ok
> >>>>to directly handle signals, however put a comment there taht says so.
> >>>>example: /* We use signals directly here because we don't have a tevent
> >>>>context yet */
> >>>>
> >>>>Simo.
> >>>>
> >>>
> >>>Very well. New patch is attached.
> >>
> >>Hi,
> >>I'm sending new patch set.
> >>
> >>[PATCH 1/2] exit original process after sssd is initialized
> >>...unchanged.
> >>
> >>[PATCH 2/2] create pid file immediately after fork again
> >>We realized that sysv and systemd does not use pid file existence
> >>as a notification of finished initialization. Therefore, we create
> >>the pid file immediately after we fork to become daemon.
> >>
> >
> >Positive testing went fine. So far I only have one remark for the code --
> >is there a reason to keep the parent pid around at all? Why not simply
> >call getppid()?
> >
> >Also, starting the service doesn't work if any provider is
> >misconfigured, it seems that the parent never exits. I tested by simply
> >putting an invalid ldap URI (ldap=someserver instead of
> >ldap://someserver).
> 
> I'm sending another set of patches after our offline discussion.
> 
> 

With these set of patches both positive and negative (breaking
configuration, injecting kill(getpid(), SIGSEGV), etc) went fine.

I only have a couple of nitpicks about the code:
    * We should use new DEBUG macros in monitor_quit_signal()
    * DEBUG(SSSDBG_FATAL_FAILURE, ("Shutting down with: %d\n", ret));
      should say returned with 

I would also like to ask Simo for a second look as the startup issues
are delicate.



More information about the sssd-devel mailing list