Re: [Freeipa-devel] [PATCH 0008-0009] use 1 as domain level to activate plugin, fix a crash when removing a replica

2015-06-04 Thread Petr Vobornik 

On 06/02/2015 12:59 PM, Fraser Tweedale wrote:

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?


The code looks good to me.

pushed to master:
* 4e05ffa22c4880e393f4770fe64035fa93cb5fd1 plugin uses 1 as minimum 
domain level to become active no calculation based on plugin version

* f87324df546055df1e7d038e63c04bb0d2250f55 crash when removing a replica



Cheers,
Fraser

--
Petr Vobornik

--
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

[Freeipa-devel] [PATCH 0008-0009] use 1 as domain level to activate plugin, fix a crash when removing a replica

2015-06-02 Thread Ludwig Krispenz 

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
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() ||
-

Re: [Freeipa-devel] [PATCH 0008-0009] use 1 as domain level to activate plugin, fix a crash when removing a replica

2015-06-02 Thread Ludwig Krispenz 


On 06/02/2015 10:12 AM, Oleg Fayans wrote:

Hi Ludwig,

Are you talking about this crash?
I'm talking about a crash in DS which MBasti reported for 
ipa-replia-manage del replica


2015-06-02T08:06:57Z DEBUG stderr=
2015-06-02T08:06:57Z DEBUG   File 
/usr/lib/python2.7/site-packages/ipaserver/install/installutils.py, 
line 733, in run_script

return_value = main_function()

  File /sbin/ipa-server-install, line 375, in main
server.uninstall(options)

  File 
/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py, line 
279, in decorated

destroy_private_ccache()

  File 
/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py, line 
267, in destroy_private_ccache

if os.path.exists(path):

  File /usr/lib64/python2.7/genericpath.py, line 18, in exists
os.stat(path)

2015-06-02T08:06:57Z DEBUG The ipa-server-install command failed, 
exception: TypeError: coercing to Unicode: need string or buffer, 
NoneType found




On 06/02/2015 10:04 AM, 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




--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.




-- 
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

Re: [Freeipa-devel] [PATCH 0008-0009] use 1 as domain level to activate plugin, fix a crash when removing a replica

2015-06-02 Thread Oleg Fayans 

Hi Ludwig,

Are you talking about this crash?

2015-06-02T08:06:57Z DEBUG stderr=
2015-06-02T08:06:57Z DEBUG   File 
/usr/lib/python2.7/site-packages/ipaserver/install/installutils.py, 
line 733, in run_script

return_value = main_function()

  File /sbin/ipa-server-install, line 375, in main
server.uninstall(options)

  File 
/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py, 
line 279, in decorated

destroy_private_ccache()

  File 
/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py, 
line 267, in destroy_private_ccache

if os.path.exists(path):

  File /usr/lib64/python2.7/genericpath.py, line 18, in exists
os.stat(path)

2015-06-02T08:06:57Z DEBUG The ipa-server-install command failed, 
exception: TypeError: coercing to Unicode: need string or buffer, 
NoneType found




On 06/02/2015 10:04 AM, 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




--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.

-- 
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

Re: [Freeipa-devel] [PATCH 0008-0009] use 1 as domain level to activate plugin, fix a crash when removing a replica

2015-06-02 Thread Fraser Tweedale 
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