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.