Re: [SSSD] [PATCH] Reactivate old fd handling conditionally
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 02/05/2010 07:22 AM, Stephen Gallagher wrote: > On 02/05/2010 07:04 AM, Sumit Bose wrote: >>> >>> man page change and debug message are in the second patch. >>> > >> sorry, this new version fixes a compiler warning. > > > Ack to both. > > Please open a ticket explaining this issue and mark it as "Affects tests". > Pushed to master. - -- Stephen Gallagher RHCE 804006346421761 Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAktsPEsACgkQeiVVYja6o6OWGQCgjjlcH08bbL2ErU/0O6tZ3wGJ rzkAoJkBwG+y3bM7XCk1HEoadqfMEJfu =xYp2 -END PGP SIGNATURE- ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Reactivate old fd handling conditionally
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 02/05/2010 07:04 AM, Sumit Bose wrote: >> >> man page change and debug message are in the second patch. >> > > sorry, this new version fixes a compiler warning. > Ack to both. Please open a ticket explaining this issue and mark it as "Affects tests". - -- Stephen Gallagher RHCE 804006346421761 Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAktsDZoACgkQeiVVYja6o6MEtACfTBSqFf48Eo03GWm/Z9vlNWAe 4WEAn0saRDanb1FtBXtnumtE4uK/BFl2 =YZ3Z -END PGP SIGNATURE- ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Reactivate old fd handling conditionally
On Thu, Feb 04, 2010 at 08:13:15PM +0100, Sumit Bose wrote: > On Thu, Feb 04, 2010 at 09:02:51AM -0500, Stephen Gallagher wrote: > > -BEGIN PGP SIGNED MESSAGE- > > Hash: SHA1 > > > > On 02/04/2010 08:15 AM, Sumit Bose wrote: > > > Hi, > > > > > > this path reintroduces the old way of handling the file description of a > > > LDAP connection if the connection callback is not available. > > > > > > Using the old way we cannot handle LDAP referrals and I think we should > > > generate a DEBUG message if ldap_referrals is set to 'true' and add a > > > hint to the man page. Do you agree? I didn't put these changes in the > > > patch to make the review easier because if can be compared line by line > > > with commit 7c8f422495347e6ff829246ebf5d7faad9f6d160 'Use ldap > > > connection callbacks to get file descriptors'. > > > > > > bye, > > > Sumit > > > > > > > The patch looks fine. I'd like to see the man page specify that > > ldap_referrals do not work for openldap libraries older than 2.4.12. > > > > The debug message should reflect the same. > > > > Please resubmit with these two changes. > > > > > > man page change and debug message are in the second patch. > sorry, this new version fixes a compiler warning. bye, Sumit From e162de2fbf6e03bc5426537b46692db38315f5dd Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Thu, 4 Feb 2010 11:53:36 +0100 Subject: [PATCH 1/2] Reactivate old fd handling conditionally Older versions of openLDAP do not provide a connection callback. This patch adds a configure check to see if the callback is available and activates the old way of handling the file description of the LDAP connection. This also means that it is not possible to follow referrals. --- server/external/ldap.m4 |4 ++ server/providers/ldap/sdap.h |4 ++ server/providers/ldap/sdap_async.c| 44 + server/providers/ldap/sdap_async_connection.c | 25 +- server/providers/ldap/sdap_async_private.h|6 +++ 5 files changed, 82 insertions(+), 1 deletions(-) diff --git a/server/external/ldap.m4 b/server/external/ldap.m4 index a17ed7e..ee425d8 100644 --- a/server/external/ldap.m4 +++ b/server/external/ldap.m4 @@ -44,6 +44,10 @@ SAVE_LIBS=$LIBS CFLAGS="$CFLAGS $OPENLDAP_CFLAGS" LIBS="$LIBS $OPENLDAP_LIBS" AC_CHECK_FUNCS([ldap_control_create]) +AC_CHECK_MEMBERS([struct ldap_conncb.lc_arg], + [AC_DEFINE([HAVE_LDAP_CONNCB], [1], + [Define if LDAP connection callbacks are available])], + [], [[#include ]]) CFLAGS=$SAVE_CFLAGS LIBS=$SAVE_LIBS diff --git a/server/providers/ldap/sdap.h b/server/providers/ldap/sdap.h index f32ce05..16dbb78 100644 --- a/server/providers/ldap/sdap.h +++ b/server/providers/ldap/sdap.h @@ -71,7 +71,11 @@ struct sdap_handle { LDAP *ldap; bool connected; +#ifdef HAVE_LDAP_CONNCB struct ldap_conncb *conncb; +#else +struct tevent_fd *fde; +#endif struct sdap_op *ops; }; diff --git a/server/providers/ldap/sdap_async.c b/server/providers/ldap/sdap_async.c index fd8c11e..88f1c4b 100644 --- a/server/providers/ldap/sdap_async.c +++ b/server/providers/ldap/sdap_async.c @@ -97,8 +97,12 @@ static void sdap_handle_release(struct sdap_handle *sh) if (sh->connected) { struct sdap_op *op; +#ifdef HAVE_LDAP_CONNCB /* remove all related fd events from the event loop */ talloc_zfree(sh->conncb->lc_arg); +#else +talloc_zfree(sh->fde); +#endif while (sh->ops) { op = sh->ops; @@ -111,7 +115,9 @@ static void sdap_handle_release(struct sdap_handle *sh) if (sh->ldap) { ldap_unbind_ext(sh->ldap, NULL, NULL); } +#ifdef HAVE_LDAP_CONNCB talloc_zfree(sh->conncb); +#endif sh->connected = false; sh->ldap = NULL; sh->ops = NULL; @@ -330,6 +336,7 @@ static void sdap_process_next_reply(struct tevent_context *ev, op->callback(op, op->list, EOK, op->data); } +#ifdef HAVE_LDAP_CONNCB int sdap_ldap_connect_callback_add(LDAP *ld, Sockbuf *sb, LDAPURLDesc *srv, struct sockaddr *addr, struct ldap_conncb *ctx) { @@ -404,6 +411,43 @@ void sdap_ldap_connect_callback_del(LDAP *ld, Sockbuf *sb, return; } +#else + +static int get_fd_from_ldap(LDAP *ldap, int *fd) +{ +int ret; + +ret = ldap_get_option(ldap, LDAP_OPT_DESC, fd); +if (ret != LDAP_OPT_SUCCESS) { +DEBUG(1, ("Failed to get fd from ldap!!\n")); +*fd = -1; +return EIO; +} + +return EOK; +} + +int sdap_install_ldap_callbacks(struct sdap_handle *sh, +struct tevent_context *ev) +{ +int fd; +int ret; + +ret = get_fd_from_ldap(sh->ldap, &fd); +if (ret) return ret; + +sh->fde = tevent_add_fd(ev, sh, fd, TEVENT_FD_READ, sdap_ldap_result, sh); +if (!sh->fde) return ENOMEM; + +DEBUG(8, ("Trace: sh[%p], connec
Re: [SSSD] [PATCH] Reactivate old fd handling conditionally
On Thu, Feb 04, 2010 at 09:02:51AM -0500, Stephen Gallagher wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA1 > > On 02/04/2010 08:15 AM, Sumit Bose wrote: > > Hi, > > > > this path reintroduces the old way of handling the file description of a > > LDAP connection if the connection callback is not available. > > > > Using the old way we cannot handle LDAP referrals and I think we should > > generate a DEBUG message if ldap_referrals is set to 'true' and add a > > hint to the man page. Do you agree? I didn't put these changes in the > > patch to make the review easier because if can be compared line by line > > with commit 7c8f422495347e6ff829246ebf5d7faad9f6d160 'Use ldap > > connection callbacks to get file descriptors'. > > > > bye, > > Sumit > > > > The patch looks fine. I'd like to see the man page specify that > ldap_referrals do not work for openldap libraries older than 2.4.12. > > The debug message should reflect the same. > > Please resubmit with these two changes. > > man page change and debug message are in the second patch. bye, Sumit > - -- > Stephen Gallagher > RHCE 804006346421761 > > Delivering value year after year. > Red Hat ranks #1 in value among software vendors. > http://www.redhat.com/promo/vendor/ > -BEGIN PGP SIGNATURE- > Version: GnuPG v1.4.10 (GNU/Linux) > Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ > > iEYEARECAAYFAktq04sACgkQeiVVYja6o6OOuwCdGDpYQNYgRCrKlD8fq6WPFiGi > FEYAn1HWCbdPyiWJ+6Dm6qGbWBvZa5Gf > =f6sA > -END PGP SIGNATURE- > ___ > sssd-devel mailing list > sssd-devel@lists.fedorahosted.org > https://fedorahosted.org/mailman/listinfo/sssd-devel From e162de2fbf6e03bc5426537b46692db38315f5dd Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Thu, 4 Feb 2010 11:53:36 +0100 Subject: [PATCH 1/2] Reactivate old fd handling conditionally Older versions of openLDAP do not provide a connection callback. This patch adds a configure check to see if the callback is available and activates the old way of handling the file description of the LDAP connection. This also means that it is not possible to follow referrals. --- server/external/ldap.m4 |4 ++ server/providers/ldap/sdap.h |4 ++ server/providers/ldap/sdap_async.c| 44 + server/providers/ldap/sdap_async_connection.c | 25 +- server/providers/ldap/sdap_async_private.h|6 +++ 5 files changed, 82 insertions(+), 1 deletions(-) diff --git a/server/external/ldap.m4 b/server/external/ldap.m4 index a17ed7e..ee425d8 100644 --- a/server/external/ldap.m4 +++ b/server/external/ldap.m4 @@ -44,6 +44,10 @@ SAVE_LIBS=$LIBS CFLAGS="$CFLAGS $OPENLDAP_CFLAGS" LIBS="$LIBS $OPENLDAP_LIBS" AC_CHECK_FUNCS([ldap_control_create]) +AC_CHECK_MEMBERS([struct ldap_conncb.lc_arg], + [AC_DEFINE([HAVE_LDAP_CONNCB], [1], + [Define if LDAP connection callbacks are available])], + [], [[#include ]]) CFLAGS=$SAVE_CFLAGS LIBS=$SAVE_LIBS diff --git a/server/providers/ldap/sdap.h b/server/providers/ldap/sdap.h index f32ce05..16dbb78 100644 --- a/server/providers/ldap/sdap.h +++ b/server/providers/ldap/sdap.h @@ -71,7 +71,11 @@ struct sdap_handle { LDAP *ldap; bool connected; +#ifdef HAVE_LDAP_CONNCB struct ldap_conncb *conncb; +#else +struct tevent_fd *fde; +#endif struct sdap_op *ops; }; diff --git a/server/providers/ldap/sdap_async.c b/server/providers/ldap/sdap_async.c index fd8c11e..88f1c4b 100644 --- a/server/providers/ldap/sdap_async.c +++ b/server/providers/ldap/sdap_async.c @@ -97,8 +97,12 @@ static void sdap_handle_release(struct sdap_handle *sh) if (sh->connected) { struct sdap_op *op; +#ifdef HAVE_LDAP_CONNCB /* remove all related fd events from the event loop */ talloc_zfree(sh->conncb->lc_arg); +#else +talloc_zfree(sh->fde); +#endif while (sh->ops) { op = sh->ops; @@ -111,7 +115,9 @@ static void sdap_handle_release(struct sdap_handle *sh) if (sh->ldap) { ldap_unbind_ext(sh->ldap, NULL, NULL); } +#ifdef HAVE_LDAP_CONNCB talloc_zfree(sh->conncb); +#endif sh->connected = false; sh->ldap = NULL; sh->ops = NULL; @@ -330,6 +336,7 @@ static void sdap_process_next_reply(struct tevent_context *ev, op->callback(op, op->list, EOK, op->data); } +#ifdef HAVE_LDAP_CONNCB int sdap_ldap_connect_callback_add(LDAP *ld, Sockbuf *sb, LDAPURLDesc *srv, struct sockaddr *addr, struct ldap_conncb *ctx) { @@ -404,6 +411,43 @@ void sdap_ldap_connect_callback_del(LDAP *ld, Sockbuf *sb, return; } +#else + +static int get_fd_from_ldap(LDAP *ldap, int *fd) +{ +int ret; + +ret = ldap_get_option(ldap, LDAP_OPT_DESC, fd); +if (ret != LDAP_OPT_SUCCESS) { +DEBUG(1, ("Failed to get fd from ldap!!\n"
Re: [SSSD] [PATCH] Reactivate old fd handling conditionally
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 02/04/2010 08:15 AM, Sumit Bose wrote: > Hi, > > this path reintroduces the old way of handling the file description of a > LDAP connection if the connection callback is not available. > > Using the old way we cannot handle LDAP referrals and I think we should > generate a DEBUG message if ldap_referrals is set to 'true' and add a > hint to the man page. Do you agree? I didn't put these changes in the > patch to make the review easier because if can be compared line by line > with commit 7c8f422495347e6ff829246ebf5d7faad9f6d160 'Use ldap > connection callbacks to get file descriptors'. > > bye, > Sumit > The patch looks fine. I'd like to see the man page specify that ldap_referrals do not work for openldap libraries older than 2.4.12. The debug message should reflect the same. Please resubmit with these two changes. - -- Stephen Gallagher RHCE 804006346421761 Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAktq04sACgkQeiVVYja6o6OOuwCdGDpYQNYgRCrKlD8fq6WPFiGi FEYAn1HWCbdPyiWJ+6Dm6qGbWBvZa5Gf =f6sA -END PGP SIGNATURE- ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
[SSSD] [PATCH] Reactivate old fd handling conditionally
Hi, this path reintroduces the old way of handling the file description of a LDAP connection if the connection callback is not available. Using the old way we cannot handle LDAP referrals and I think we should generate a DEBUG message if ldap_referrals is set to 'true' and add a hint to the man page. Do you agree? I didn't put these changes in the patch to make the review easier because if can be compared line by line with commit 7c8f422495347e6ff829246ebf5d7faad9f6d160 'Use ldap connection callbacks to get file descriptors'. bye, Sumit From e162de2fbf6e03bc5426537b46692db38315f5dd Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Thu, 4 Feb 2010 11:53:36 +0100 Subject: [PATCH] Reactivate old fd handling conditionally Older versions of openLDAP do not provide a connection callback. This patch adds a configure check to see if the callback is available and activates the old way of handling the file description of the LDAP connection. This also means that it is not possible to follow referrals. --- server/external/ldap.m4 |4 ++ server/providers/ldap/sdap.h |4 ++ server/providers/ldap/sdap_async.c| 44 + server/providers/ldap/sdap_async_connection.c | 25 +- server/providers/ldap/sdap_async_private.h|6 +++ 5 files changed, 82 insertions(+), 1 deletions(-) diff --git a/server/external/ldap.m4 b/server/external/ldap.m4 index a17ed7e..ee425d8 100644 --- a/server/external/ldap.m4 +++ b/server/external/ldap.m4 @@ -44,6 +44,10 @@ SAVE_LIBS=$LIBS CFLAGS="$CFLAGS $OPENLDAP_CFLAGS" LIBS="$LIBS $OPENLDAP_LIBS" AC_CHECK_FUNCS([ldap_control_create]) +AC_CHECK_MEMBERS([struct ldap_conncb.lc_arg], + [AC_DEFINE([HAVE_LDAP_CONNCB], [1], + [Define if LDAP connection callbacks are available])], + [], [[#include ]]) CFLAGS=$SAVE_CFLAGS LIBS=$SAVE_LIBS diff --git a/server/providers/ldap/sdap.h b/server/providers/ldap/sdap.h index f32ce05..16dbb78 100644 --- a/server/providers/ldap/sdap.h +++ b/server/providers/ldap/sdap.h @@ -71,7 +71,11 @@ struct sdap_handle { LDAP *ldap; bool connected; +#ifdef HAVE_LDAP_CONNCB struct ldap_conncb *conncb; +#else +struct tevent_fd *fde; +#endif struct sdap_op *ops; }; diff --git a/server/providers/ldap/sdap_async.c b/server/providers/ldap/sdap_async.c index fd8c11e..88f1c4b 100644 --- a/server/providers/ldap/sdap_async.c +++ b/server/providers/ldap/sdap_async.c @@ -97,8 +97,12 @@ static void sdap_handle_release(struct sdap_handle *sh) if (sh->connected) { struct sdap_op *op; +#ifdef HAVE_LDAP_CONNCB /* remove all related fd events from the event loop */ talloc_zfree(sh->conncb->lc_arg); +#else +talloc_zfree(sh->fde); +#endif while (sh->ops) { op = sh->ops; @@ -111,7 +115,9 @@ static void sdap_handle_release(struct sdap_handle *sh) if (sh->ldap) { ldap_unbind_ext(sh->ldap, NULL, NULL); } +#ifdef HAVE_LDAP_CONNCB talloc_zfree(sh->conncb); +#endif sh->connected = false; sh->ldap = NULL; sh->ops = NULL; @@ -330,6 +336,7 @@ static void sdap_process_next_reply(struct tevent_context *ev, op->callback(op, op->list, EOK, op->data); } +#ifdef HAVE_LDAP_CONNCB int sdap_ldap_connect_callback_add(LDAP *ld, Sockbuf *sb, LDAPURLDesc *srv, struct sockaddr *addr, struct ldap_conncb *ctx) { @@ -404,6 +411,43 @@ void sdap_ldap_connect_callback_del(LDAP *ld, Sockbuf *sb, return; } +#else + +static int get_fd_from_ldap(LDAP *ldap, int *fd) +{ +int ret; + +ret = ldap_get_option(ldap, LDAP_OPT_DESC, fd); +if (ret != LDAP_OPT_SUCCESS) { +DEBUG(1, ("Failed to get fd from ldap!!\n")); +*fd = -1; +return EIO; +} + +return EOK; +} + +int sdap_install_ldap_callbacks(struct sdap_handle *sh, +struct tevent_context *ev) +{ +int fd; +int ret; + +ret = get_fd_from_ldap(sh->ldap, &fd); +if (ret) return ret; + +sh->fde = tevent_add_fd(ev, sh, fd, TEVENT_FD_READ, sdap_ldap_result, sh); +if (!sh->fde) return ENOMEM; + +DEBUG(8, ("Trace: sh[%p], connected[%d], ops[%p], fde[%p], ldap[%p]\n", + sh, (int)sh->connected, sh->ops, sh->fde, sh->ldap)); + +return EOK; +} + +#endif + + /* ==LDAP-Operations-Helpers== */ static int sdap_op_destructor(void *mem) diff --git a/server/providers/ldap/sdap_async_connection.c b/server/providers/ldap/sdap_async_connection.c index 1ed6b3f..18e47d3 100644 --- a/server/providers/ldap/sdap_async_connection.c +++ b/server/providers/ldap/sdap_async_connection.c @@ -56,7 +56,6 @@ struct tevent_req *sdap_connect_send(TALLOC_CTX *memctx, int lret; int ret = EOK; int msgid; -struct ldap_cb_data *cb_data; bool ldap_referrals; req = tevent