this patch adresses issues in checking existing segments for one directional segments and correctly handles the merging of segments, so that all agreements will be removed when the merged segment is deleted
>From ad9850b00f369be67c0240b084afaf2ce1c97a9f Mon Sep 17 00:00:00 2001
From: Ludwig Krispenz <lkris...@redhat.com>
Date: Tue, 16 Jun 2015 10:25:22 +0200
Subject: [PATCH] correct management of one directional segments

    this patch contains the following improvements:
    check for existing segments works for all combinations of one directional and bidirectional segments
    rdns of replication agreements generated from one directional segments are preserves after
        merging of segments, so that deletion of the segment deletes the corresponding replication
        agreements
---
 daemons/ipa-slapi-plugins/topology/topology.h      |   5 +-
 daemons/ipa-slapi-plugins/topology/topology_cfg.c  |  18 ++-
 daemons/ipa-slapi-plugins/topology/topology_post.c |   8 +-
 daemons/ipa-slapi-plugins/topology/topology_pre.c  |   9 +-
 daemons/ipa-slapi-plugins/topology/topology_util.c | 159 +++++++++++++++++----
 5 files changed, 160 insertions(+), 39 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/topology/topology.h b/daemons/ipa-slapi-plugins/topology/topology.h
index 4135a8ff71b9160919a089fde63a95a989830de8..9c680e7cfd636bb783b51ed904c74f34c4e4cf20 100644
--- a/daemons/ipa-slapi-plugins/topology/topology.h
+++ b/daemons/ipa-slapi-plugins/topology/topology.h
@@ -177,11 +177,12 @@ void ipa_topo_cfg_host_add(Slapi_Entry *hostentry);
 void ipa_topo_cfg_host_del(Slapi_Entry *hostentry);
 TopoReplicaHost *ipa_topo_cfg_host_find(TopoReplica *tconf, char *host, int lock);
 TopoReplicaHost *ipa_topo_cfg_host_new(char *newhost);
+int ipa_topo_util_segm_dir(char *direction);
 TopoReplicaSegment *
-ipa_topo_cfg_segment_find(char *repl_root, char *leftHost, char *rightHost);
+ipa_topo_cfg_segment_find(char *repl_root, char *leftHost, char *rightHosti, int dir);
 TopoReplicaSegment *
 ipa_topo_cfg_replica_segment_find(TopoReplica *tconf, char *leftHost,
-                                  char *rightHost, int lock);
+                                  char *rightHost, int dir, int lock);
 void ipa_topo_cfg_segment_set_visited(TopoReplica *tconf, TopoReplicaSegment *tsegm);
 void ipa_topo_cfg_segment_add(TopoReplica *tconf, TopoReplicaSegment *tsegm);
 void ipa_topo_cfg_segment_del(TopoReplica *tconf, TopoReplicaSegment *tsegm);
diff --git a/daemons/ipa-slapi-plugins/topology/topology_cfg.c b/daemons/ipa-slapi-plugins/topology/topology_cfg.c
index 982ad647db9737c1aa0fc7f68c7d9b20de895fb6..19b6947a76d2264367e05a89c9c24487816d078a 100644
--- a/daemons/ipa-slapi-plugins/topology/topology_cfg.c
+++ b/daemons/ipa-slapi-plugins/topology/topology_cfg.c
@@ -545,19 +545,25 @@ ipa_topo_cfg_host_del(Slapi_Entry *hostentry)
 }
 
 TopoReplicaSegment *
