On Tue, Jun 02, 2015 at 10:04:55AM +0200, Ludwig Krispenz wrote: > Hi, > > with the first patch the topo plugin no longer uses plugin version to > compare to set domainlevel, always gets activated if dom level >= 1 > the second patch fixes a crash at replica removal > > Ludwig
These patches fix the issue for me. I don't know what is (supposed to be) happening in the code. Is my testing enough for the ACK? Cheers, Fraser > >From 7e08b6181973cc51e50eae69974682878b8ca66b Mon Sep 17 00:00:00 2001 > From: Ludwig Krispenz <[email protected]> > Date: Tue, 2 Jun 2015 09:17:54 +0200 > Subject: [PATCH] plugin uses 1 as minimum domain level to become active no > calculation based on plugin version > > --- > daemons/ipa-slapi-plugins/topology/topology.h | 9 ++------ > daemons/ipa-slapi-plugins/topology/topology_cfg.c | 25 > +++++++--------------- > daemons/ipa-slapi-plugins/topology/topology_init.c | 2 +- > daemons/ipa-slapi-plugins/topology/topology_util.c | 4 +--- > 4 files changed, 12 insertions(+), 28 deletions(-) > > diff --git a/daemons/ipa-slapi-plugins/topology/topology.h > b/daemons/ipa-slapi-plugins/topology/topology.h > index > 38c2823f50153c6b02a234608869617c92d1bdf2..4135a8ff71b9160919a089fde63a95a989830de8 > 100644 > --- a/daemons/ipa-slapi-plugins/topology/topology.h > +++ b/daemons/ipa-slapi-plugins/topology/topology.h > @@ -125,11 +125,6 @@ typedef struct topo_plugin_config { > int activated; > } TopoPluginConf; > > -typedef struct ipa_domain_level { > - int major; > - int minor; > -} IpaDomainLevel; > - > #define CONFIG_ATTR_SHARED_BASE "nsslapd-topo-plugin-shared-config-base" > #define CONFIG_ATTR_REPLICA_ROOT "nsslapd-topo-plugin-shared-replica-root" > #define CONFIG_ATTR_SHARED_BINDDNGROUP > "nsslapd-topo-plugin-shared-binddngroup" > @@ -158,8 +153,8 @@ int ipa_topo_get_plugin_version_major(void); > int ipa_topo_get_plugin_version_minor(void); > char *ipa_topo_get_domain_level_entry(void); > Slapi_DN *ipa_topo_get_domain_level_entry_dn(void); > -int ipa_topo_get_domain_level_major(void); > -int ipa_topo_get_domain_level_minor(void); > +int ipa_topo_get_domain_level(void); > +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); > diff --git a/daemons/ipa-slapi-plugins/topology/topology_cfg.c > b/daemons/ipa-slapi-plugins/topology/topology_cfg.c > index > 17493495af83d1095fbafead104d6f56bd7af10e..982ad647db9737c1aa0fc7f68c7d9b20de895fb6 > 100644 > --- a/daemons/ipa-slapi-plugins/topology/topology_cfg.c > +++ b/daemons/ipa-slapi-plugins/topology/topology_cfg.c > @@ -10,7 +10,8 @@ > */ > static TopoPluginConf topo_plugin_conf = {0}; > static TopoReplicaConf topo_shared_conf = {0}; > -static IpaDomainLevel ipa_domain_level = {0,0}; > +static int ipa_domain_level = 0; > +static int topo_min_domain_level = 1; > > char *ipa_topo_plugin_managed_attrs[] = { > "nsds5ReplicaStripAttrs", > @@ -95,15 +96,15 @@ ipa_topo_get_domain_level_entry_dn(void) > } > > int > -ipa_topo_get_domain_level_major(void) > +ipa_topo_get_min_domain_level(void) > { > - return ipa_domain_level.major; > + return topo_min_domain_level; > } > > int > -ipa_topo_get_domain_level_minor(void) > +ipa_topo_get_domain_level(void) > { > - return ipa_domain_level.minor; > + return ipa_domain_level; > } > > char * > @@ -199,22 +200,12 @@ ipa_topo_set_plugin_shared_bindgroup(char *bindgroup) > void > ipa_topo_set_domain_level(char *level) > { > - char *minor; > - > if (level == NULL) { > - ipa_domain_level.major = 0; > - ipa_domain_level.minor = 0; > + ipa_domain_level = 0; > return; > } > > - minor = strchr(level,'.'); > - if (minor) { > - *minor = '\0'; > - ipa_domain_level.minor = atoi(++minor); > - } else { > - ipa_domain_level.minor = 0; > - } > - ipa_domain_level.major = atoi(level); > + ipa_domain_level = atoi(level); > } > > void > diff --git a/daemons/ipa-slapi-plugins/topology/topology_init.c > b/daemons/ipa-slapi-plugins/topology/topology_init.c > index > 77e740ea182c2331c88d2716d1c4f7be8ef8c257..af5b8021f4ba6833dff11d9c89543e9bb74bdeb9 > 100644 > --- a/daemons/ipa-slapi-plugins/topology/topology_init.c > +++ b/daemons/ipa-slapi-plugins/topology/topology_init.c > @@ -264,7 +264,7 @@ ipa_topo_rootdse_search(Slapi_PBlock *pb, Slapi_Entry* e, > Slapi_Entry* entryAfte > /* we expose temporarily the domain level in this function, should > * finally be handled in a plugin managing the domain level > */ > - char *level = slapi_ch_smprintf("%d", ipa_topo_get_domain_level_major()); > + char *level = slapi_ch_smprintf("%d", ipa_topo_get_domain_level()); > slapi_entry_attr_set_charptr(e, "ipaDomainLevel", level); > slapi_ch_free_string(&version); > slapi_ch_free_string(&level); > diff --git a/daemons/ipa-slapi-plugins/topology/topology_util.c > b/daemons/ipa-slapi-plugins/topology/topology_util.c > index > f206464a5b47b9dc7e0edd5dd764228b076b6dd9..d487cfb638ac9bd0fb94cdd2638d5fd5ae4e6908 > 100644 > --- a/daemons/ipa-slapi-plugins/topology/topology_util.c > +++ b/daemons/ipa-slapi-plugins/topology/topology_util.c > @@ -110,9 +110,7 @@ ipa_topo_util_get_pluginhost(void) > void > ipa_topo_util_check_plugin_active(void) > { > - if (ipa_topo_get_plugin_version_major() < > ipa_topo_get_domain_level_major() || > - (ipa_topo_get_plugin_version_major() == > ipa_topo_get_domain_level_major() && > - ipa_topo_get_plugin_version_minor() <= > ipa_topo_get_domain_level_minor())) { > + if (ipa_topo_get_min_domain_level() <= ipa_topo_get_domain_level()) { > ipa_topo_set_plugin_active(1); > } else { > ipa_topo_set_plugin_active(0); > -- > 2.1.0 > > >From 2f27a90394da56925694d771592c9fe3ae40eeeb Mon Sep 17 00:00:00 2001 > From: Ludwig Krispenz <[email protected]> > Date: Tue, 2 Jun 2015 09:29:23 +0200 > Subject: [PATCH] crash when removing a replica > > when a server is removed from the topology the plugin tries to remove the > credentials from the replica and the bind dn group. > It performs an internal search for the ldap principal, but can fail if it was > already removed > Due to an unitialized variable in this case it can eitehr crash or > erroneously remove all > principals. > --- > daemons/ipa-slapi-plugins/topology/topology_util.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/daemons/ipa-slapi-plugins/topology/topology_util.c > b/daemons/ipa-slapi-plugins/topology/topology_util.c > index > d487cfb638ac9bd0fb94cdd2638d5fd5ae4e6908..67014a05d4f89260d4307e5212a5594335617482 > 100644 > --- a/daemons/ipa-slapi-plugins/topology/topology_util.c > +++ b/daemons/ipa-slapi-plugins/topology/topology_util.c > @@ -1201,7 +1201,15 @@ void > ipa_topo_util_disable_repl_from_host(char *repl_root, char *delhost) > { > char *principal = ipa_topo_util_get_ldap_principal(repl_root, delhost); > - ipa_topo_util_disable_repl_for_principal(repl_root, principal); > + if (principal) { > + ipa_topo_util_disable_repl_for_principal(repl_root, principal); > + slapi_ch_free_string(&principal); > + } else { > + slapi_log_error(SLAPI_LOG_PLUGIN, IPA_TOPO_PLUGIN_SUBSYSTEM, > + "ipa_topo_util_disable_repl_from_host: " > + "failed to get ldap principal for host: %s \n", > + delhost); > + } > } > > void > @@ -1322,10 +1330,10 @@ char * > ipa_topo_util_get_ldap_principal(char *repl_root, char *hostname) > { > int rc = 0; > - Slapi_Entry **entries; > + Slapi_Entry **entries = NULL; > Slapi_PBlock *pb = NULL; > char *filter; > - char *dn; > + char *dn = NULL; > > filter = slapi_ch_smprintf("krbprincipalname=ldap/%s*",hostname); > pb = slapi_pblock_new(); > -- > 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 -- 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
