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 <lkris...@redhat.com>
> 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 <lkris...@redhat.com>
> 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

Reply via email to