new patch with comments attached

On 06/30/2015 10:43 AM, thierry bordaz wrote:
On 06/30/2015 09:19 AM, Ludwig Krispenz wrote:


On 06/26/2015 02:14 PM, thierry bordaz wrote:
On 06/22/2015 11:35 AM, Ludwig Krispenz wrote:
fix for ticket #5065, removing start
- after online init copmpleted
- additionally check after startup


Hi Ludwig,

The fix looks good to me.
I have just a clarification regarding ipa_topo_util_reset_init. It resets 'nsds5BeginReplicaRefresh' at the condition the segment->[left,right]->target=localhost.
it is called "post_init", after an online initialization, so the host where this is checked was the target of an init. at startup, when there is a check, if it is still present, it will check that it is the origin of a refresh, clear it and not repeat the init

OK I understand my mistake now. Thanks for your explanations.

Would you add a comment that when calling ipa_topo_util_remove_init_attr (in ipa_topo_util_update_agmt_list) that it will reset 'nsds5BeginReplicaRefresh' when the host is a supplier. Also when calling ipa_topo_util_reset_init (in ipa_topo_apply_shared_config) that it will reset 'nsds5BeginReplicaRefresh' when the host is a consumer.

An other point, ipa_topo_apply_shared_config is called after an online init of the main suffix. It will reset all 'nsds5BeginReplicaRefresh' (via ipa_topo_apply_shared_replica_config and via ipa_topo_util_reset_init) on all suffixes. IMHO it is fine because reinit the shared tree should reset all administrative tasks, but may be it worth a comment.

Otherwise the patch is ok for me.

ACK


thanks
thierry

I would expect it resets the flag on the master side and so it tests 'segment->[left,right]->origin=localhost'.

thanks
thierry



>From f18e2a378f8f32c183fc49782853c8bd3a706c18 Mon Sep 17 00:00:00 2001
From: Ludwig Krispenz <lkris...@redhat.com>
Date: Tue, 30 Jun 2015 11:27:50 +0200
Subject: [PATCH] v2 clear start attr from segment after initialization
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

    Online initialization can be triggered by setting "nsds5BeginReplicaRefresh[;left|;right]": start to a
    segment. But this field remained in the segment and after restart the init would be executed again.
    see Ticket #5065

    To fix this the field is cleared:
    - after a backend comes back online after being initialized
    - since there is a delay and the sending server could be restarted in between,
        the field is also scheced and renḿoved at startup
---
 daemons/ipa-slapi-plugins/topology/topology.h      |   4 +
 daemons/ipa-slapi-plugins/topology/topology_cfg.c  |  12 +++
 daemons/ipa-slapi-plugins/topology/topology_init.c |  13 +++
 daemons/ipa-slapi-plugins/topology/topology_util.c | 100 +++++++++++++++++++++
 4 files changed, 129 insertions(+)

diff --git a/daemons/ipa-slapi-plugins/topology/topology.h b/daemons/ipa-slapi-plugins/topology/topology.h
index 9c680e7cfd636bb783b51ed904c74f34c4e4cf20..953a5e33f2fac379a9621910a86aa8ce5a8c89b2 100644
--- a/daemons/ipa-slapi-plugins/topology/topology.h
+++ b/daemons/ipa-slapi-plugins/topology/topology.h
@@ -123,6 +123,7 @@ typedef struct topo_plugin_config {
     char **managed_attrs;
     char **restricted_attrs;
     int activated;
+    int post_init;
 } TopoPluginConf;
 
 #define CONFIG_ATTR_SHARED_BASE "nsslapd-topo-plugin-shared-config-base"
@@ -138,6 +139,7 @@ void ipa_topo_init_shared_config(void);
 int ipa_topo_init_config(Slapi_PBlock *pb);
 void *ipa_topo_get_plugin_id(void);
 int ipa_topo_get_plugin_active(void);
+int ipa_topo_get_post_init(void);
 char *ipa_topo_get_plugin_shared_config(void);
 Slapi_DN *ipa_topo_get_plugin_shared_topo_dn(void);
 Slapi_DN *ipa_topo_get_plugin_shared_hosts_dn(void);
@@ -158,6 +160,7 @@ int ipa_topo_get_min_domain_level(void);
 int ipa_topo_get_plugin_startup_delay(void);
 void ipa_topo_set_plugin_id(void *plg_id);
 void ipa_topo_set_plugin_active(int state);
