Re: [Freeipa-devel] [PATCH] 0006 Hold bind and plugin global settings in LDAP
On 02/28/2012 02:25 PM, Adam Tkac wrote:
On 02/22/2012 12:42 PM, Petr Spacek wrote:
Hello,
this patch fixes https://fedorahosted.org/bind-dyndb-ldap/ticket/43 -
hold bind and plugin global settings in LDAP.
Currently it's not optimized for performance. Patch for avoiding
unnecessary locking will follow tomorrow or on Friday.
After discussion with Martin we decided to defer support for changing
persistent search option & support for LDAP configuration override
(https://fedorahosted.org/bind-dyndb-ldap/ticket/48).
Both deferred cases require bigger changes and it's no so simple to
test it, so it is risky before a release.
Hello Petr,
thanks for your patch. Please see my comment below, after you fix the
patch please push it to master.
Regards, Adam
Fixed and pushed to master.
https://fedorahosted.org/bind-dyndb-ldap/changeset/8856c072c9373ede7662c90e0e0bc85d68188854/
--
Petr^2 Spacek
bind-dyndb-ldap-pspacek-0006-Support-for-plugin-configuration-from-LDAP-without-p.patch
From 01296f949ec10f77ac1285993dc4b6c9b2358e02 Mon Sep 17 00:00:00 2001
From: Petr Spacek
Date: Wed, 22 Feb 2012 11:01:33 +0100
Subject: [PATCH] Support for plugin configuration from LDAP, without
performance optimization. Patched BIND with postponed
plugin start is required for forwarders option to work.
Signed-off-by: Petr Spacek
---
README | 18 -
src/ldap_helper.c | 61 +---
src/zone_manager.c | 71
++--
src/zone_manager.h | 4 +++
4 files changed, 129 insertions(+), 25 deletions(-)
diff --git a/README b/README
index 9cd1493..2770c46 100644
--- a/README
+++ b/README
@@ -15,7 +15,7 @@ Hopefully, the patch will once be included in the
official BIND release.
===
* support for dynamic updates
* SASL authentication
-* persistent search for zones
+* persistent search for zones (experimental)
3. Installation
@@ -245,6 +245,22 @@ idnsZoneActive attribute is set to True. For each
entry it will find, it
will register a new zone with BIND. The LDAP back-end will keep each
record it gets from LDAP in its cache for 5 minutes.
+5.3 Configuration in LDAP
+-
+Some options can be configured in LDAP as idnsConfigObject attributes.
+Value configured in LDAP has priority over value in configuration file.
+(This behavior will change in future versions!)
+Configuration is updated at same time as zone refresh.
+
+Following options are supported (option = attribute equivalent):
+
+forwarders = idnsForwarders (BIND native option)
+forward = idnsForwardPolicy (BIND native option)
+sync_ptr = idnsAllowSyncPTR
+zone_refresh = idnsZoneRefresh
+
+Forward policy option cannot be set without setting forwarders at the
same time.
+
6. License
==
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index b26c9c9..0027eee 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -45,6 +45,8 @@
#include
#include
#include
+#include
+#include
#include
#define LDAP_DEPRECATED 1
@@ -93,8 +95,10 @@
* Note about locking in this source.
*
* ldap_instance_t structure is equal to dynamic-db {}; statement in
named.conf.
- * Attributes in ldap_instance_t can be modified only in
new_ldap_instance
- * function, which means server is started or reloaded.
+ * Attributes in ldap_instance_t are be modified in new_ldap_instance
function,
+ * which means server is started or reloaded (running single-thread).
+ * Before modifying at other places, switch to single-thread mode via
+ * isc_task_beginexclusive() and then return back via
isc_task_endexclusive()!
*
* ldap_connection_t structure represents connection to the LDAP
database and
* per-connection specific data. Access is controlled via
@@ -856,6 +860,7 @@ parse_nameserver(char *token, struct sockaddr *sa)
return result;
}
+/* TODO: Code hardering. */
static void *
get_in_addr(struct sockaddr *sa)
{
@@ -960,7 +965,22 @@ ldap_parse_configentry(ldap_entry_t *entry,
ldap_instance_t *inst)
{
isc_result_t result;
ldap_valuelist_t values;
+ isc_boolean_t unlock_required;
You should set unlock_required to ISC_FALSE here, otherwise it can be
undefined (and used in cleanup:) when isc_task_beginexclusive() below
returns ISC_R_LOCKBUSY.
+ isc_timer_t *timer_inst;
+ isc_interval_t timer_interval;
+ isc_uint32_t interval_sec;
+ isc_timertype_t timer_type;
+
+ /* Before changing parameters transit to single-thread operation. */
+ result = isc_task_beginexclusive(inst->task);
+ RUNTIME_CHECK(result == ISC_R_SUCCESS ||
+ result == ISC_R_LOCKBUSY);
+ if (result == ISC_R_SUCCESS)
+ unlock_required = ISC_TRUE;
+
+ log_debug(3, "Parsing configuration object");
+ /* idnsForwardPolicy change is handled by
configure_zone_forwarders() */
result = ldap_entry_getvalues(entry, "idnsForwarders",&values);
if (result == ISC_R_SUCCESS) {
log_debug(2, "Setting global forwarders");
@@ -969,7 +989,38 @@ ldap_parse_configentry(ldap_entry_t *entry,
ldap_instance_t *inst)
log_error("Global forwarder could not be
Re: [Freeipa-devel] [PATCH] 0006 Hold bind and plugin global settings in LDAP
On 02/22/2012 12:42 PM, Petr Spacek wrote:
Hello,
this patch fixes https://fedorahosted.org/bind-dyndb-ldap/ticket/43 -
hold bind and plugin global settings in LDAP.
Currently it's not optimized for performance. Patch for avoiding
unnecessary locking will follow tomorrow or on Friday.
After discussion with Martin we decided to defer support for changing
persistent search option & support for LDAP configuration override
(https://fedorahosted.org/bind-dyndb-ldap/ticket/48).
Both deferred cases require bigger changes and it's no so simple to
test it, so it is risky before a release.
Hello Petr,
thanks for your patch. Please see my comment below, after you fix the
patch please push it to master.
Regards, Adam
bind-dyndb-ldap-pspacek-0006-Support-for-plugin-configuration-from-LDAP-without-p.patch
From 01296f949ec10f77ac1285993dc4b6c9b2358e02 Mon Sep 17 00:00:00 2001
From: Petr Spacek
Date: Wed, 22 Feb 2012 11:01:33 +0100
Subject: [PATCH] Support for plugin configuration from LDAP, without
performance optimization. Patched BIND with postponed
plugin start is required for forwarders option to work.
Signed-off-by: Petr Spacek
---
README | 18 -
src/ldap_helper.c | 61 +---
src/zone_manager.c | 71 ++--
src/zone_manager.h |4 +++
4 files changed, 129 insertions(+), 25 deletions(-)
diff --git a/README b/README
index 9cd1493..2770c46 100644
--- a/README
+++ b/README
@@ -15,7 +15,7 @@ Hopefully, the patch will once be included in the official
BIND release.
===
* support for dynamic updates
* SASL authentication
-* persistent search for zones
+* persistent search for zones (experimental)
3. Installation
@@ -245,6 +245,22 @@ idnsZoneActive attribute is set to True. For each entry it
will find, it
will register a new zone with BIND. The LDAP back-end will keep each
record it gets from LDAP in its cache for 5 minutes.
+5.3 Configuration in LDAP
+-
+Some options can be configured in LDAP as idnsConfigObject attributes.
+Value configured in LDAP has priority over value in configuration file.
+(This behavior will change in future versions!)
+Configuration is updated at same time as zone refresh.
+
+Following options are supported (option = attribute equivalent):
+
+forwarders = idnsForwarders (BIND native option)
+forward = idnsForwardPolicy (BIND native option)
+sync_ptr = idnsAllowSyncPTR
+zone_refresh = idnsZoneRefresh
+
+Forward policy option cannot be set without setting forwarders at the same
time.
+
6. License
==
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index b26c9c9..0027eee 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -45,6 +45,8 @@
#include
#include
#include
+#include
+#include
#include
#define LDAP_DEPRECATED 1
@@ -93,8 +95,10 @@
* Note about locking in this source.
*
* ldap_instance_t structure is equal to dynamic-db {}; statement in
named.conf.
- * Attributes in ldap_instance_t can be modified only in new_ldap_instance
- * function, which means server is started or reloaded.
+ * Attributes in ldap_instance_t are be modified in new_ldap_instance function,
+ * which means server is started or reloaded (running single-thread).
+ * Before modifying at other places, switch to single-thread mode via
+ * isc_task_beginexclusive() and then return back via isc_task_endexclusive()!
*
* ldap_connection_t structure represents connection to the LDAP database and
* per-connection specific data. Access is controlled via
@@ -856,6 +860,7 @@ parse_nameserver(char *token, struct sockaddr *sa)
return result;
}
+/* TODO: Code hardering. */
static void *
get_in_addr(struct sockaddr *sa)
{
@@ -960,7 +965,22 @@ ldap_parse_configentry(ldap_entry_t *entry,
ldap_instance_t *inst)
{
isc_result_t result;
ldap_valuelist_t values;
+ isc_boolean_t unlock_required;
You should set unlock_required to ISC_FALSE here, otherwise it can be
undefined (and used in cleanup:) when isc_task_beginexclusive() below
returns ISC_R_LOCKBUSY.
+ isc_timer_t *timer_inst;
+ isc_interval_t timer_interval;
+ isc_uint32_t interval_sec;
+ isc_timertype_t timer_type;
+
+ /* Before changing parameters transit to single-thread operation. */
+ result = isc_task_beginexclusive(inst->task);
+ RUNTIME_CHECK(result == ISC_R_SUCCESS ||
+ result == ISC_R_LOCKBUSY);
+ if (result == ISC_R_SUCCESS)
+ unlock_required = ISC_TRUE;
+
+ log_debug(3, "Parsing configuration object");
+ /* idnsForwardPolicy change is handled by configure_zone_forwarders() */
result = ldap_entry_getvalues(entry, "idnsForwarders",&values);
if (result == ISC_R_SUCCESS) {
log_debug(2, "Setting global forwarders");
@@ -969,7 +989,38 @@ ldap_parse_
