Re: [Freeipa-devel] [PATCH 0123-0126] Separate master and forward zones (add idnsForwardZone object class)

2013-04-02 Thread Petr Spacek 

On 22.3.2013 13:03, Petr Spacek wrote:

Hello,

this patch set separates master zones (idnsZone objectClass) from forward
zones (idnsForwardZone objectClass). Support for forward zones in idnsZone
objectClass is still present to ease upgrades.

See each commit message for all the gory details.


I encountered a crash caused by bug in patch 126. Fixed version is attached.

Diff between patch 126 version 1 and 2:

--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -3391,7 +3391,7 @@ update_zone(isc_task_t *task, isc_event_t *event)

CHECK(dn_to_dnsname(inst-mctx, pevent-dn, currname, NULL));

-   if (result == ISC_R_SUCCESS 
+   if (ldap_qresult_zone != NULL 
HEAD(ldap_qresult_zone-ldap_entries) != NULL) {
entry_zone = HEAD(ldap_qresult_zone-ldap_entries);
CHECK(ldap_entry_getclass(entry_zone, objclass));

--
Petr^2 Spacek
From 85ed07b47fb9480faea13218b8a6c28659506449 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 22 Mar 2013 12:38:55 +0100
Subject: [PATCH] Add support for pure forward zones - idnsForwardZone
 objectClass.

Master zones are stored in zone_register and pure forward zones
are stored in fwd_register.

This patch doesn't remove support for forward zones within
idnsZone objectClass. Support for forward zones in both
objectClasses enables incremental update, where old and new
plugin versions operate on the same LDAP database.

Support for forward zones defined by idnsZone objectClass
will be removed in near future.

Forward zones defined in idnsZone objectClass are not disabled
after removing from LDAP if persistent search is disabled
(see ticket #106).
This problem doesn't affect zones defined with idnsForwardZone
objectClass.

https://fedorahosted.org/bind-dyndb-ldap/ticket/99

Signed-off-by: Petr Spacek pspa...@redhat.com
---
 src/Makefile.am|   4 +
 src/fwd_register.c | 156 +
 src/fwd_register.h |  35 ++
 src/ldap_entry.c   |  33 --
 src/ldap_entry.h   |   7 +-
 src/ldap_helper.c  | 336 ++---
 6 files changed, 441 insertions(+), 130 deletions(-)
 create mode 100644 src/fwd_register.c
 create mode 100644 src/fwd_register.h

diff --git a/src/Makefile.am b/src/Makefile.am
index 252255788b01e003031f5f0ee2fc8469b53633be..87c3252736fa4f918f105166497b32b0219ef8ea 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -5,11 +5,13 @@ HDRS =\
 	acl.h			\
 	cache.h			\
 	compat.h		\
+	fwd_register.h		\
 	krb5_helper.h		\
 	ldap_convert.h		\
 	ldap_entry.h		\
 	ldap_helper.h		\
 	log.h			\
+	rbt_helper.h		\
 	rdlist.h		\
 	semaphore.h		\
 	settings.h		\
@@ -23,12 +25,14 @@ ldap_la_SOURCES =		\
 	$(HDRS)			\
 	acl.c			\
 	cache.c			\
+	fwd_register.c		\
 	krb5_helper.c		\
 	ldap_convert.c		\
 	ldap_driver.c		\
 	ldap_entry.c		\
 	ldap_helper.c		\
 	log.c			\
+	rbt_helper.c		\
 	rdlist.c		\
 	semaphore.c		\
 	settings.c		\
diff --git a/src/fwd_register.c b/src/fwd_register.c
new file mode 100644
index ..c663b25909b0e393421c49950d1f29a1352cfe6c
--- /dev/null
+++ b/src/fwd_register.c
@@ -0,0 +1,156 @@
+#include isc/rwlock.h
+#include dns/name.h
+
+#include rbt_helper.h
+#include fwd_register.h
+#include util.h
+
+struct fwd_register {
+	isc_mem_t	*mctx;
+	isc_rwlock_t	rwlock;
+	dns_rbt_t	*rbt;
+};
+
+isc_result_t
+fwdr_create(isc_mem_t *mctx, fwd_register_t **fwdrp)
+{
+	isc_result_t result;
+	fwd_register_t *fwdr = NULL;
+
+	REQUIRE(fwdrp != NULL  *fwdrp == NULL);
+
+	CHECKED_MEM_GET_PTR(mctx, fwdr);
+	ZERO_PTR(fwdr);
+	isc_mem_attach(mctx, fwdr-mctx);
+	CHECK(dns_rbt_create(mctx, NULL, NULL, fwdr-rbt));
+	CHECK(isc_rwlock_init(fwdr-rwlock, 0, 0));
+
+	*fwdrp = fwdr;
+	return ISC_R_SUCCESS;
+
+cleanup:
+	if (fwdr != NULL) {
+		if (fwdr-rbt != NULL)
+			dns_rbt_destroy(fwdr-rbt);
+		MEM_PUT_AND_DETACH(fwdr);
+	}
+
+	return result;
+}
+
+void
+fwdr_destroy(fwd_register_t **fwdrp)
+{
+	fwd_register_t *fwdr;
+
+	if (fwdrp == NULL || *fwdrp == NULL)
+		return;
+
+	fwdr = *fwdrp;
+
+	RWLOCK(fwdr-rwlock, isc_rwlocktype_write);
+	dns_rbt_destroy(fwdr-rbt);
+	RWUNLOCK(fwdr-rwlock, isc_rwlocktype_write);
+	isc_rwlock_destroy(fwdr-rwlock);
+	MEM_PUT_AND_DETACH(fwdr);
+
+	*fwdrp = NULL;
+}
+
+/*
+ * Add forward zone to the forwarding register 'fwdr'. Origin of the zone
+ * must be absolute and the zone cannot already be in the register.
+ */
+isc_result_t
+fwdr_add_zone(fwd_register_t *fwdr, dns_name_t *name)
+{
+	isc_result_t result;
+	void *dummy = NULL;
+
+	REQUIRE(fwdr != NULL);
+	REQUIRE(name != NULL);
+
+	if (!dns_name_isabsolute(name)) {
+		log_bug(forward zone with bad origin);
+		return ISC_R_FAILURE;
+	}
+
+	RWLOCK(fwdr-rwlock, isc_rwlocktype_write);
+
+	/*
+	 * First make sure the node doesn't exist. Partial matches mean
+	 * there are also child zones in the LDAP database which is allowed.
+	 */
+	result = dns_rbt_findname(fwdr-rbt, name, 0, NULL, dummy);
+	if (result != ISC_R_NOTFOUND  result !=

Re: [Freeipa-devel] [PATCH 0123-0126] Separate master and forward zones (add idnsForwardZone object class)

2013-04-02 Thread Adam Tkac 
On Fri, Mar 22, 2013 at 01:03:12PM +0100, Petr Spacek wrote:
 Hello,
 
 this patch set separates master zones (idnsZone objectClass) from
 forward zones (idnsForwardZone objectClass). Support for forward
 zones in idnsZone objectClass is still present to ease upgrades.
 
 See each commit message for all the gory details.

Since patches are non-trivial, I will review them per partes (i.e. each patch
in separate mail). Please check my comments below.

Regards, Adam

 From d0c598ea7e9c02a1ec786c6f1c596ae1be7ac1e2 Mon Sep 17 00:00:00 2001
 From: Petr Spacek pspa...@redhat.com
 Date: Fri, 22 Mar 2013 12:17:07 +0100
 Subject: [PATCH] Add helper functions for generic iteration over RBT.
 
 https://fedorahosted.org/bind-dyndb-ldap/ticket/99
 
 Signed-off-by: Petr Spacek pspa...@redhat.com
 ---
  src/rbt_helper.c | 150 
 +++
  src/rbt_helper.h |  29 +++
  2 files changed, 179 insertions(+)
  create mode 100644 src/rbt_helper.c
  create mode 100644 src/rbt_helper.h
 
 diff --git a/src/rbt_helper.c b/src/rbt_helper.c
 new file mode 100644
 index 
 ..70ab06134694e36a6ae049284d506bbf5bc3a977
 --- /dev/null
 +++ b/src/rbt_helper.c
 @@ -0,0 +1,150 @@
 +#include dns/rbt.h
 +
 +#include rbt_helper.h
 +
 +#define LDAPDB_RBTITER_MAGIC ISC_MAGIC('L', 'D', 'P', 'I')
 +
 +/**
 + * Copy the RBT node name, i.e. copies the name pointed to by RBT iterator.
 + *
 + * @param[in]  iter Initialized RBT iterator.
 + * @param[out] nodename Target dns_name suitable for rbt_fullnamefromnode() 
 call.
 + *
 + * @pre Nodename has pre-allocated storage space.
 + *
 + * @retval ISC_R_SUCCESS   Actual name was copied to nodename.
 + * @retval ISC_R_NOTFOUND  Iterator doesn't point to any node.
 + * @retval DNS_R_EMPTYNAME Iterator points to name without assigned data,
 + * nodename is unchanged.
 + * @retval others  Errors from dns_name_concatenate() and others.
 + *
 + */
 +static isc_result_t
 +rbt_iter_getnodename(rbt_iterator_t *iter, dns_name_t *nodename) {
 + isc_result_t result;
 + dns_rbtnode_t *node = NULL;
 +
 + REQUIRE(iter != NULL);
 + REQUIRE(nodename != NULL);
 + REQUIRE(ISC_MAGIC_VALID(iter, LDAPDB_RBTITER_MAGIC));
 +
 + CHECK(dns_rbtnodechain_current(iter-chain, NULL, NULL, node));
 + if (node-data == NULL)
 + return DNS_R_EMPTYNAME;
 +
 + CHECK(dns_rbt_fullnamefromnode(node, nodename));
 + result = ISC_R_SUCCESS;
 +
 +cleanup:
 + return result;
 +}
 +
 +/**
 + * Initialize RBT iterator, lock RBT and copy name of the first node with
 + * non-NULL data. Empty RBT nodes (with data == NULL) are ignored.
 + *
 + * RBT remains locked after iterator initialization. RBT has to be
 + * unlocked by reaching end of iteration or explicit rbt_iter_stop() call.
 + *
 + * @param[in,out] rwlock   guard for RBT, will be read-locked
 + * @param[out]iter iterator structure, will be initialized
 + * @param[out]nodename dns_name with pre-allocated storage
 + *
 + * @pre Nodename has pre-allocated storage space.
 + *
 + * @retval ISC_R_SUCCESS   Node with non-NULL data found,
 + * RBT is in locked state, iterator is valid,
 + * nodename holds copy of actual RBT node name.
 + * @retval ISC_R_NOTFOUND  Node with non-NULL data is not present,
 + * RBT is in unlocked state, iterator is invalid.
 + * @retval others  Any error from rbt_iter_getnodename() and
 + * rbt_iter_next().
 + */
 +isc_result_t
 +rbt_iter_first(isc_mem_t *mctx, dns_rbt_t *rbt, isc_rwlock_t *rwlock,
 +rbt_iterator_t *iter, dns_name_t *nodename) {
 +
 + isc_result_t result;
 +
 + REQUIRE(rbt != NULL);
 + REQUIRE(rwlock != NULL);
 + REQUIRE(iter != NULL);
 +
 + ZERO_PTR(iter);
 +
 + isc_mem_attach(mctx, iter-mctx);
 + dns_rbtnodechain_init(iter-chain, mctx);
 + iter-rbt = rbt;
 + iter-rwlock = rwlock;
 + iter-locktype = isc_rwlocktype_read;
 + iter-magic = LDAPDB_RBTITER_MAGIC;
 +
 + RWLOCK(iter-rwlock, iter-locktype);
 +
 + result = dns_rbtnodechain_first(iter-chain, rbt, NULL, NULL);
 + if (result != DNS_R_NEWORIGIN) {
 + rbt_iter_stop(iter);
 + return result;

I would substitute those two lines with goto cleanup;.

 + }
 +
 + result = rbt_iter_getnodename(iter, nodename);
 + if (result == DNS_R_EMPTYNAME)
 + result = rbt_iter_next(iter, nodename);
 + if (result == ISC_R_NOMORE)
 + result = ISC_R_NOTFOUND;

In my opinion this function should leave rbt in locked state only when it
returns ISC_R_SUCCESS. All other cases should unlock the tree. I recommend to
add this statement:

cleanup:
if (result != ISC_R_SUCCESS)
rbt_iter_stop(iter);

 +
 + return result;
 +}
 +
 +/**
 + * Copy name of the next non-empty node in RBT.
 + *
 + *

Re: [Freeipa-devel] [PATCH 0123-0126] Separate master and forward zones (add idnsForwardZone object class)

2013-04-02 Thread Adam Tkac 
On Fri, Mar 22, 2013 at 01:03:12PM +0100, Petr Spacek wrote:
 Hello,
 
 this patch set separates master zones (idnsZone objectClass) from
 forward zones (idnsForwardZone objectClass). Support for forward
 zones in idnsZone objectClass is still present to ease upgrades.
 
 See each commit message for all the gory details.
 
 -- 
 Petr^2 Spacek

Ack for patch 0124.

 From 005707761a5b99d50871de91252f9f23a7441d19 Mon Sep 17 00:00:00 2001
 From: Petr Spacek pspa...@redhat.com
 Date: Fri, 22 Mar 2013 12:19:02 +0100
 Subject: [PATCH] Add missing includes to util.h.
 
 Now include util.h should be enough for util.h usage.
 
 Signed-off-by: Petr Spacek pspa...@redhat.com
 ---
  src/util.h | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)
 
 diff --git a/src/util.h b/src/util.h
 index 
 d6d3c73e6d25657805eee904e6047c542e52a656..17a3f3b4ca65ab4a80c4e4fcc9ea909bb7a9178c
  100644
 --- a/src/util.h
 +++ b/src/util.h
 @@ -21,10 +21,17 @@
  #ifndef _LD_UTIL_H_
  #define _LD_UTIL_H_
  
 -extern isc_boolean_t verbose_checks; /* from settings.c */
 +#include string.h
 +
 +#include isc/mem.h
 +#include isc/buffer.h
 +#include dns/types.h
 +#include dns/name.h
  
  #include log.h
  
 +extern isc_boolean_t verbose_checks; /* from settings.c */
 +
  #define CLEANUP_WITH(result_code)\
   do {\
   result = (result_code); \
 -- 
 1.7.11.7

-- 
Adam Tkac, Red Hat, Inc.

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

Re: [Freeipa-devel] [PATCH 0123-0126] Separate master and forward zones (add idnsForwardZone object class)

2013-04-02 Thread Petr Spacek 

On 2.4.2013 17:17, Adam Tkac wrote:

On Fri, Mar 22, 2013 at 01:03:12PM +0100, Petr Spacek wrote:

Hello,

this patch set separates master zones (idnsZone objectClass) from
forward zones (idnsForwardZone objectClass). Support for forward
zones in idnsZone objectClass is still present to ease upgrades.

See each commit message for all the gory details.


Since patches are non-trivial, I will review them per partes (i.e. each patch
in separate mail). Please check my comments below.

Regards, Adam


 From d0c598ea7e9c02a1ec786c6f1c596ae1be7ac1e2 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 22 Mar 2013 12:17:07 +0100
Subject: [PATCH] Add helper functions for generic iteration over RBT.

https://fedorahosted.org/bind-dyndb-ldap/ticket/99

Signed-off-by: Petr Spacek pspa...@redhat.com
---
  src/rbt_helper.c | 150 +++
  src/rbt_helper.h |  29 +++
  2 files changed, 179 insertions(+)
  create mode 100644 src/rbt_helper.c
  create mode 100644 src/rbt_helper.h

diff --git a/src/rbt_helper.c b/src/rbt_helper.c
new file mode 100644
index 
..70ab06134694e36a6ae049284d506bbf5bc3a977
--- /dev/null
+++ b/src/rbt_helper.c
@@ -0,0 +1,150 @@
+#include dns/rbt.h
+
+#include rbt_helper.h
+
+#define LDAPDB_RBTITER_MAGIC ISC_MAGIC('L', 'D', 'P', 'I')
+
+/**
+ * Copy the RBT node name, i.e. copies the name pointed to by RBT iterator.
+ *
+ * @param[in]  iter Initialized RBT iterator.
+ * @param[out] nodename Target dns_name suitable for rbt_fullnamefromnode() 
call.
+ *
+ * @pre Nodename has pre-allocated storage space.
+ *
+ * @retval ISC_R_SUCCESS   Actual name was copied to nodename.
+ * @retval ISC_R_NOTFOUND  Iterator doesn't point to any node.
+ * @retval DNS_R_EMPTYNAME Iterator points to name without assigned data,
+ * nodename is unchanged.
+ * @retval others  Errors from dns_name_concatenate() and others.
+ *
+ */
+static isc_result_t
+rbt_iter_getnodename(rbt_iterator_t *iter, dns_name_t *nodename) {
+   isc_result_t result;
+   dns_rbtnode_t *node = NULL;
+
+   REQUIRE(iter != NULL);
+   REQUIRE(nodename != NULL);
+   REQUIRE(ISC_MAGIC_VALID(iter, LDAPDB_RBTITER_MAGIC));
+
+   CHECK(dns_rbtnodechain_current(iter-chain, NULL, NULL, node));
+   if (node-data == NULL)
+   return DNS_R_EMPTYNAME;
+
+   CHECK(dns_rbt_fullnamefromnode(node, nodename));
+   result = ISC_R_SUCCESS;
+
+cleanup:
+   return result;
+}
+
+/**
+ * Initialize RBT iterator, lock RBT and copy name of the first node with
+ * non-NULL data. Empty RBT nodes (with data == NULL) are ignored.
+ *
+ * RBT remains locked after iterator initialization. RBT has to be
+ * unlocked by reaching end of iteration or explicit rbt_iter_stop() call.
+ *
+ * @param[in,out] rwlock   guard for RBT, will be read-locked
+ * @param[out]iter iterator structure, will be initialized
+ * @param[out]nodename dns_name with pre-allocated storage
+ *
+ * @pre Nodename has pre-allocated storage space.
+ *
+ * @retval ISC_R_SUCCESS   Node with non-NULL data found,
+ * RBT is in locked state, iterator is valid,
+ * nodename holds copy of actual RBT node name.
+ * @retval ISC_R_NOTFOUND  Node with non-NULL data is not present,
+ * RBT is in unlocked state, iterator is invalid.
+ * @retval others  Any error from rbt_iter_getnodename() and
+ * rbt_iter_next().
+ */
+isc_result_t
+rbt_iter_first(isc_mem_t *mctx, dns_rbt_t *rbt, isc_rwlock_t *rwlock,
+  rbt_iterator_t *iter, dns_name_t *nodename) {
+
+   isc_result_t result;
+
+   REQUIRE(rbt != NULL);
+   REQUIRE(rwlock != NULL);
+   REQUIRE(iter != NULL);
+
+   ZERO_PTR(iter);
+
+   isc_mem_attach(mctx, iter-mctx);
+   dns_rbtnodechain_init(iter-chain, mctx);
+   iter-rbt = rbt;
+   iter-rwlock = rwlock;
+   iter-locktype = isc_rwlocktype_read;
+   iter-magic = LDAPDB_RBTITER_MAGIC;
+
+   RWLOCK(iter-rwlock, iter-locktype);
+
+   result = dns_rbtnodechain_first(iter-chain, rbt, NULL, NULL);
+   if (result != DNS_R_NEWORIGIN) {
+   rbt_iter_stop(iter);
+   return result;


I would substitute those two lines with goto cleanup;.


+   }
+
+   result = rbt_iter_getnodename(iter, nodename);
+   if (result == DNS_R_EMPTYNAME)
+   result = rbt_iter_next(iter, nodename);
+   if (result == ISC_R_NOMORE)
+   result = ISC_R_NOTFOUND;


In my opinion this function should leave rbt in locked state only when it
returns ISC_R_SUCCESS. All other cases should unlock the tree. I recommend to
add this statement:

cleanup:
if (result != ISC_R_SUCCESS)
rbt_iter_stop(iter);


+
+   return result;
+}
+
+/**
+ * Copy name of the next non-empty node in RBT.
+ *
+ * @param[in]

Re: [Freeipa-devel] [PATCH 0123-0126] Separate master and forward zones (add idnsForwardZone object class)

2013-04-02 Thread Adam Tkac 
On Fri, Mar 22, 2013 at 01:03:12PM +0100, Petr Spacek wrote:
 Hello,
 
 this patch set separates master zones (idnsZone objectClass) from
 forward zones (idnsForwardZone objectClass). Support for forward
 zones in idnsZone objectClass is still present to ease upgrades.
 
 See each commit message for all the gory details.

Just check one comment below, otherwise ack.

 From 71fc42de24d3709efbe7dee24973c1b456b37fe4 Mon Sep 17 00:00:00 2001
 From: Petr Spacek pspa...@redhat.com
 Date: Fri, 22 Mar 2013 12:38:55 +0100
 Subject: [PATCH] Add support for pure forward zones - idnsForwardZone
  objectClass.
 
 Master zones are stored in zone_register and pure forward zones
 are stored in fwd_register.
 
 This patch doesn't remove support for forward zones within
 idnsZone objectClass. Support for forward zones in both
 objectClasses enables incremental update, where old and new
 plugin versions operate on the same LDAP database.
 
 Support for forward zones defined by idnsZone objectClass
 will be removed in near future.
 
 Forward zones defined in idnsZone objectClass are not disabled
 after removing from LDAP if persistent search is disabled
 (see ticket #106).
 This problem doesn't affect zones defined with idnsForwardZone
 objectClass.
 
 https://fedorahosted.org/bind-dyndb-ldap/ticket/99
 
 Signed-off-by: Petr Spacek pspa...@redhat.com
 ---
  src/Makefile.am|   4 +
  src/fwd_register.c | 156 +
  src/fwd_register.h |  35 ++
  src/ldap_entry.c   |  33 --
  src/ldap_entry.h   |   7 +-
  src/ldap_helper.c  | 334 
 ++---
  6 files changed, 440 insertions(+), 129 deletions(-)
  create mode 100644 src/fwd_register.c
  create mode 100644 src/fwd_register.h
 
 diff --git a/src/Makefile.am b/src/Makefile.am
 index 
 252255788b01e003031f5f0ee2fc8469b53633be..87c3252736fa4f918f105166497b32b0219ef8ea
  100644
 --- a/src/Makefile.am
 +++ b/src/Makefile.am
 @@ -5,11 +5,13 @@ HDRS =  \
   acl.h   \
   cache.h \
   compat.h\
 + fwd_register.h  \
   krb5_helper.h   \
   ldap_convert.h  \
   ldap_entry.h\
   ldap_helper.h   \
   log.h   \
 + rbt_helper.h\
   rdlist.h\
   semaphore.h \
   settings.h  \
 @@ -23,12 +25,14 @@ ldap_la_SOURCES = \
   $(HDRS) \
   acl.c   \
   cache.c \
 + fwd_register.c  \
   krb5_helper.c   \
   ldap_convert.c  \
   ldap_driver.c   \
   ldap_entry.c\
   ldap_helper.c   \
   log.c   \
 + rbt_helper.c\
   rdlist.c\
   semaphore.c \
   settings.c  \
 diff --git a/src/fwd_register.c b/src/fwd_register.c
 new file mode 100644
 index 
 ..c663b25909b0e393421c49950d1f29a1352cfe6c
 --- /dev/null
 +++ b/src/fwd_register.c
 @@ -0,0 +1,156 @@
 +#include isc/rwlock.h
 +#include dns/name.h
 +
 +#include rbt_helper.h
 +#include fwd_register.h
 +#include util.h
 +
 +struct fwd_register {
 + isc_mem_t   *mctx;
 + isc_rwlock_trwlock;
 + dns_rbt_t   *rbt;
 +};
 +
 +isc_result_t
 +fwdr_create(isc_mem_t *mctx, fwd_register_t **fwdrp)
 +{
 + isc_result_t result;
 + fwd_register_t *fwdr = NULL;
 +
 + REQUIRE(fwdrp != NULL  *fwdrp == NULL);
 +
 + CHECKED_MEM_GET_PTR(mctx, fwdr);
 + ZERO_PTR(fwdr);
 + isc_mem_attach(mctx, fwdr-mctx);
 + CHECK(dns_rbt_create(mctx, NULL, NULL, fwdr-rbt));
 + CHECK(isc_rwlock_init(fwdr-rwlock, 0, 0));
 +
 + *fwdrp = fwdr;
 + return ISC_R_SUCCESS;
 +
 +cleanup:
 + if (fwdr != NULL) {
 + if (fwdr-rbt != NULL)
 + dns_rbt_destroy(fwdr-rbt);
 + MEM_PUT_AND_DETACH(fwdr);
 + }
 +
 + return result;
 +}
 +
 +void
 +fwdr_destroy(fwd_register_t **fwdrp)
 +{
 + fwd_register_t *fwdr;
 +
 + if (fwdrp == NULL || *fwdrp == NULL)
 + return;
 +
 + fwdr = *fwdrp;
 +
 + RWLOCK(fwdr-rwlock, isc_rwlocktype_write);
 + dns_rbt_destroy(fwdr-rbt);
 + RWUNLOCK(fwdr-rwlock, isc_rwlocktype_write);
 + isc_rwlock_destroy(fwdr-rwlock);
 + MEM_PUT_AND_DETACH(fwdr);
 +
 + *fwdrp = NULL;
 +}
 +
 +/*
 + * Add forward zone to the forwarding register 'fwdr'. Origin of the zone
 + * must be absolute and the zone cannot already be in the register.
 + */
 +isc_result_t
 +fwdr_add_zone(fwd_register_t *fwdr, dns_name_t *name)
 +{
 + isc_result_t result;
 + void *dummy = NULL;
 +
 + REQUIRE(fwdr != NULL);
 + REQUIRE(name != NULL);
 +
 + if (!dns_name_isabsolute(name)) {
 + log_bug(forward zone with bad origin);
 + return ISC_R_FAILURE;
 + }

Re: [Freeipa-devel] [PATCH 0123-0126] Separate master and forward zones (add idnsForwardZone object class)

2013-04-02 Thread Petr Spacek 

On 2.4.2013 17:18, Adam Tkac wrote:

On Fri, Mar 22, 2013 at 01:03:12PM +0100, Petr Spacek wrote:

Hello,

this patch set separates master zones (idnsZone objectClass) from
forward zones (idnsForwardZone objectClass). Support for forward
zones in idnsZone objectClass is still present to ease upgrades.

See each commit message for all the gory details.

--
Petr^2  Spacek

Ack for patch 0124.


Pushed to master: 59b157618c2b241740f3b3125e6da6230fa0314c

--
Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH 0123-0126] Separate master and forward zones (add idnsForwardZone object class)

2013-04-02 Thread Petr Spacek 

On 2.4.2013 18:34, Adam Tkac wrote:

On Fri, Mar 22, 2013 at 01:03:12PM +0100, Petr Spacek wrote:

Hello,

this patch set separates master zones (idnsZone objectClass) from
forward zones (idnsForwardZone objectClass). Support for forward
zones in idnsZone objectClass is still present to ease upgrades.

See each commit message for all the gory details.

Just check one comment below, otherwise ack.

[...]


if (result == ISC_R_SUCCESS) {
log_debug(5, Refresh %s, entry-dn);
/* Add found zone to RB-tree for later check. */
-   result = dns_rbt_addname(rbt, name, NULL);
+   if (zone_class  LDAP_ENTRYCLASS_MASTER)
+   result = dns_rbt_addname(master_rbt, name, 
NULL);
+   else


In my opinion you should use else if (zone_class  LDAP_ENTRYCLASS_FORWARD)
here.


+   result = dns_rbt_addname(forward_rbt, name, 
NULL);
}


Fixed version pushed to master: 760bebb0e8744301420cf6e4918690ed171529a2

--
Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH 0123-0126] Separate master and forward zones (add idnsForwardZone object class)

2013-04-02 Thread Petr Spacek 

On 2.4.2013 17:30, Adam Tkac wrote:

On Fri, Mar 22, 2013 at 01:03:12PM +0100, Petr Spacek wrote:

Hello,

this patch set separates master zones (idnsZone objectClass) from
forward zones (idnsForwardZone objectClass). Support for forward
zones in idnsZone objectClass is still present to ease upgrades.

See each commit message for all the gory details.

--
Petr^2  Spacek

Ack for patch 125 as is.


Pushed to master: edb6dbcf7a81605e6ccbd8efe1e323862710e0f7

--
Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH 0123-0126] Separate master and forward zones (add idnsForwardZone object class)

2013-04-02 Thread Petr Spacek 

On 2.4.2013 17:17, Adam Tkac wrote:

On Fri, Mar 22, 2013 at 01:03:12PM +0100, Petr Spacek wrote:

Hello,

this patch set separates master zones (idnsZone objectClass) from
forward zones (idnsForwardZone objectClass). Support for forward
zones in idnsZone objectClass is still present to ease upgrades.

See each commit message for all the gory details.

Since patches are non-trivial, I will review them per partes (i.e. each patch
in separate mail). Please check my comments below.

Regards, Adam


After discussion I pushed original version to master:
9d073c1ef7c28e29397a766320d12ecdb7e1941b

--
Petr^2 Spacek

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