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(a)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?
bye,
Sumit