-ipa_topo_cfg_replica_segment_find(TopoReplica *replica, char *leftHost, char *rightHost, int lock)
+ipa_topo_cfg_replica_segment_find(TopoReplica *replica, char *leftHost, char *rightHost, int dir, int lock)
 {
     TopoReplicaSegment *tsegm = NULL;
     TopoReplicaSegmentList *segments = NULL;
+    int reverse_dir = SEGMENT_BIDIRECTIONAL;
+
+    if (dir == SEGMENT_LEFT_RIGHT) reverse_dir = SEGMENT_RIGHT_LEFT;
+    else if (dir == SEGMENT_RIGHT_LEFT) reverse_dir = SEGMENT_LEFT_RIGHT;
+    else reverse_dir = SEGMENT_BIDIRECTIONAL;
 
     if (lock) slapi_lock_mutex(replica->repl_lock);
     segments = replica->repl_segments;
     while (segments) {
+
         tsegm = segments->segm;
         if ( (!strcasecmp(leftHost,tsegm->from) && !strcasecmp(rightHost,tsegm->to) &&
-             (tsegm->direct == SEGMENT_BIDIRECTIONAL || tsegm->direct == SEGMENT_LEFT_RIGHT)) ||
+             (tsegm->direct & dir)) ||
              (!strcasecmp(leftHost,tsegm->to) && !strcasecmp(rightHost,tsegm->from) &&
-             (tsegm->direct == SEGMENT_BIDIRECTIONAL || tsegm->direct == SEGMENT_RIGHT_LEFT))) {
+             (tsegm->direct & reverse_dir))) {
            break;
         }
         tsegm = NULL;
@@ -680,7 +686,7 @@ ipa_topo_cfg_segment_dup(TopoReplicaSegment *orig)
 }
 
 TopoReplicaSegment *
