On Mon, Jul 07, 2014 at 03:57:28PM +0200, Sumit Bose wrote:
> On Mon, Jul 07, 2014 at 03:21:54PM +0200, Jakub Hrozek wrote:
> > On Tue, Jun 03, 2014 at 09:53:13AM +0200, Jakub Hrozek wrote:
> > > On Mon, Feb 24, 2014 at 08:39:25AM -0500, Simo Sorce wrote:
> > > > On Mon, 2014-02-24 at 11:54 +0100, Jakub Hrozek wrote:
> > > > > On Thu, Feb 20, 2014 at 08:47:30AM -0500, Stephen Gallagher
wrote:
> > > > > > -----BEGIN PGP SIGNED MESSAGE-----
> > > > > > Hash: SHA1
> > > > > >
> > > > > > On 02/19/2014 05:18 PM, Lukas Slebodnik wrote:
> > > > > > > On (19/02/14 22:03), Sumit Bose wrote:
> > > > > > >> On Wed, Feb 19, 2014 at 09:04:48PM +0100, Lukas
Slebodnik wrote:
> > > > > > >>> On (19/02/14 12:37), Jakub Hrozek wrote:
> > > > > > >>>> On Wed, Feb 19, 2014 at 12:02:23PM +0100,
Jakub Hrozek
> > > > > > >>>> wrote:
> > > > > > >>>>>> I realized I made a mistake in the
rebasing above. I
> > > > > > >>>>>> fixed that and then needed to do
another manual rebase of
> > > > > > >>>>>> these patches atop it. These
patches apply atop the
> > > > > > >>>>>> "[SSSD] DEBUG macro
refactoring v6" patches.
> > > > > > >>>>>
> > > > > > >>>>> These patches work for me, so ACK.
> > > > > > >>>>
> > > > > > >>>> Pushed to master.
> > > > > > >>>
> > > > > > >>> find_uid-tests failed with these patches on
fedora20.
> > > > > > >>>
http://kojipkgs.fedoraproject.org//work/tasks/8367/6548367/build.log
> > > > > > >>
> > > > > > >>
> > > > > > >>>
> > > > > > Do you have additional patches in this build?
> > > > > > >>
> > > > > > >>
/builddir/build/SRPMS/sssd-1.11.90-0.20140219.1816.git22a9323._temp.fc20.src.rpm
> > > > > > >>
> > > > > > >>
> > > > > > >>
> > > > > > The git hash does not look like anything from master.
> > > > > > >>
> > > > > > > Yes, I was testing some patches.
> > > > > > >
> > > > > > >> I started a koji build
> > > > > > >>
http://koji.fedoraproject.org/koji/taskinfo?taskID=6549081 with
> > > > > > >> current master
(4cde267bec52ae1723a125d19439a5c75b47ebb7) which
> > > > > > >> is working fine.
> > > > > > >>
> > > > > > > I tried one more time with master and koji works fine.
> > > > > > > Unfortunately, I am able to reproduce it locally in
mock :-(
> > > > > > >
> > > > > >
> > > > > >
> > > > > > I've seen that happen intermittently as well in mock
and koji. I
> > > > > > suspect there may be a bug/race condition in
sd_uid_get_sessions(),
> > > > > > but I can't reproduce it consistently (and the DEBUG
error doesn't
> > > > > > seem to fire...)
> > > > > >
> > > > > > A quick look at the code suggests that we're probably
missing a
> > > > > > 'return EOK' after '*result = false',
though.
> > > > > >
> > > > > > I'll run a couple tests and see if that fixes the
issue...
> > > > >
> > > > > I found another issue related to journald support..if you
compile sssd
> > > > > with journald support and run it on the foreground (-i) then the
debug
> > > > > messages are still redirected to journal. That seems strange to
me, but
> > > > > also in line with what a comment in the code says:
> > > > >
> > > > > /* If we are not outputting logs to files, we should be
sending them
> > > > > * to journald.
> > > > > * NOTE: on modern systems, this is where
> > > > > * stdout/stderr will end up
> > > > >
> > > > > So I'm not entirely sure about the expected behaviour, but I
would
> > > > > prefer if there was still a way to see the debug messages
directly on
> > > > > the console.
> > > > >
> > > > > I hacked up a patch that also adds a third state where if sssd
is running
> > > > > on the foreground even with journald support, then just fprintf
is used.
> > > > > See the attachment.
> > > >
> > > > Wouldn't it be simpler to just set debug_file = stderr ?
> > >
> > > I tried to go this way, but it seems actually harder to me..
> > >
> > > To summarize:
> > > with "sssd -f" debug_file should be a log file
> > > with "sssd" debug_file should be NULL, indicating journald
> > > with "sssd -i", debug_file should be stderr, indicating stderr.
> > >
> > > The problem is, that we don't pass the "-i" flag down to the
responders
> > > and back ends when forking them. So the alternative would be to add
"-i"
> > > to sssd_be and all responders, pass the flag to the child processes and
> > > only set debug_file to stderr if '-i' is present. If you prefer
this way,
> > > I can prepare a patch along these lines.
> > >
> > > But then we also terminate the process which is running with
"-i" if its
> > > stdin goes away, I guess it would be safe to keep this logic since stdin
> > > in the child processes is /dev/null already, I just wanted to mention
> > > there is a number of cases to test..
> >
> > Any opinions? I'd like to fix this bug before tagging 1.12.0..
>
> Maybe having an option --debug-to-stdout would be nicer for the child
> processes because it fits better to the existing SSSD_DEBUG_OPTS and
> will have no additional meaning as -i? For the monitor this option
> should be rejected and hidden from the help.
>
> bye,
> Sumit
Perhaps..
to clarify, you're suggesting that --debug-to-stdout would be added to
all processes (monitor and child), but only child processes in libexec
would expose it and this option would be used when spawning the child
processes?
yes, my idea was to add it to SSSD_DEBUG_OPTS which would make it
immediately available to all processes. But having it for the monitor
make no sense because '-D --debug-to-stdout' and '-f --debug-to-stdout'
are config errors and '-i --debug-to-stdout' is a no-op.
bye,
Sumit
I wouldn't like to change the meaning of "-i" for the monitor process,
though..but there we can detect sssd is running on the foreground.
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel