Hello all,

this series of 8 patches makes internal caching framework of the
bind-dyndb-ldap more flexible. This flexibility is required by future
persistent search changes.

Any comments are welcomed.

Regards, Adam
>From b5dafda69fe243fec9a0e861e68af29b57c72724 Mon Sep 17 00:00:00 2001
From: Adam Tkac <at...@redhat.com>
Date: Tue, 21 Jun 2011 13:10:10 +0200
Subject: [PATCH 1/8] Add ldap_cache_{add,get}rdatalist and ldap_cache_enabled
 functions.

Signed-off-by: Adam Tkac <at...@redhat.com>
---
 src/cache.c |   80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/cache.h |   29 ++++++++++++++++++++-
 2 files changed, 108 insertions(+), 1 deletions(-)

diff --git a/src/cache.c b/src/cache.c
index db6457b..cd45b71 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -76,6 +76,7 @@ cache_node_deleter(void *data, void *deleter_arg)
        MEM_PUT_AND_DETACH(node);
 }
 
+/* TODO: Remove the rdatalist parameter */
 static isc_result_t
 cache_node_create(ldap_cache_t *cache, ldapdb_rdatalist_t rdatalist,
                  cache_node_t **nodep)
@@ -231,6 +232,85 @@ cleanup:
 }
 
 isc_result_t
