An updates series of patches is attached:
0001-GSSAPI-ticket-expiry-time-is-returned-from-ldap_chil.patch 0002-Added-an-interface-to-query-number-of-configured-and.patch 0003-LDAP-connection-usage-tracking-sharing-and-failover-.patch 0004-Add-an-interface-to-try-next-fail-over-server-after-.patch 0005-Use-new-LDAP-connection-framework-to-get-user-accoun.patch 0006-Use-new-LDAP-connection-framework-to-get-group-accou.patch 0007-Use-new-LDAP-connection-framework-to-get-user-accoun.patch 0008-Use-new-LDAP-connection-framework-for-LDAP-user-and-.patch 0009-Use-new-LDAP-connection-framework-in-LDAP-access-bac.patch 0010-Use-new-LDAP-connection-framework-in-IPA-access-back.patch 0011-Use-new-LDAP-connection-framework-in-IPA-dynamic-DNS.patch 0012-Remove-remainder-of-now-unused-global-LDAP-connectio.patch
Out of them slightly changed (nitpicks): 0003-LDAP-connection-usage-tracking-sharing-and-failover-.patch (2 lines in sdap_id_op.h) 0005-Use-new-LDAP-connection-framework-to-get-user-accoun.patch (according to Stephen comments) 0006-Use-new-LDAP-connection-framework-to-get-group-accou.patch (according to Stephen comments) 0007-Use-new-LDAP-connection-framework-to-get-user-accoun.patch (according to Stephen comments)
Rewritten: 0010-Use-new-LDAP-connection-framework-in-IPA-access-back.patch 0011-Use-new-LDAP-connection-framework-in-IPA-dynamic-DNS.patch
IPA access changes has been retested. In dynamic DNS I have tested code establishing LDAP connection, but was not able to test dynamic DNS update itself as I do not have test environment for that.
Eugene
On 07/06/2010 11:17 PM, Stephen Gallagher wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 07/02/2010 11:35 AM, Eugene Indenbom wrote:
The complete set of patches for LDAP connection framework is attached:
0001-GSSAPI-ticket-expiry-time-is-returned-from-ldap_chil.patch
Ack.
0002-Added-an-interface-to-query-number-of-configured-and.patch
Ack.
0003-LDAP-connection-usage-tracking-sharing-and-failover-.patch
Ack.
0004-Add-an-interface-to-try-next-fail-over-server-after-.patch
Ack.
0005-Use-new-LDAP-connection-framework-to-get-user-accoun.patch
Nitpick: in users_connect_get_done()
- int ret, dp_error = DP_ERR_FATAL;
Please either use: int ret, dp_error; or int ret; int dp_error = DP_ERR_FATAL;
It's easier to read.
However, since dp_error is assigned in the very next line (in a way that can't fail) I don't see a good reason to initialize it here.
0006-Use-new-LDAP-connection-framework-to-get-group-accou.patch
Same nitpick as patch 5.
0007-Use-new-LDAP-connection-framework-to-get-user-accoun.patch
Same nitpick as patch 5.
0008-Use-new-LDAP-connection-framework-for-LDAP-user-and-.patch
Ack.
0009-Use-new-LDAP-connection-framework-in-LDAP-access-bac.patch
Ack.
0010-Use-new-LDAP-connection-framework-in-IPA-access-back.patch
Nack.
Using hbac_ctx->sdap_op == NULL to mean "operating in offline mode" is very hard to read. Please just set a boolean state like hbac_ctx->offline instead. When I read "if (!hbac_ctx->sdap_op)", my natural expectation is for the following lines to set up the sdap_op.
I'm not finished reviewing this patch. This may take a bit more time.
0011-Use-new-LDAP-connection-framework-in-IPA-dynamic-DNS.patch
Nack.
There are several problems with this patch:
- When the dynamic DNS update code was written, it expected that when
we are online, we would always have an active LDAP connection. This is no longer guaranteed to be true. As we have discussed previously, it will be possible with this new interface (eventually) to be able to configure it so that connections to LDAP are not maintained after the currently-running set of operations completes. So it's possible that it will not be available to sdap_id_get_cached_connection() here. This function needs to be able to create a connection when necessary (so we'll need to convert part of the dynamic DNS update code to expect an async operation at this point.
- sdap_id_op_connect_cached() is never used anywhere. As mentioned
above, we should ask the connection cache if there is an available connection and using it if so, creating a new connection if not.
0012-Remove-remainder-of-now-unused-global-LDAP-connectio.patch
Ack.
Great work, Eugene. We're almost there :)
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 v2.0.14 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAkwzgTYACgkQeiVVYja6o6P8igCgqGLoLggH1wo2Q0R57AmAO2eN w6kAnjeuRKZ+rwEsSnpR8vRk8A3PUO+y =2fLp -----END PGP SIGNATURE----- _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel