[SSSD] [PATCH] sss_client: Return a different error when sssd is not running

Lukas Slebodnik lslebodn at redhat.com
Mon Nov 24 08:51:05 UTC 2014


On (22/11/14 17:12), Jakub Hrozek wrote:
>On Tue, Nov 18, 2014 at 12:25:40PM +0100, Lukas Slebodnik wrote:
>> On (18/11/14 12:22), Lukas Slebodnik wrote:
>> >On (13/11/14 10:08), Simo Sorce wrote:
>> >>On Thu, 13 Nov 2014 10:44:45 +0100
>> >>Jakub Hrozek <jhrozek at redhat.com> wrote:
>> >>
>> >>> On Wed, Nov 12, 2014 at 08:04:46PM -0500, Simo Sorce wrote:
>> >>> > On Wed, 12 Nov 2014 16:36:00 +0100
>> >>> > Lukas Slebodnik <lslebodn at redhat.com> wrote:
>> >>> > 
>> >>> > > On (12/11/14 10:00), Simo Sorce wrote:
>> >>> > > >I would create a helper function to be called on return that
>> >>> > > >transforms the error accordingly. This will allow to write the
>> >>> > > >code _and_ the comment once.
>> >>> > > >
>> >>> > > In this case, Stephan's patch is better
>> >>> > > https://bugzilla.redhat.com/attachment.cgi?id=788567
>> >>> > 
>> >>> > Yes, this is a valid alternative.
>> >>> > 
>> >>> > > >The comment should be changed to something like this in either
>> >>> > > >case: /* When sssd is stopped return a safe error code as if sss
>> >>> > > >was not configured at all in nsswitch. This prevents bogus
>> >>> > > >errors from causing issues in applications, before sssd starts
>> >>> > > >or if it fails to respond. */
>> >>> > > >
>> >>> > > >No need to mention that sss is by default in nsswitch, as it is
>> >>> > > >not in all distributions and it really is inconsequential, the
>> >>> > > >same behaviour change hleps when sss is not the default but is
>> >>> > > >has been manually added and sssd is stopped or not started yet
>> >>> > > >(for example during boot).
>> >>> > > nss-pam-ldapd has the same behaviour in the same situation.
>> >>> > > Will we patch it as well? It's very likely we won't.
>> >>> > 
>> >>> > Sorry, I do not see how that matters :)
>> >>> > 
>> >>> > > The biggest problem is that sss is by default in nsswitch on
>> >>> > > fedora/rhel>=7 due to glibc caching and problem with GNOME,
>> >>> > > a) sssd-client is installed by default on this platforms.
>> >>> > > b) sssd need't be configured by default and in most cases won't be
>> >>> > >     => sssd cannot run
>> >>> > > c) glibc developers don't want to adjust the error return code in
>> >>> > > glibc
>> >>> > > 
>> >>> > > As a result of this, we need to patch sssd.
>> >>> > > I would say we should patch sssd just in downstream and
>> >>> > > Stephan's patch works well. I tested it.
>> >>> > 
>> >>> > Ok, then let's go with Stephen's patch.
>> >>> > 
>> >>> > Simo.
>> >>> > 
>> >>> 
>> >>> I don't personally mind whether we use Lukas' or Stephen's patch but I
>> >>> would /strongly/ prefer the patch to be pushed upstream.
>> >>> 
>> >>> Non-upstream patches should be considered valid only as a temporary
>> >>> workarounds..
>> >>
>> >>I was not aware the patch was not proposed for upstream. I strongly
>> >>disagree on this kind of patches being platform specific too.
>> >>
>> >New version of patch is attached.
>> >
>> Ups, one more time.
>> 
>> LS
>
>> From eb617a980d50c7081ecb6eda0d489fc0eb8963f7 Mon Sep 17 00:00:00 2001
>> From: Lukas Slebodnik <lslebodn at redhat.com>
>> Date: Tue, 18 Nov 2014 12:02:10 +0100
>> Subject: [PATCH] sss_client: Work around glibc bug
>> 
>> glibc is inconsistent with how it treats and returns NSS_STATUS_UNAVAIL.
>> 
>> The sss nss plugin is present in nsswitch by default on some platforms
>> due to glibc caching and problem with long living applications (e.g. GNOME).
>> The SSSD nss plugin should behave as if it was functioning but had no data even
>> thought sssd is not running. The errors have to be passed from nss plugin up to
>> the user with minimal moidiffication.
>> 
>> Thanks to Stephen Gallagher for initial patch.
>> 
>> Resolves:
>> https://fedorahosted.org/sssd/ticket/2439
>> ---
>>  src/conf_macros.m4      | 13 +++++++++++++
>>  src/sss_client/common.c | 10 ++++++++++
>>  2 files changed, 23 insertions(+)
>> 
>> diff --git a/src/conf_macros.m4 b/src/conf_macros.m4
>> index fbee81f56e484b618379f7c987ecee50ae48917e..bb283ef248ee017fd608bd85cc68666db0300fd9 100644
>> --- a/src/conf_macros.m4
>> +++ b/src/conf_macros.m4
>> @@ -708,6 +708,19 @@ AC_ARG_ENABLE([dbus-tests],
>>                [build_dbus_tests=yes])
>>  AM_CONDITIONAL([BUILD_DBUS_TESTS], [test x$build_dbus_tests = xyes])
>>  
>> +AC_ARG_ENABLE([sss-default-nss-plugin],
>> +              [AS_HELP_STRING([--enable-sss-default-nss-plugin],
>> +                              [This option change standard behaviour of sss nss
>> +                               plugin. If this option is enabled the sss nss
>> +                               plugin will behave as it was not in
>> +                               nsswitch.conf when sssd is not running.
>> +                               [default=no]])],
>> +              [],
>
>Didn't you mean to add:
>    enable_sss-default-nss-plugin=$enablevar
It means that default value will be used which is
"enable_sss-default-nss-plugin=$enablevar", but I can add it.
>
>here?
>
>Additionally, it would be nice to explain in the commit message why the
>feature is disabled -- explain the effect enabling by default would have
>on searches that don't find anything.
The commit message contains link to ticket and it containsall necessary
information. Should I copy all of them to commit message?

LS



More information about the sssd-devel mailing list