+void ipa_topo_set_post_init(int state);
 void ipa_topo_set_plugin_shared_config(char *);
 void ipa_topo_set_plugin_hostname(char *hostname);
 void ipa_topo_set_plugin_replica_root(char **roots);
@@ -296,3 +299,4 @@ char *ipa_topo_util_get_pluginhost(void);
 TopoReplica *ipa_topo_util_get_replica_conf(char *repl_root);
 TopoReplicaSegmentList *ipa_topo_util_get_replica_segments(TopoReplica *replica);
 void ipa_topo_util_set_domain_level(void);
+void ipa_topo_util_reset_init(char *repl_root);
diff --git a/daemons/ipa-slapi-plugins/topology/topology_cfg.c b/daemons/ipa-slapi-plugins/topology/topology_cfg.c
index 19b6947a76d2264367e05a89c9c24487816d078a..9c4b02ba3ab959c47632adc9b71cc275eaa3c6b4 100644
--- a/daemons/ipa-slapi-plugins/topology/topology_cfg.c
+++ b/daemons/ipa-slapi-plugins/topology/topology_cfg.c
@@ -171,6 +171,18 @@ ipa_topo_get_plugin_active(void)
     return topo_plugin_conf.activated;
 }
 
+
+void
+ipa_topo_set_post_init(int state)
+{
+     topo_plugin_conf.post_init = state;
+}
+int
+ipa_topo_get_post_init(void)
+{
+    return topo_plugin_conf.post_init;
+}
+
 void
 ipa_topo_set_plugin_shared_config(char *cfg)
 {
diff --git a/daemons/ipa-slapi-plugins/topology/topology_init.c b/daemons/ipa-slapi-plugins/topology/topology_init.c
index af5b8021f4ba6833dff11d9c89543e9bb74bdeb9..073e4bf422c9ef1bd7cc2c9e4b091dc1c059e954 100644
--- a/daemons/ipa-slapi-plugins/topology/topology_init.c
+++ b/daemons/ipa-slapi-plugins/topology/topology_init.c
@@ -160,6 +160,18 @@ ipa_topo_apply_shared_config(void)
     /* initialize the list of managed servers */
     rc = ipa_topo_setup_managed_servers();
 
+    if (ipa_topo_get_post_init()) {
+        /* this server has just been initialized, we reset the init
+         * flag in the segments which triggered this init
+         */
+        i = 0;
+        while(shared_replica_root[i]) {
+            ipa_topo_util_reset_init(shared_replica_root[i]);
+            i++;
+        }
+        ipa_topo_set_post_init(0);
+    }
+
     ipa_topo_release_startup_inprogress();
     return (rc);
 }
@@ -295,6 +307,7 @@ ipa_topo_be_state_change(void *handle, char *be_name,
         ipa_topo_util_set_domain_level();
         ipa_topo_util_check_plugin_active();
         if (ipa_topo_get_plugin_active()) {
+            ipa_topo_set_post_init(1);
             ipa_topo_util_start(1);
         }
     } else if (new_be_state == SLAPI_BE_STATE_OFFLINE) {
diff --git a/daemons/ipa-slapi-plugins/topology/topology_util.c b/daemons/ipa-slapi-plugins/topology/topology_util.c
index fa249b96aa7b963e87272beb92c53e636263c590..ea9a9c74c3bb755decadb80c82225a0047d7bdeb 100644
--- a/daemons/ipa-slapi-plugins/topology/topology_util.c
+++ b/daemons/ipa-slapi-plugins/topology/topology_util.c
@@ -473,6 +473,54 @@ ipa_topo_util_conf_from_entry(Slapi_Entry *entry)
         return conf;
     }
 }
