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:

1) 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.

2) 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