-ipa_topo_cfg_segment_find(char *repl_root, char *leftHost, char *rightHost)
+ipa_topo_cfg_segment_find(char *repl_root, char *leftHost, char *rightHost, int dir)
 {
     TopoReplicaSegment *tsegm = NULL;
     TopoReplica *replica = NULL;
@@ -689,7 +695,7 @@ ipa_topo_cfg_segment_find(char *repl_root, char *leftHost, char *rightHost)
 
     replica = ipa_topo_cfg_replica_find(repl_root, 0);
     if (replica) {
-        tsegm = ipa_topo_cfg_replica_segment_find(replica,leftHost,rightHost, 1);
+        tsegm = ipa_topo_cfg_replica_segment_find(replica,leftHost,rightHost, dir, 1);
     }
     slapi_unlock_mutex(topo_shared_conf.conf_lock);
     return tsegm;
@@ -731,7 +737,7 @@ ipa_topo_cfg_segment_add(TopoReplica *replica, TopoReplicaSegment *tsegm)
     slapi_lock_mutex(replica->repl_lock);
     if (ipa_topo_cfg_replica_segment_find(replica,
                                           tsegm->from,
-                                          tsegm->to, 0)){
+                                          tsegm->to, tsegm->direct, 0)){
         /* already exists: log error */
         slapi_log_error(SLAPI_LOG_PLUGIN, IPA_TOPO_PLUGIN_SUBSYSTEM,
                         "ipa_topo_cfg_segment_add: error: segment exists: %s\n",
diff --git a/daemons/ipa-slapi-plugins/topology/topology_post.c b/daemons/ipa-slapi-plugins/topology/topology_post.c
index 33ded2ad33a3c56f4ecc4c1606187ff31ad9744e..c01505bdedc1b09488b21561315c721dc3847969 100644
--- a/daemons/ipa-slapi-plugins/topology/topology_post.c
+++ b/daemons/ipa-slapi-plugins/topology/topology_post.c
@@ -129,14 +129,16 @@ ipa_topo_post_mod(Slapi_PBlock *pb)
     int result = SLAPI_PLUGIN_SUCCESS;
     int entry_type;
     Slapi_Entry *mod_entry = NULL;
+    Slapi_Entry *pre_entry = NULL;
 
     slapi_log_error(SLAPI_LOG_PLUGIN, IPA_TOPO_PLUGIN_SUBSYSTEM,
                     "--> ipa_topo_post_mod\n");
 
     /* 1. get entry  */
     slapi_pblock_get(pb,SLAPI_ENTRY_POST_OP,&mod_entry);
+    slapi_pblock_get(pb,SLAPI_ENTRY_PRE_OP,&pre_entry);
 
-    if (mod_entry == NULL) {
+    if (mod_entry == NULL || pre_entry == NULL) {
         slapi_log_error(SLAPI_LOG_PLUGIN, IPA_TOPO_PLUGIN_SUBSYSTEM, "no entry\n");
         return (1);
     }
@@ -155,8 +157,8 @@ ipa_topo_post_mod(Slapi_PBlock *pb)
     case TOPO_SEGMENT_ENTRY: {
         LDAPMod **mods;
         TopoReplica *tconf = ipa_topo_util_get_conf_for_segment(mod_entry);
-        TopoReplicaSegment *tsegm;
-        tsegm = ipa_topo_util_find_segment(tconf, mod_entry);
+        TopoReplicaSegment *tsegm = NULL;
+        if (tconf) tsegm = ipa_topo_util_find_segment(tconf, pre_entry);
         if (tsegm == NULL) {
             slapi_log_error(SLAPI_LOG_FATAL, IPA_TOPO_PLUGIN_SUBSYSTEM,
                             "ipa_topo_post_mod - segment to be modified does not exist\n");
diff --git a/daemons/ipa-slapi-plugins/topology/topology_pre.c b/daemons/ipa-slapi-plugins/topology/topology_pre.c
index cf935d8d2ad65b3c95f408d44511a8c0449c7288..c324cf69ed8396431a3171aefa5a02fc9e1dd1d9 100644
--- a/daemons/ipa-slapi-plugins/topology/topology_pre.c
+++ b/daemons/ipa-slapi-plugins/topology/topology_pre.c
@@ -306,7 +306,13 @@ ipa_topo_check_segment_is_valid(Slapi_PBlock *pb)
          */
         char *leftnode = slapi_entry_attr_get_charptr(add_entry,"ipaReplTopoSegmentLeftNode");
         char *rightnode = slapi_entry_attr_get_charptr(add_entry,"ipaReplTopoSegmentRightNode");
-        if (0 == strcasecmp(leftnode,rightnode)) {
+        char *dir = slapi_entry_attr_get_charptr(add_entry,"ipaReplTopoSegmentDirection");
+        if (strcasecmp(dir,SEGMENT_DIR_BOTH) && strcasecmp(dir,SEGMENT_DIR_LEFT_ORIGIN) &&
+            strcasecmp(dir,SEGMENT_DIR_RIGHT_ORIGIN)) {
+                slapi_log_error(SLAPI_LOG_FATAL, IPA_TOPO_PLUGIN_SUBSYSTEM,
+                                "segment has unknown direction: %s\n", dir);
+                rc = 1;
+        } else if (0 == strcasecmp(leftnode,rightnode)) {
                 slapi_log_error(SLAPI_LOG_FATAL, IPA_TOPO_PLUGIN_SUBSYSTEM,
                                 "segment is self referential\n");
                 rc = 1;
@@ -322,6 +328,7 @@ ipa_topo_check_segment_is_valid(Slapi_PBlock *pb)
         }
         slapi_ch_free_string(&leftnode);
         slapi_ch_free_string(&rightnode);
+        slapi_ch_free_string(&dir);
     }
     return rc;
 }
diff --git a/daemons/ipa-slapi-plugins/topology/topology_util.c b/daemons/ipa-slapi-plugins/topology/topology_util.c
index cd97827b17d3a276974331f7da7bf0eae40c5a81..9851df059dc3e79ea0ae48d094de79b84ecfb086 100644
--- a/daemons/ipa-slapi-plugins/topology/topology_util.c
+++ b/daemons/ipa-slapi-plugins/topology/topology_util.c
@@ -319,20 +319,38 @@ ipa_topo_util_agmt_from_entry(Slapi_Entry *entry, char *replRoot, char *fromHost
     }
     return agmt;
 }
+
+int
+ipa_topo_util_segm_dir(char *direction)
+{
+    int dir = -1;
+    if (strcasecmp(direction,SEGMENT_DIR_BOTH) == 0){
+        dir = SEGMENT_BIDIRECTIONAL;
+    } else if (strcasecmp(direction,SEGMENT_DIR_LEFT_ORIGIN) == 0) {
+        dir = SEGMENT_LEFT_RIGHT;
+    } else if (strcasecmp(direction,SEGMENT_DIR_RIGHT_ORIGIN) == 0) {
+        dir = SEGMENT_RIGHT_LEFT;
+    }
+    return dir;
+}
+
 TopoReplicaSegment *
 ipa_topo_util_find_segment(TopoReplica *conf, Slapi_Entry *entry)
 {
     char *leftHost;
     char *rightHost;
+    char *direction;
     TopoReplicaSegment *segment = NULL;
 
     leftHost = slapi_entry_attr_get_charptr(entry,"ipaReplTopoSegmentLeftNode");
     rightHost = slapi_entry_attr_get_charptr(entry,"ipaReplTopoSegmentRightNode");
+    direction = slapi_entry_attr_get_charptr(entry,"ipaReplTopoSegmentDirection");
 
-    segment = ipa_topo_cfg_segment_find(conf->repl_root, leftHost, rightHost);
+    segment = ipa_topo_cfg_segment_find(conf->repl_root, leftHost, rightHost, ipa_topo_util_segm_dir(direction));
 
     slapi_ch_free((void **)&leftHost);
     slapi_ch_free((void **)&rightHost);
+    slapi_ch_free((void **)&direction);
     return segment;
 }
 
@@ -450,13 +468,63 @@ ipa_topo_util_conf_from_entry(Slapi_Entry *entry)
     }
 }
 
+void
+ipa_topo_util_set_agmt_rdn(TopoReplicaAgmt *topo_agmt, Slapi_Entry *repl_agmt)
+{
+    const Slapi_DN *agmt_dn = slapi_entry_get_sdn_const(repl_agmt);
+    Slapi_RDN *agmt_rdn = slapi_rdn_new();
+    slapi_sdn_get_rdn(agmt_dn, agmt_rdn);
+    const char *agmt_rdn_str  = slapi_rdn_get_rdn(agmt_rdn);
+    if (strcasecmp(agmt_rdn_str, topo_agmt->rdn)) {
+        slapi_ch_free_string(&topo_agmt->rdn);
+        topo_agmt->rdn = slapi_ch_strdup(agmt_rdn_str);
+    }
+    slapi_rdn_free(&agmt_rdn);
+}
+
+int
+ipa_topo_util_update_agmt_rdn(TopoReplica *conf, TopoReplicaAgmt *agmt,
+                              char *toHost)
+{
+    int rc = 0;
+    Slapi_PBlock *pb = NULL;
+    Slapi_Entry **entries = NULL;
+    char *filter;
+
+    pb = slapi_pblock_new();
+    filter = slapi_ch_smprintf("(&(objectclass=nsds5replicationagreement)"
+                               "(nsds5replicaroot=%s)(nsds5replicahost=%s))",
+                               conf->repl_root, toHost);
+    slapi_search_internal_set_pb(pb, "cn=config", LDAP_SCOPE_SUB,
+                                 filter, NULL, 0, NULL, NULL,
+                                 ipa_topo_get_plugin_id(), 0);
+    slapi_search_internal_pb(pb);
+    slapi_pblock_get(pb, SLAPI_PLUGIN_INTOP_RESULT, &rc);
+    if (rc == 0) {
+        slapi_pblock_get(pb, SLAPI_PLUGIN_INTOP_SEARCH_ENTRIES, &entries);
+    }
+
+    if (NULL == entries || NULL == entries[0]) {
+        slapi_log_error(SLAPI_LOG_PLUGIN, IPA_TOPO_PLUGIN_SUBSYSTEM,
+                            "ipa_topo_util_update_agmt_rdn: "
+                            "no agreements found\n");
+    } else {
+        ipa_topo_util_set_agmt_rdn(agmt, entries[0]);
+    }
+
+    slapi_free_search_results_internal(pb);
+    slapi_ch_free_string(&filter);
+    slapi_pblock_destroy(pb);
+    return rc;
+}
+
 int
 ipa_topo_util_update_agmt_list(TopoReplica *conf, TopoReplicaSegmentList *repl_segments)
 {
     int rc = 0;
     int i;
     int nentries;
-    Slapi_Entry **entries;
+    Slapi_Entry **entries = NULL;
     Slapi_Entry *repl_agmt;
     Slapi_PBlock *pb = NULL;
     char *filter;
@@ -482,7 +550,7 @@ ipa_topo_util_update_agmt_list(TopoReplica *conf, TopoReplicaSegmentList *repl_s
         if (NULL == entries || NULL == entries[0]) {
             slapi_log_error(SLAPI_LOG_PLUGIN, IPA_TOPO_PLUGIN_SUBSYSTEM,
                             "ipa_topo_util_update_agmts_list: "
-                            "no agrements found\n");
+                            "no agreements found\n");
             goto update_only;
         }
     }
@@ -511,15 +579,7 @@ ipa_topo_util_update_agmt_list(TopoReplica *conf, TopoReplicaSegmentList *repl_s
                                                     targetHost);
         if (topo_agmt) {
             /* compare rdns, use rdn of existing agreement */
-            const Slapi_DN *agmt_dn = slapi_entry_get_sdn_const(repl_agmt);
-            Slapi_RDN *agmt_rdn = slapi_rdn_new();
-            slapi_sdn_get_rdn(agmt_dn, agmt_rdn);
-            const char *agmt_rdn_str  = slapi_rdn_get_rdn(agmt_rdn);
-            if (strcasecmp(agmt_rdn_str, topo_agmt->rdn)) {
-                slapi_ch_free_string(&topo_agmt->rdn);
-                topo_agmt->rdn = slapi_ch_strdup(agmt_rdn_str);
-            }
-            slapi_rdn_free(&agmt_rdn);
+            ipa_topo_util_set_agmt_rdn(topo_agmt, repl_agmt);
 
             /* update agreement params which are different in the segment*/
             char *segm_attr_val;
@@ -742,10 +802,29 @@ ipa_topo_util_segment_update(TopoReplica *repl_conf,
         case LDAP_MOD_REPLACE:
             if (0 == strcasecmp(mods[i]->mod_type,"ipaReplTopoSegmentDirection")) {
                 if (0 == strcasecmp(mods[i]->mod_bvalues[0]->bv_val,"both")) {
+                    TopoReplicaSegment *ex_segm;
                     if (segment->direct == SEGMENT_LEFT_RIGHT) {
-                        segment->right = ipa_topo_cfg_agmt_dup_reverse(segment->left);
+                        ex_segm = ipa_topo_cfg_replica_segment_find(repl_conf, segment->from, segment->to,
+                                                    SEGMENT_RIGHT_LEFT, 1);
+                        if (ex_segm) {
+                            segment->right = ipa_topo_cfg_agmt_dup(ex_segm->left?ex_segm->left:ex_segm->right);
+                        } else {
+                            segment->right = ipa_topo_cfg_agmt_dup_reverse(segment->left);
+                            if(0 == strcasecmp(fromHost,segment->right->origin)) {
+                                ipa_topo_util_update_agmt_rdn(repl_conf, segment->right, segment->right->target);
+                            }
+                        }
                     } else if (segment->direct == SEGMENT_RIGHT_LEFT) {
-                        segment->left = ipa_topo_cfg_agmt_dup_reverse(segment->right);
+                        ex_segm = ipa_topo_cfg_replica_segment_find(repl_conf, segment->from, segment->to,
+                                                    SEGMENT_LEFT_RIGHT, 1);
+                        if (ex_segm) {
+                            segment->left = ipa_topo_cfg_agmt_dup(ex_segm->left?ex_segm->left:ex_segm->right);
+                        } else {
+                            segment->left = ipa_topo_cfg_agmt_dup_reverse(segment->right);
+                            if(0 == strcasecmp(fromHost,segment->left->origin)) {
+                                ipa_topo_util_update_agmt_rdn(repl_conf, segment->left, segment->left->target);
+                            }
+                        }
                     }
                     segment->direct = SEGMENT_BIDIRECTIONAL;
                 } else {
@@ -983,6 +1062,32 @@ ipa_topo_util_segm_order(TopoReplicaSegment *l, TopoReplicaSegment *r)
 }
 
 void
+ipa_topo_util_segment_do_merge(TopoReplica *tconf,
+                               TopoReplicaSegment *ex_segm, TopoReplicaSegment *tsegm)
+{
+    /* we are merging two one directional segments, tehy can have been created in 
+     * several ways and we need to find the agreeemnt to copy 
+     */
+    if (NULL == tsegm->right) {
+        if(ex_segm->left) {
+            tsegm->right = ipa_topo_cfg_agmt_dup(ex_segm->left);
+        } else {
+            tsegm->right = ipa_topo_cfg_agmt_dup(ex_segm->right);
+        }
+    } else {
+        if(ex_segm->left) {
+            tsegm->left = ipa_topo_cfg_agmt_dup(ex_segm->left);
+        } else {
+            tsegm->left = ipa_topo_cfg_agmt_dup(ex_segm->right);
+        }
+    }
+    ipa_topo_util_segm_update(tconf,ex_segm, SEGMENT_OBSOLETE);
+    ipa_topo_util_segm_remove(tconf, ex_segm);
+    ipa_topo_util_segm_update(tconf,tsegm, SEGMENT_BIDIRECTIONAL);
+}
+
+
+void
 ipa_topo_util_segment_merge(TopoReplica *tconf,
                                  TopoReplicaSegment *tsegm)
 {
@@ -996,7 +1101,13 @@ ipa_topo_util_segment_merge(TopoReplica *tconf,
         return;
     }
 
-    ex_segm = ipa_topo_cfg_replica_segment_find(tconf, tsegm->to, tsegm->from, 1 /*lock*/);
+    if (tsegm->direct == SEGMENT_LEFT_RIGHT) {
+        ex_segm = ipa_topo_cfg_replica_segment_find(tconf, tsegm->from, tsegm->to,
+                                                    SEGMENT_RIGHT_LEFT, 1 /*lock*/);
+    } else {
+        ex_segm = ipa_topo_cfg_replica_segment_find(tconf, tsegm->from, tsegm->to,
+                                                    SEGMENT_LEFT_RIGHT, 1 /*lock*/);
+    }
     if (ex_segm == NULL) return;
 
     /* to avoid conflicts merging has to be done only once and
@@ -1005,17 +1116,11 @@ ipa_topo_util_segment_merge(TopoReplica *tconf,
      */
     if (ipa_topo_util_segm_order(ex_segm, tsegm) > 0) {
         if (0 == strcasecmp(tsegm->from,ipa_topo_get_plugin_hostname())) {
-            tsegm->right = ipa_topo_cfg_agmt_dup(ex_segm->left);
-            ipa_topo_util_segm_update(tconf,ex_segm, SEGMENT_OBSOLETE);
-            ipa_topo_util_segm_remove(tconf, ex_segm);
-            ipa_topo_util_segm_update(tconf,tsegm, SEGMENT_BIDIRECTIONAL);
+            ipa_topo_util_segment_do_merge(tconf, ex_segm, tsegm);
         }
     } else {
         if (0 == strcasecmp(ex_segm->from,ipa_topo_get_plugin_hostname())) {
-            ex_segm->right = ipa_topo_cfg_agmt_dup(tsegm->left);
-            ipa_topo_util_segm_update(tconf,tsegm, SEGMENT_OBSOLETE);
-            ipa_topo_util_segm_remove(tconf, tsegm);
-            ipa_topo_util_segm_update(tconf,ex_segm, SEGMENT_BIDIRECTIONAL);
+            ipa_topo_util_segment_do_merge(tconf, tsegm, ex_segm);
         }
     }
 
@@ -1221,8 +1326,8 @@ ipa_topo_util_delete_segments_for_host(char *repl_root, char *delhost)
     int check_reverse = 1;
 
     /* first check if a segment originating at localhost exists */
-    segm = ipa_topo_cfg_segment_find(repl_root,
-                                     ipa_topo_get_plugin_hostname(), delhost);
+    segm = ipa_topo_cfg_segment_find(repl_root, ipa_topo_get_plugin_hostname(),
+                                     delhost, SEGMENT_LEFT_RIGHT);
     if (segm) {
         /* mark segment as removable, bypass connectivity check when replicated */
         if (segm->direct == SEGMENT_BIDIRECTIONAL) check_reverse = 0;
@@ -1233,8 +1338,8 @@ ipa_topo_util_delete_segments_for_host(char *repl_root, char *delhost)
     }
     /* check if one directional segment in reverse direction exists */
     if (check_reverse) {
-        segm = ipa_topo_cfg_segment_find(repl_root,
-                                     delhost, ipa_topo_get_plugin_hostname());
+        segm = ipa_topo_cfg_segment_find(repl_root, delhost,
+                                         ipa_topo_get_plugin_hostname(), SEGMENT_LEFT_RIGHT);
         if (segm) {
             ipa_topo_util_segm_update(tconf,segm, SEGMENT_REMOVED);
             /* mark and delete, no repl agmt on this server */
-- 
2.1.0

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to