+void
+ipa_topo_util_segm_modify (TopoReplica *tconf,
+                               TopoReplicaSegment *tsegm,
+                               Slapi_Mods *smods)
+{
+    char *dn = NULL;
+
+    dn = ipa_topo_segment_dn(tconf, tsegm->name);
+
+    if (dn  == NULL) return;
+
+    if (slapi_mods_get_num_mods(smods) > 0) {
+        Slapi_DN *sdn = slapi_sdn_new_normdn_byref(dn);
+        ipa_topo_util_modify(sdn, smods);
+        slapi_sdn_free(&sdn);
+    }
+
+    slapi_ch_free_string(&dn);
+}
+
+void
+ipa_topo_util_remove_init_attr(TopoReplica *repl_conf, TopoReplicaAgmt *topo_agmt)
+{
+    TopoReplicaSegmentList *seglist = repl_conf->repl_segments;
+    TopoReplicaSegment *segment = NULL;
+    char *dirattr = NULL;
+
+    while (seglist) {
+        segment = seglist->segm;
+        if (segment->left == topo_agmt) {
+            dirattr = "nsds5beginreplicarefresh;left";
+            break;
+        } else if (segment->right == topo_agmt) {
+            dirattr = "nsds5beginreplicarefresh;right";
+            break;
+        } else {
+            segment = NULL;
+        }
+        seglist = seglist->next;
+    }
+    if (segment) {
+        Slapi_Mods *smods = slapi_mods_new();
+        slapi_mods_add_string(smods, LDAP_MOD_DELETE,
+                              dirattr, "");
+        ipa_topo_util_segm_modify (repl_conf, segment, smods);
+        slapi_mods_free(&smods);
+    }
+}
 
 void
 ipa_topo_util_set_agmt_rdn(TopoReplicaAgmt *topo_agmt, Slapi_Entry *repl_agmt)
@@ -595,6 +643,14 @@ ipa_topo_util_update_agmt_list(TopoReplica *conf, TopoReplicaSegmentList *repl_s
             for (i=0; mattrs[i]; i++) {
                 segm_attr_val = ipa_topo_util_get_segm_attr(topo_agmt,mattrs[i]);
                 if (segm_attr_val) {
+                    if (0 == strcasecmp(mattrs[i], "nsds5BeginReplicaRefresh")) {
+                        /* we have to remove this attr from the segment, this is
+                         * processed on a server, which is the supplier side of
+                         * an agreement.
+                         */
+                        ipa_topo_util_remove_init_attr(conf, topo_agmt);
+                        continue;
+                    }
                     agmt_attr_val =  slapi_entry_attr_get_charptr(repl_agmt,mattrs[i]);
                     if (agmt_attr_val == NULL ||
                         strcasecmp(agmt_attr_val,segm_attr_val)) {
@@ -1560,5 +1616,49 @@ ipa_topo_util_disable_repl_for_principal(char *repl_root, char *principal)
     slapi_sdn_free(&sdn);
     slapi_log_error(SLAPI_LOG_PLUGIN, IPA_TOPO_PLUGIN_SUBSYSTEM,
                             "<-- ipa_topo_util_disable_repl_for_principal\n");
+}
+
+void
+ipa_topo_util_reset_init(char *repl_root)
+{
+    TopoReplica *replica_config = NULL;
+    TopoReplicaSegmentList *seglist = NULL;
+    TopoReplicaSegment *segment = NULL;
+    char *localhost = ipa_topo_get_plugin_hostname();
+    char *dirattr;
+
+    replica_config = ipa_topo_cfg_replica_find(repl_root, 1);
+    if (replica_config) {
+        seglist = ipa_topo_util_get_replica_segments(replica_config);
+    } else {
+        slapi_log_error(SLAPI_LOG_PLUGIN, IPA_TOPO_PLUGIN_SUBSYSTEM,
+                        "ipa_topo_util_reset_init: no replica found for: %s\n", repl_root);
+        return;
+    }
 
+    while (seglist) {
+        /* this is executed after an online init completed, reset the init in segments
+         * which target this server
+         */
+        segment = seglist->segm;
+        if (segment->left && (0 == strcasecmp(localhost,segment->left->target))
+                && ipa_topo_util_get_segm_attr(segment->left,"nsds5BeginReplicaRefresh")) {
+            dirattr = "nsds5BeginReplicaRefresh;left";
+            break;
+        } else if (segment->right && (0 == strcasecmp(localhost,segment->right->target))
+                && ipa_topo_util_get_segm_attr(segment->right,"nsds5BeginReplicaRefresh")) {
+            dirattr = "nsds5BeginReplicaRefresh;right";
+            break;
+        } else {
+            segment = NULL;
+        }
+        seglist = seglist->next;
+    }
+    if (segment) {
+        Slapi_Mods *smods = slapi_mods_new();
+        slapi_mods_add_string(smods, LDAP_MOD_DELETE,
+                              dirattr, "");
+        ipa_topo_util_segm_modify (replica_config, segment, smods);
+        slapi_mods_free(&smods);
+    }
 }
-- 
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