[SSSD] [PATCH] Fix log filling with error message

Simo Sorce ssorce at redhat.com
Mon Oct 4 11:45:32 UTC 2010


On Mon, 4 Oct 2010 12:14:22 +0200
Sumit Bose <sbose at redhat.com> wrote:

> On Mon, Oct 04, 2010 at 12:02:50PM +0200, Jan Zelený wrote:
> > I'm sending a patch solving ticket #599 for review. After I tried
> > some other solutions, this one seemed to be the most simple one
> > which didn't cause any problems.
> > 
> > My only concern was whether monitoring the LDAP socket will be
> > restored once the backend goes online. Looking at the code, it
> > should, because sdap_install_callbacks is called every time
> > sh->connected is set to true. But still I wanted to be sure. From
> > testing performed by Dmitri, it seems that I was right and the
> > backend is correctly restored.
> > 
> > Here is a complete test case:
> > 1. Disconnect from the network
> > 2. Try to log in
> > -- the data provider should now go offline
> > 3. Connect to the network
> > 4. Wait for the log filling message to appear in the log
> > -- ! The messsage should appear only once !
> > 5. If the message doesn't appear, repeat from step 1
> > 6. If the message appeared, try to log in
> > 7. If successful, check that the login was performed online and not
> > from cached credentials
> > 8. Step 7 might need to be repeated a couple times, it takes some
> > time for SSSD to go online again
> > 
> > Note: the reproduction of this issue might not be that easy. To
> > achieve the message appearing in the log, you would have to try
> > different scenarios (NB connected using VPN -> suspended -> woke up
> > in the company network -> suspended -> ...)
> > 
> > Jan
> 
> > From 9446ce9061cdabe7685fff9995543c5916339584 Mon Sep 17 00:00:00
> > 2001 From: Jan Zeleny <jzeleny at redhat.com>
> > Date: Mon, 4 Oct 2010 11:03:08 +0200
> > Subject: [PATCH] Disable events on ldap fd when offline.
> > 
> > Erase events on LDAP socket when backend is offline and an event
> > appears on the socket. Normally this would lead to infinite loop,
> > because event is present on the fd, but instead of being processed,
> > an error log is written and the program continues to wait for the
> > event.
> > 
> > Ticket: #599
> > ---
> >  src/providers/ldap/sdap.h           |    8 ++++++++
> >  src/providers/ldap/sdap_async.c     |    3 +++
> >  src/providers/ldap/sdap_fd_events.c |    8 --------
> >  3 files changed, 11 insertions(+), 8 deletions(-)
> > 
> > diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h
> > index 4426dac..276891a 100644
> > --- a/src/providers/ldap/sdap.h
> > +++ b/src/providers/ldap/sdap.h
> > @@ -82,6 +82,14 @@ struct sup_list {
> >      char **vals;
> >  };
> >  
> > +struct sdap_fd_events {
> > +#ifdef HAVE_LDAP_CONNCB
> > +    struct ldap_conncb *conncb;
> > +#else
> > +    struct tevent_fd *fde;
> > +#endif
> > +};
> > +
> 
> I would prefer a solution where this struct definition stays in
> sdap_fd_events.c. Would you mind to add a function to sdap_fd_events.c
> which does the talloc_free on fde similar to
> remove_ldap_connection_callbacks(). I haven't checked where
> remove_ldap_connection_callbacks() is used, maybe it makes sense to
> rename remove_ldap_connection_callbacks() and call
> talloc_zfree(sh->sdap_fd_events->fde) from here, too?

I second.
Please keep the structure private and use an auxiliary function instead.

Simo.


-- 
Simo Sorce * Red Hat, Inc * New York


More information about the sssd-devel mailing list