+ldap_cache_getrdatalist(isc_mem_t *mctx, ldap_cache_t *cache,
+                       dns_name_t *name, ldapdb_rdatalist_t *rdatalist)
+{
+       isc_result_t result;
+       ldapdb_rdatalist_t rdlist;
+       cache_node_t *node = NULL;
+       isc_time_t now;
+
+       REQUIRE(cache != NULL);
+
+       /* Return NOTFOUND if caching is disabled */
+       if (!ldap_cache_enabled(cache))
+               return ISC_R_NOTFOUND;
+
+       LOCK(&cache->mutex);
+       result = dns_rbt_findname(cache->rbt, name, 0, NULL, (void *)&node);
+       switch (result) {
+       case ISC_R_SUCCESS:
+               CHECK(isc_time_now(&now));
+               if (isc_time_compare(&now, &node->valid_until) > 0) {
+                       /* Delete expired records and treat them as NOTFOUND */
+                       CHECK(dns_rbt_deletename(cache->rbt, name, ISC_FALSE));
+                       result = ISC_R_NOTFOUND;
+               } else {
+                       rdlist = node->rdatalist;
+                       CHECK(ldap_rdatalist_copy(mctx, rdlist, rdatalist));
+                       INSIST(!EMPTY(*rdatalist)); /* Empty rdatalist 
indicates a bug */
+               }
+               break;
+       case DNS_R_PARTIALMATCH:
+               result = ISC_R_NOTFOUND;
+               /* Fall through */
+       case ISC_R_NOTFOUND:
+               goto cleanup;
+               /* Not reached */
+       default:
+               result = ISC_R_FAILURE;
+       }
+
+cleanup:
+       UNLOCK(&cache->mutex);
+       return result;
+}
+
+isc_boolean_t
+ldap_cache_enabled(ldap_cache_t *cache)
+{
+       return (cache->rbt != NULL) ? ISC_TRUE : ISC_FALSE;
+}
+
+isc_result_t
+ldap_cache_addrdatalist(ldap_cache_t *cache, dns_name_t *name,
+                       ldapdb_rdatalist_t *rdatalist)
+{
+       isc_result_t result;
+       ldapdb_rdatalist_t rdlist;
+       cache_node_t *node = NULL;
+
+       REQUIRE(cache != NULL);
+
+       if (!ldap_cache_enabled(cache))
+               return ISC_R_SUCCESS; /* Caching is disabled */
+
+       CHECK(cache_node_create(cache, rdlist, &node));
+       CHECK(ldap_rdatalist_copy(cache->mctx, *rdatalist, &node->rdatalist));
+
+       LOCK(&cache->mutex);
+       result = dns_rbt_addname(cache->rbt, name, (void *)node);
+       INSIST(result != ISC_R_EXISTS); /* Indicates a bug */
+       
+       UNLOCK(&cache->mutex);
+cleanup:
+       if (result != ISC_R_SUCCESS && node != NULL)
+               MEM_PUT_AND_DETACH(node);
+               
+       return result;
+}
+
+isc_result_t
 discard_from_cache(ldap_cache_t *cache, dns_name_t *name)
 {
        isc_result_t result;
diff --git a/src/cache.h b/src/cache.h
index 60a5aab..ac5cc0d 100644
--- a/src/cache.h
+++ b/src/cache.h
@@ -1,7 +1,8 @@
 /*
  * Authors: Martin Nagy <mn...@redhat.com>
+ *         Adam Tkac <at...@redhat.com>
  *
- * Copyright (C) 2009  Red Hat
+ * Copyright (C) 2009 - 2011  Red Hat
  * see file 'COPYING' for use and warranty information
  *
  * This program is free software; you can redistribute it and/or
@@ -49,6 +50,32 @@ cached_ldap_rdatalist_get(isc_mem_t *mctx, ldap_cache_t 
*cache,
                          dns_name_t *origin, ldapdb_rdatalist_t *rdatalist);
 
 /*
+ * Get rdatalist from cache.
+ *
+ * Returns ISC_R_SUCCESS, ISC_R_NOTFOUND or ISC_R_FAILURE.
+ */
+isc_result_t
+ldap_cache_getrdatalist(isc_mem_t *mctx, ldap_cache_t *cache,
+                       dns_name_t *name, ldapdb_rdatalist_t *rdatalist);
+
+/*
+ * Add rdatalist to the cache.
+ *
+ * Note: No rdatalist can be bound to the name.
+ *
+ * Returns ISC_R_SUCCESS, ISC_R_NOMEMORY and ISC_R_FAILURE.
+ */
+isc_result_t
+ldap_cache_addrdatalist(ldap_cache_t *cache, dns_name_t *name,
+                       ldapdb_rdatalist_t *rdatalist);
+
+/*
+ * Returns ISC_TRUE when cache is enabled.
+ */
+isc_boolean_t
+ldap_cache_enabled(ldap_cache_t *cache);
+
+/*
  * Discard 'name' from the cache. If caching is not really turned on or 'name'
  * is not cached, this function will still return ISC_R_SUCCESS.
  */
-- 
1.7.6

>From 6eeb2a1155e02db843720a46ca5dcaaf94cda2a4 Mon Sep 17 00:00:00 2001
From: Adam Tkac <at...@redhat.com>
Date: Tue, 21 Jun 2011 14:52:00 +0200
Subject: [PATCH 2/8] Replace cached_ldap_rdatalist_get calls by
 ldap_cache_{add,get}rdatalist.

Signed-off-by: Adam Tkac <at...@redhat.com>
---
 src/ldap_driver.c |   55 ++++++++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 46 insertions(+), 9 deletions(-)

diff --git a/src/ldap_driver.c b/src/ldap_driver.c
index e812038..819f66d 100644
--- a/src/ldap_driver.c
+++ b/src/ldap_driver.c
@@ -373,10 +373,29 @@ findnode(dns_db_t *db, dns_name_t *name, isc_boolean_t 
create,
 
        REQUIRE(VALID_LDAPDB(ldapdb));
 
-       result = cached_ldap_rdatalist_get(ldapdb->common.mctx,
-                                          ldapdb->ldap_cache, 
ldapdb->ldap_inst,
-                                          name, &ldapdb->common.origin,
-                                          &rdatalist);
+       result = ldap_cache_getrdatalist(ldapdb->common.mctx, 
ldapdb->ldap_cache,
+                                        name, &rdatalist);
+       switch (result) {
+       case ISC_R_SUCCESS:
+               goto cachehit;
+               /* Not reached */
+       case ISC_R_NOTFOUND:
+               goto cachemiss;
+               /* Not reached */
+       default:
+               goto cleanup;
+       }
+
+cachemiss:
+       INIT_LIST(rdatalist); /* XXX Should this be moved to 
ldapdb_rdatalist_get ? */
+       result = ldapdb_rdatalist_get(ldapdb->common.mctx, ldapdb->ldap_inst,
+                                     name, &ldapdb->common.origin,
+                                     &rdatalist);
+       /* Cache rdatalist if found. */
+       if (result == ISC_R_SUCCESS)
+               CHECK(ldap_cache_addrdatalist(ldapdb->ldap_cache, name,
+                                             &rdatalist));
+cachehit:
        if (create == ISC_FALSE) {
                if (result != ISC_R_SUCCESS)
                        goto cleanup;
@@ -424,12 +443,30 @@ find(dns_db_t *db, dns_name_t *name, dns_dbversion_t 
*version,
                REQUIRE(version == ldapdb_version);
        }
 
-       result = cached_ldap_rdatalist_get(ldapdb->common.mctx,
-                                          ldapdb->ldap_cache, 
ldapdb->ldap_inst,
-                                          name, &ldapdb->common.origin,
-                                          &rdatalist);
-       INSIST(result != DNS_R_PARTIALMATCH); /* XXX Not yet implemented */
+       result = ldap_cache_getrdatalist(ldapdb->common.mctx, 
ldapdb->ldap_cache,
+                                        name, &rdatalist);
+       switch (result) {
+       case ISC_R_SUCCESS:
+               goto cachehit;
+               /* Not reached */
+       case ISC_R_NOTFOUND:
+               goto cachemiss;
+               /* Not reached */
+       default:
+               goto cleanup;
+       }
 
+cachemiss:
+       /* XXX Move ldap_cache_addrdatalist into ldapdb_rdatalist_get. */
+       INIT_LIST(rdatalist); /* XXX Should this be moved to 
ldapdb_rdatalist_get ? */
+       result = ldapdb_rdatalist_get(ldapdb->common.mctx, ldapdb->ldap_inst,
+                                     name, &ldapdb->common.origin,
+                                     &rdatalist);
+       /* Cache rdatalist if found. */
+       if (result == ISC_R_SUCCESS)
+               CHECK(ldap_cache_addrdatalist(ldapdb->ldap_cache, name,
+                                             &rdatalist));
+cachehit:
        if (result != ISC_R_SUCCESS && result != DNS_R_PARTIALMATCH)
                return (result == ISC_R_NOTFOUND) ? DNS_R_NXDOMAIN : result;
 
-- 
1.7.6

>From 2bb149c21c0017998499dbc2e382c9907d4d63fa Mon Sep 17 00:00:00 2001
From: Adam Tkac <at...@redhat.com>
Date: Tue, 21 Jun 2011 14:53:48 +0200
Subject: [PATCH 3/8] Remove unused cached_ldap_rdatalist_get() function.

Signed-off-by: Adam Tkac <at...@redhat.com>
---
 src/cache.c |   84 -----------------------------------------------------------
 src/cache.h |   11 -------
 2 files changed, 0 insertions(+), 95 deletions(-)

diff --git a/src/cache.c b/src/cache.c
index cd45b71..863c9d9 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -36,21 +36,6 @@
 #include "settings.h"
 #include "util.h"
 
-/* These macros require that variable 'is_locked' exists. */
-#define CONTROLED_LOCK(lock)                                           \
-       do {                                                            \
-               LOCK(lock);                                             \
-               is_locked = 1;                                          \
-       } while (0)
-
-#define CONTROLED_UNLOCK(lock)                                         \
-       do {                                                            \
-               if (is_locked) {                                        \
-                       UNLOCK(lock);                                   \
-                       is_locked = 0;                                  \
-               }                                                       \
-       } while (0)
-
 struct ldap_cache {
        isc_mutex_t     mutex;
        isc_mem_t       *mctx;
@@ -163,75 +148,6 @@ destroy_ldap_cache(ldap_cache_t **cachep)
 }
 
 isc_result_t
-cached_ldap_rdatalist_get(isc_mem_t *mctx, ldap_cache_t *cache,
-                         ldap_instance_t *ldap_inst, dns_name_t *name,
-                         dns_name_t *origin, ldapdb_rdatalist_t *rdatalist)
-{
-       isc_result_t result;
-       ldapdb_rdatalist_t rdlist;
-       cache_node_t *node = NULL;
-       int in_cache = 0;
-       int is_locked = 0;
-
-       REQUIRE(cache != NULL);
-
-       INIT_LIST(*rdatalist);
-
-       if (cache->rbt == NULL)
-               return ldapdb_rdatalist_get(mctx, ldap_inst, name, origin,
-                                           rdatalist);
-
-       CONTROLED_LOCK(&cache->mutex);
-       result = dns_rbt_findname(cache->rbt, name, 0, NULL, (void *)&node);
-       if (result == ISC_R_SUCCESS) {
-               isc_time_t now;
-
-               CHECK(isc_time_now(&now));
-
-               /* Check if the record is still valid. */
-               if (isc_time_compare(&now, &node->valid_until) > 0) {
-                       CHECK(dns_rbt_deletename(cache->rbt, name, ISC_FALSE));
-                       in_cache = 0;
-               } else {
-                       rdlist = node->rdatalist;
-                       in_cache = 1;
-               }
-       } else if (result != ISC_R_NOTFOUND && result != DNS_R_PARTIALMATCH) {
-               goto cleanup;
-       }
-       CONTROLED_UNLOCK(&cache->mutex);
-
-       if (!in_cache) {
-               INIT_LIST(rdlist);
-               result = ldapdb_rdatalist_get(mctx, ldap_inst, name, origin,
-                                             &rdlist);
-               /* TODO: Cache entries that are not found. */
-               if (result != ISC_R_SUCCESS)
-                       goto cleanup;
-               CONTROLED_LOCK(&cache->mutex);
-               /* Check again to make sure. */
-               node = NULL;
-               result = dns_rbt_findname(cache->rbt, name, 0, NULL,
-                                         (void *)&node);
-               if (result == ISC_R_NOTFOUND || result == DNS_R_PARTIALMATCH) {
-                       node = NULL;
-                       CHECK(cache_node_create(cache, rdlist, &node));
-                       CHECK(dns_rbt_addname(cache->rbt, name, (void *)node));
-               }
-               CONTROLED_UNLOCK(&cache->mutex);
-       }
-
-       CHECK(ldap_rdatalist_copy(mctx, rdlist, rdatalist));
-
-       if (EMPTY(*rdatalist))
-               result = ISC_R_NOTFOUND;
-
-cleanup:
-       CONTROLED_UNLOCK(&cache->mutex);
-       return result;
-}
-
-isc_result_t
 ldap_cache_getrdatalist(isc_mem_t *mctx, ldap_cache_t *cache,
                        dns_name_t *name, ldapdb_rdatalist_t *rdatalist)
 {
diff --git a/src/cache.h b/src/cache.h
index ac5cc0d..026aba4 100644
--- a/src/cache.h
+++ b/src/cache.h
@@ -38,17 +38,6 @@ new_ldap_cache(isc_mem_t *mctx, const char * const *argv, 
ldap_cache_t **cachep)
 void
 destroy_ldap_cache(ldap_cache_t **cachep);
 
-
-/*
- * If caching is enabled, lookup 'name' in 'cache'. If the record is found and
- * is not expired, make a copy and return it. If the record is not found or is
- * expired, look it up in LDAP and cache it.
- */
-isc_result_t
-cached_ldap_rdatalist_get(isc_mem_t *mctx, ldap_cache_t *cache,
-                         ldap_instance_t *ldap_inst, dns_name_t *name,
-                         dns_name_t *origin, ldapdb_rdatalist_t *rdatalist);
-
 /*
  * Get rdatalist from cache.
  *
-- 
1.7.6

>From eac83e1d32270a60483b4830d35680d9370a6ad6 Mon Sep 17 00:00:00 2001
From: Adam Tkac <at...@redhat.com>
Date: Tue, 21 Jun 2011 14:55:27 +0200
Subject: [PATCH 4/8] Remove unneeded cache_node_create() parameter.

Signed-off-by: Adam Tkac <at...@redhat.com>
---
 src/cache.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/cache.c b/src/cache.c
index 863c9d9..855e1f3 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -63,8 +63,7 @@ cache_node_deleter(void *data, void *deleter_arg)
 
 /* TODO: Remove the rdatalist parameter */
 static isc_result_t
-cache_node_create(ldap_cache_t *cache, ldapdb_rdatalist_t rdatalist,
-                 cache_node_t **nodep)
+cache_node_create(ldap_cache_t *cache, cache_node_t **nodep)
 {
        isc_result_t result;
        cache_node_t *node;
@@ -75,7 +74,7 @@ cache_node_create(ldap_cache_t *cache, ldapdb_rdatalist_t 
rdatalist,
        CHECKED_MEM_GET_PTR(cache->mctx, node);
        ZERO_PTR(node);
        isc_mem_attach(cache->mctx, &node->mctx);
-       node->rdatalist = rdatalist;
+       ZERO_PTR(&node->rdatalist);
        CHECK(isc_time_nowplusinterval(&node->valid_until, &cache->cache_ttl));
 
        *nodep = node;
@@ -203,7 +202,6 @@ ldap_cache_addrdatalist(ldap_cache_t *cache, dns_name_t 
*name,
                        ldapdb_rdatalist_t *rdatalist)
 {
        isc_result_t result;
-       ldapdb_rdatalist_t rdlist;
        cache_node_t *node = NULL;
 
        REQUIRE(cache != NULL);
@@ -211,7 +209,7 @@ ldap_cache_addrdatalist(ldap_cache_t *cache, dns_name_t 
*name,
        if (!ldap_cache_enabled(cache))
                return ISC_R_SUCCESS; /* Caching is disabled */
 
-       CHECK(cache_node_create(cache, rdlist, &node));
+       CHECK(cache_node_create(cache, &node));
        CHECK(ldap_rdatalist_copy(cache->mctx, *rdatalist, &node->rdatalist));
 
        LOCK(&cache->mutex);
-- 
1.7.6

>From dc68d38c946cd77f630e3907a0ea0f8244de901a Mon Sep 17 00:00:00 2001
From: Adam Tkac <at...@redhat.com>
Date: Tue, 21 Jun 2011 15:23:14 +0200
Subject: [PATCH 5/8] Introduce src/types.h which contains declaration of
 common data types.

Signed-off-by: Adam Tkac <at...@redhat.com>
---
 src/ldap_helper.h |   19 ++-----------------
 src/types.h       |   39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 17 deletions(-)
 create mode 100644 src/types.h

diff --git a/src/ldap_helper.h b/src/ldap_helper.h
index 1423e3c..e82ca1c 100644
--- a/src/ldap_helper.h
+++ b/src/ldap_helper.h
@@ -2,7 +2,7 @@
  * Authors: Martin Nagy <mn...@redhat.com>
  *          Adam Tkac <at...@redhat.com>
  *
- * Copyright (C) 2008, 2009  Red Hat
+ * Copyright (C) 2008 - 2011 Red Hat
  * see file 'COPYING' for use and warranty information
  *
  * This program is free software; you can redistribute it and/or
@@ -22,7 +22,7 @@
 #ifndef _LD_LDAP_HELPER_H_
 #define _LD_LDAP_HELPER_H_
 
-#include "ldap_helper.h"
+#include "types.h"
 
 #include <isc/util.h>
 
@@ -35,21 +35,6 @@ struct ldap_value {
        LINK(ldap_value_t)      link;
 };
 
-/*
- * some nice words about ldapdb_rdatalist_t:
- * - it is list of all RRs which have same owner name
- * - rdata buffer is reachable only via dns_rdata_toregion()
- *
- * structure:
- *
- * class1                               class2
- * type1                                type2
- * ttl1                                 ttl2
- * rdata1 -> rdata2 -> rdata3           rdata4 -> rdata5
- * next_rdatalist              ->       next_rdatalist  ...
- */
-typedef LIST(dns_rdatalist_t) ldapdb_rdatalist_t;
-
 isc_result_t ldapdb_rdatalist_findrdatatype(ldapdb_rdatalist_t *rdatalist,
                                            dns_rdatatype_t rdtype,
                                            dns_rdatalist_t **rdlistp);
diff --git a/src/types.h b/src/types.h
new file mode 100644
index 0000000..b39f497
--- /dev/null
+++ b/src/types.h
@@ -0,0 +1,39 @@
+/*
+ * Authors: Adam Tkac <at...@redhat.com>
+ *
+ * Copyright (C) 2011 Red Hat
+ * see file 'COPYING' for use and warranty information
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; version 2 or later
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#ifndef _LD_TYPES_H_
+#define _LD_TYPES_H_
+
+/*
+ * some nice words about ldapdb_rdatalist_t:
+ * - it is list of all RRs which have same owner name
+ * - rdata buffer is reachable only via dns_rdata_toregion()
+ *
+ * structure:
+ *
+ * class1                               class2
+ * type1                                type2
+ * ttl1                                 ttl2
+ * rdata1 -> rdata2 -> rdata3           rdata4 -> rdata5
+ * next_rdatalist              ->       next_rdatalist  ...
+ */
+typedef LIST(dns_rdatalist_t) ldapdb_rdatalist_t;
+
+#endif /* !_LD_TYPES_H_ */
-- 
1.7.6

>From e7b8523495be31475d42894ce16f150171167a8e Mon Sep 17 00:00:00 2001
From: Adam Tkac <at...@redhat.com>
Date: Tue, 21 Jun 2011 15:37:13 +0200
Subject: [PATCH 6/8] Move per-LDAP-instance RRs cache into ldap_instance_t.

Signed-off-by: Adam Tkac <at...@redhat.com>
---
 src/cache.h        |    2 +-
 src/ldap_driver.c  |   30 +++++++++++++++++-------------
 src/ldap_helper.c  |   14 ++++++++++++++
 src/ldap_helper.h  |    4 ++++
 src/zone_manager.c |    9 +--------
 src/zone_manager.h |    7 ++++---
 6 files changed, 41 insertions(+), 25 deletions(-)

diff --git a/src/cache.h b/src/cache.h
index 026aba4..f5956cf 100644
--- a/src/cache.h
+++ b/src/cache.h
@@ -22,7 +22,7 @@
 #ifndef _LD_CACHE_H_
 #define _LD_CACHE_H_
 
-#include "ldap_helper.h"
+#include "types.h"
 
 typedef struct ldap_cache ldap_cache_t;
 
diff --git a/src/ldap_driver.c b/src/ldap_driver.c
index 819f66d..dcbb5a4 100644
--- a/src/ldap_driver.c
+++ b/src/ldap_driver.c
@@ -65,7 +65,6 @@ typedef struct {
        dns_db_t                        common;
        isc_refcount_t                  refs;
        ldap_instance_t                 *ldap_inst;
-       ldap_cache_t                    *ldap_cache;
 } ldapdb_t;
 
 typedef struct {
@@ -370,11 +369,13 @@ findnode(dns_db_t *db, dns_name_t *name, isc_boolean_t 
create,
        isc_result_t result;
        ldapdb_rdatalist_t rdatalist;
        ldapdbnode_t *node = NULL;
+       ldap_cache_t *cache;
 
        REQUIRE(VALID_LDAPDB(ldapdb));
 
-       result = ldap_cache_getrdatalist(ldapdb->common.mctx, 
ldapdb->ldap_cache,
-                                        name, &rdatalist);
+       cache = ldap_instance_getcache(ldapdb->ldap_inst);
+       result = ldap_cache_getrdatalist(ldapdb->common.mctx, cache, name,
+                                        &rdatalist);
        switch (result) {
        case ISC_R_SUCCESS:
                goto cachehit;
@@ -393,8 +394,7 @@ cachemiss:
                                      &rdatalist);
        /* Cache rdatalist if found. */
        if (result == ISC_R_SUCCESS)
-               CHECK(ldap_cache_addrdatalist(ldapdb->ldap_cache, name,
-                                             &rdatalist));
+               CHECK(ldap_cache_addrdatalist(cache, name, &rdatalist));
 cachehit:
        if (create == ISC_FALSE) {
                if (result != ISC_R_SUCCESS)
@@ -430,6 +430,7 @@ find(dns_db_t *db, dns_name_t *name, dns_dbversion_t 
*version,
        dns_rdatalist_t *rdlist = NULL;
        isc_boolean_t is_cname = ISC_FALSE;
        ldapdb_rdatalist_t rdatalist;
+       ldap_cache_t *cache;
 
        UNUSED(now);
        UNUSED(options);
@@ -443,8 +444,9 @@ find(dns_db_t *db, dns_name_t *name, dns_dbversion_t 
*version,
                REQUIRE(version == ldapdb_version);
        }
 
-       result = ldap_cache_getrdatalist(ldapdb->common.mctx, 
ldapdb->ldap_cache,
-                                        name, &rdatalist);
+       cache = ldap_instance_getcache(ldapdb->ldap_inst);
+       result = ldap_cache_getrdatalist(ldapdb->common.mctx, cache, name,
+                                        &rdatalist);
        switch (result) {
        case ISC_R_SUCCESS:
                goto cachehit;
@@ -464,8 +466,7 @@ cachemiss:
                                      &rdatalist);
        /* Cache rdatalist if found. */
        if (result == ISC_R_SUCCESS)
-               CHECK(ldap_cache_addrdatalist(ldapdb->ldap_cache, name,
-                                             &rdatalist));
+               CHECK(ldap_cache_addrdatalist(cache, name, &rdatalist));
 cachehit:
        if (result != ISC_R_SUCCESS && result != DNS_R_PARTIALMATCH)
                return (result == ISC_R_NOTFOUND) ? DNS_R_NXDOMAIN : result;
@@ -717,6 +718,7 @@ addrdataset(dns_db_t *db, dns_dbnode_t *node, 
dns_dbversion_t *version,
        dns_rdatalist_t diff;
        isc_result_t result;
        isc_boolean_t rdatalist_exists = ISC_FALSE;
+       ldap_cache_t *cache;
 
        UNUSED(now);
        UNUSED(db);
@@ -770,7 +772,8 @@ addrdataset(dns_db_t *db, dns_dbnode_t *node, 
dns_dbversion_t *version,
        }
 
        CHECK(write_to_ldap(&ldapdbnode->owner, ldapdb->ldap_inst, new_rdlist));
-       CHECK(discard_from_cache(ldapdb->ldap_cache, &ldapdbnode->owner));
+       cache = ldap_instance_getcache(ldapdb->ldap_inst);
+       CHECK(discard_from_cache(cache, &ldapdbnode->owner));
 
        if (addedrdataset != NULL) {
                result = dns_rdatalist_tordataset(new_rdlist, addedrdataset);
@@ -823,6 +826,7 @@ subtractrdataset(dns_db_t *db, dns_dbnode_t *node, 
dns_dbversion_t *version,
        dns_rdatalist_t diff;
        isc_result_t result;
        isc_boolean_t delete_node = ISC_FALSE;
+       ldap_cache_t *cache;
 
        REQUIRE(version == ldapdb_version);
 
@@ -870,7 +874,8 @@ subtractrdataset(dns_db_t *db, dns_dbnode_t *node, 
dns_dbversion_t *version,
 
        CHECK(remove_from_ldap(&ldapdbnode->owner, ldapdb->ldap_inst, &diff,
                               delete_node));
-       CHECK(discard_from_cache(ldapdb->ldap_cache, &ldapdbnode->owner));
+       cache = ldap_instance_getcache(ldapdb->ldap_inst);
+       CHECK(discard_from_cache(cache, &ldapdbnode->owner));
 
        if (newrdataset != NULL) {
                result = dns_rdatalist_tordataset(found_rdlist, newrdataset);
@@ -1116,8 +1121,7 @@ ldapdb_create(isc_mem_t *mctx, dns_name_t *name, 
dns_dbtype_t type,
        CHECK(dns_name_dupwithoffsets(name, mctx, &ldapdb->common.origin));
 
        CHECK(isc_refcount_init(&ldapdb->refs, 1));
-       CHECK(manager_get_ldap_instance_and_cache(argv[0], &ldapdb->ldap_inst,
-                                           &ldapdb->ldap_cache));
+       CHECK(manager_get_ldap_instance_and_cache(argv[0], &ldapdb->ldap_inst));
 
        *dbp = (dns_db_t *)ldapdb;
 
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index b651b70..af77dc0 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -129,6 +129,9 @@ struct ldap_instance {
        semaphore_t             conn_semaphore;
        ldap_connection_t       **conns;
 
+       /* RRs cache */
+       ldap_cache_t            *cache;
+
        /* Our own list of zones. */
        zone_register_t         *zone_register;
 
@@ -426,6 +429,7 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
                        ldap_inst->connections * sizeof(ldap_connection_t *));
        memset(ldap_inst->conns, 0, ldap_inst->connections * 
sizeof(ldap_connection_t *));
 
+       CHECK(new_ldap_cache(mctx, argv, &ldap_inst->cache));
 retry:
        for (i = 0; i < ldap_inst->connections; i++) {
                ldap_conn = NULL;
@@ -503,6 +507,9 @@ destroy_ldap_instance(ldap_instance_t **ldap_instp)
 
        DESTROYLOCK(&ldap_inst->kinit_lock);
 
+       if (ldap_inst->cache != NULL)
+               destroy_ldap_cache(&ldap_inst->cache);
+
        zr_destroy(&ldap_inst->zone_register);
 
        isc_mem_putanddetach(&ldap_inst->mctx, ldap_inst, 
sizeof(ldap_instance_t));
@@ -2127,3 +2134,10 @@ remove_from_ldap(dns_name_t *owner, ldap_instance_t 
*ldap_inst,
        return modify_ldap_common(owner, ldap_inst, rdlist, LDAP_MOD_DELETE,
                                  delete_node);
 }
+
+ldap_cache_t *
+ldap_instance_getcache(ldap_instance_t *ldap_inst)
+{
+       return ldap_inst->cache;
+}
+
diff --git a/src/ldap_helper.h b/src/ldap_helper.h
index e82ca1c..14ad4ce 100644
--- a/src/ldap_helper.h
+++ b/src/ldap_helper.h
@@ -22,6 +22,7 @@
 #ifndef _LD_LDAP_HELPER_H_
 #define _LD_LDAP_HELPER_H_
 
+#include "cache.h"
 #include "types.h"
 
 #include <isc/util.h>
@@ -93,4 +94,7 @@ isc_result_t write_to_ldap(dns_name_t *owner, ldap_instance_t 
*ldap_inst,
 isc_result_t remove_from_ldap(dns_name_t *owner, ldap_instance_t *ldap_inst,
                dns_rdatalist_t *rdlist, isc_boolean_t delete_node);
 
+/* Get cache associated with ldap_inst */
+ldap_cache_t *ldap_instance_getcache(ldap_instance_t *ldap_inst);
+
 #endif /* !_LD_LDAP_HELPER_H_ */
diff --git a/src/zone_manager.c b/src/zone_manager.c
index d6b5437..59aac95 100644
--- a/src/zone_manager.c
+++ b/src/zone_manager.c
@@ -42,7 +42,6 @@ struct db_instance {
        isc_mem_t               *mctx;
        char                    *name;
        ldap_instance_t         *ldap_inst;
-       ldap_cache_t            *ldap_cache;
        isc_timer_t             *timer;
        LINK(db_instance_t)     link;
 };
@@ -95,8 +94,6 @@ destroy_db_instance(db_instance_t **db_instp)
                isc_timer_detach(&db_inst->timer);
        if (db_inst->ldap_inst != NULL)
                destroy_ldap_instance(&db_inst->ldap_inst);
-       if (db_inst->ldap_cache != NULL)
-               destroy_ldap_cache(&db_inst->ldap_cache);
        if (db_inst->name != NULL)
                isc_mem_free(db_inst->mctx, db_inst->name);
 
@@ -147,7 +144,6 @@ manager_create_db_instance(isc_mem_t *mctx, const char 
*name,
        isc_mem_attach(mctx, &db_inst->mctx);
        CHECKED_MEM_STRDUP(mctx, name, db_inst->name);
        CHECK(new_ldap_instance(mctx, db_inst->name, argv, dyndb_args, 
&db_inst->ldap_inst));
-       CHECK(new_ldap_cache(mctx, argv, &db_inst->ldap_cache));
 
        task = dns_dyndb_get_task(dyndb_args);
        result = refresh_zones_from_ldap(task, db_inst->ldap_inst, ISC_TRUE);
@@ -196,15 +192,13 @@ refresh_zones_action(isc_task_t *task, isc_event_t *event)
 }
 
 isc_result_t
-manager_get_ldap_instance_and_cache(const char *name, ldap_instance_t 
**ldap_inst,
-                                   ldap_cache_t **ldap_cache)
+manager_get_ldap_instance_and_cache(const char *name, ldap_instance_t 
**ldap_inst)
 {
        isc_result_t result;
        db_instance_t *db_inst;
 
        REQUIRE(name != NULL);
        REQUIRE(ldap_inst != NULL);
-       REQUIRE(ldap_cache != NULL);
 
        isc_once_do(&initialize_once, initialize_manager);
 
@@ -212,7 +206,6 @@ manager_get_ldap_instance_and_cache(const char *name, 
ldap_instance_t **ldap_ins
        CHECK(find_db_instance(name, &db_inst));
 
        *ldap_inst = db_inst->ldap_inst;
-       *ldap_cache = db_inst->ldap_cache;
 
 cleanup:
        return result;
diff --git a/src/zone_manager.h b/src/zone_manager.h
index 85eab15..056b9da 100644
--- a/src/zone_manager.h
+++ b/src/zone_manager.h
@@ -1,7 +1,8 @@
 /*
  * Authors: Martin Nagy <mn...@redhat.com>
+ *         Adam Tkac <at...@redhat.com>
  *
- * Copyright (C) 2009  Red Hat
+ * Copyright (C) 2009 - 2011 Red Hat
  * see file 'COPYING' for use and warranty information
  *
  * This program is free software; you can redistribute it and/or
@@ -36,7 +37,7 @@ manager_create_db_instance(isc_mem_t *mctx, const char *name,
                           dns_dyndb_arguments_t *dyndb_args);
 
 isc_result_t
-manager_get_ldap_instance_and_cache(const char *name, ldap_instance_t 
**ldap_inst,
-                                   ldap_cache_t **ldap_cache);
+manager_get_ldap_instance_and_cache(const char *name,
+                                   ldap_instance_t **ldap_inst);
 
 #endif /* !_LD_ZONE_MANAGER_H_ */
-- 
1.7.6

>From cde7cc471df099fd2c6914f4a0583493659bf40e Mon Sep 17 00:00:00 2001
From: Adam Tkac <at...@redhat.com>
Date: Tue, 21 Jun 2011 15:42:39 +0200
Subject: [PATCH 7/8] Remove unneeded INIT_LIST calls.

Signed-off-by: Adam Tkac <at...@redhat.com>
---
 src/ldap_driver.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/src/ldap_driver.c b/src/ldap_driver.c
index dcbb5a4..6815776 100644
--- a/src/ldap_driver.c
+++ b/src/ldap_driver.c
@@ -388,7 +388,6 @@ findnode(dns_db_t *db, dns_name_t *name, isc_boolean_t 
create,
        }
 
 cachemiss:
-       INIT_LIST(rdatalist); /* XXX Should this be moved to 
ldapdb_rdatalist_get ? */
        result = ldapdb_rdatalist_get(ldapdb->common.mctx, ldapdb->ldap_inst,
                                      name, &ldapdb->common.origin,
                                      &rdatalist);
@@ -460,7 +459,6 @@ find(dns_db_t *db, dns_name_t *name, dns_dbversion_t 
*version,
 
 cachemiss:
        /* XXX Move ldap_cache_addrdatalist into ldapdb_rdatalist_get. */
-       INIT_LIST(rdatalist); /* XXX Should this be moved to 
ldapdb_rdatalist_get ? */
        result = ldapdb_rdatalist_get(ldapdb->common.mctx, ldapdb->ldap_inst,
                                      name, &ldapdb->common.origin,
                                      &rdatalist);
-- 
1.7.6

>From 9d0c98375cfdb1f011fa2aea4d6906deba9b2b9d Mon Sep 17 00:00:00 2001
From: Adam Tkac <at...@redhat.com>
Date: Wed, 29 Jun 2011 18:04:56 +0200
Subject: [PATCH 8/8] Improve ldap_cache_addrdatalist to replace cache entry
 if already exists.

Signed-off-by: Adam Tkac <at...@redhat.com>
---
 src/cache.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/src/cache.c b/src/cache.c
index 855e1f3..a6233df 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -202,6 +202,7 @@ ldap_cache_addrdatalist(ldap_cache_t *cache, dns_name_t 
*name,
                        ldapdb_rdatalist_t *rdatalist)
 {
        isc_result_t result;
+       isc_boolean_t free_rdlist = ISC_FALSE;
        cache_node_t *node = NULL;
 
        REQUIRE(cache != NULL);
@@ -211,14 +212,27 @@ ldap_cache_addrdatalist(ldap_cache_t *cache, dns_name_t 
*name,
 
        CHECK(cache_node_create(cache, &node));
        CHECK(ldap_rdatalist_copy(cache->mctx, *rdatalist, &node->rdatalist));
+       free_rdlist = ISC_TRUE;
 
        LOCK(&cache->mutex);
+retry:
        result = dns_rbt_addname(cache->rbt, name, (void *)node);
-       INSIST(result != ISC_R_EXISTS); /* Indicates a bug */
-       
+       if (result == ISC_R_EXISTS) {
+               /* Replace it */
+               CHECK(dns_rbt_deletename(cache->rbt, name, ISC_FALSE));
+               goto retry;
+       } else if (result != ISC_R_SUCCESS) {
+               UNLOCK(&cache->mutex);
+               goto cleanup;
+       }
        UNLOCK(&cache->mutex);
+
+       return ISC_R_SUCCESS;
+
 cleanup:
-       if (result != ISC_R_SUCCESS && node != NULL)
+       if (free_rdlist)
+               ldapdb_rdatalist_destroy(cache->mctx, &node->rdatalist);
+       if (node != NULL)
                MEM_PUT_AND_DETACH(node);
                
        return result;
-- 
1.7.6

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to