On Mon, Jul 07, 2014 at 10:20:21PM +0200, Jakub Hrozek wrote:
> 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
Patches are attached. I found out the same issue also affected the CLI
tools and tests, which is fixed with the second patch.
In order to test, you need to run configure --with-initscript=systemd
--with-systemdunitdir=/usr/lib/systemd/system --with-syslog=journald
sssd -i -d <N> logs to stderr
sssd -i -f -d <N> logs to files
sssd -D -d <N> logs to journald
sssd -D -f -d <N> logs to files
It's not possible to log to journald when running on the foreground or
to stderr when running as deamon.