On Wed, Jul 09, 2014 at 03:14:31PM +0200, Michal Židek wrote:
> On 07/09/2014 11:43 AM, Jakub Hrozek wrote:
> >On Tue, Jul 08, 2014 at 09:53:50AM +0200, Sumit Bose wrote:
> >>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.
> >
>
> I think the behaviour with these patches makes sense and
> works as expected. Ack.
Thanks for the review, pushed to master:
6b57784f0f175275fd900eca21c77415e3a5ea52
9a990aa9f7e8c105e0cfeea8d8cbdc776c2d5d7a
>
> Michal
btw feel free to open the ticket you mentioned in office today about
better (more easier) configuration of the debugging..
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
