[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-09-14 Thread Lukas Slebodnik 
On (13/09/16 17:57), Petr Cech wrote:
>On 09/13/2016 04:27 PM, Lukas Slebodnik wrote:
>> On (13/09/16 16:24), Lukas Slebodnik wrote:
>> > On (13/09/16 14:11), Fabiano Fidêncio wrote:
>> > > On Mon, Sep 12, 2016 at 9:40 AM, Petr Cech  wrote:
>> > > > Bump.
>> > > > 
>> > > > 
>> > > > --
>> > > > Petr^4 Čech
>> > > > ___
>> > > > sssd-devel mailing list
>> > > > sssd-devel@lists.fedorahosted.org
>> > > > https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
>> > > 
>> > > Patch looks good and all the requested changed were done.
>> > > I haven't done any tests with the patch, but the changes themselves
>> > > look good to me.
>> > > 
>> > master:
>> > * aef0171e0bdc9a683958d69c7ee984fb10cd5de7
>> > 
>> > http://sssd-ci.duckdns.org/logs/job/53/30/summary.html
>> > 
>> Could you also prepare patch for 1.13 branch?
>
>Yes, see attachment, please.
>
sssd-1-13:
* 90c62a1b4bac450712bc5a194b793761329a1d3a

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-09-13 Thread Petr Cech 



On 09/13/2016 04:27 PM, Lukas Slebodnik wrote:

On (13/09/16 16:24), Lukas Slebodnik wrote:

On (13/09/16 14:11), Fabiano Fidêncio wrote:

On Mon, Sep 12, 2016 at 9:40 AM, Petr Cech  wrote:

Bump.


--
Petr^4 Čech
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org


Patch looks good and all the requested changed were done.
I haven't done any tests with the patch, but the changes themselves
look good to me.


master:
* aef0171e0bdc9a683958d69c7ee984fb10cd5de7

http://sssd-ci.duckdns.org/logs/job/53/30/summary.html


Could you also prepare patch for 1.13 branch?


Yes, see attachment, please.

Regards

--
Petr^4 Čech
>From 476ec80536205bb538c329252a6a162009210253 Mon Sep 17 00:00:00 2001
From: Petr Cech 
Date: Wed, 24 Aug 2016 14:41:09 +0200
Subject: [PATCH] PROXY: Adding proxy_max_children option

The new option 'proxy_max_children' is applicable
in domain section. Default value is 10.

Resolves:
https://fedorahosted.org/sssd/ticket/3153
---
 src/confdb/confdb.h   |  1 +
 src/config/SSSDConfig/__init__.py.in  |  3 +++
 src/config/etc/sssd.api.d/sssd-proxy.conf |  1 +
 src/man/sssd.conf.5.xml   | 17 +
 src/providers/proxy/proxy_init.c  | 21 +++--
 5 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index 6d8601b31cf4ce1a42f824a8400cef8c4ffadf9a..3161fc2181f9af641c3019adbdb67bcb417efdd8 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -215,6 +215,7 @@
 #define CONFDB_PROXY_LIBNAME "proxy_lib_name"
 #define CONFDB_PROXY_PAM_TARGET "proxy_pam_target"
 #define CONFDB_PROXY_FAST_ALIAS "proxy_fast_alias"
+#define CONFDB_PROXY_MAX_CHILDREN "proxy_max_children"
 
 struct confdb_ctx;
 struct config_file_ctx;
diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
index a400c831eb0e44f562c010f2a3649def21913287..56ede34c4b4bf8002f0fe4ac8212ed8523726092 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -419,6 +419,9 @@ option_strings = {
 'default_shell' : _('Default shell, /bin/bash'),
 'base_directory' : _('Base for home directories'),
 
+# [provider/proxy]
+'proxy_max_children' : _('The number of preforked proxy children.'),
+
 # [provider/proxy/id]
 'proxy_lib_name' : _('The name of the NSS library to use'),
 'proxy_fast_alias' : _('Whether to look up canonical group name from cache if possible'),
diff --git a/src/config/etc/sssd.api.d/sssd-proxy.conf b/src/config/etc/sssd.api.d/sssd-proxy.conf
index 89a6503f9b84b7eab5fb3b0dd591dea905b43adb..09bf82affcb4263de3abbb67d1d484f6b01a1824 100644
--- a/src/config/etc/sssd.api.d/sssd-proxy.conf
+++ b/src/config/etc/sssd.api.d/sssd-proxy.conf
@@ -1,4 +1,5 @@
 [provider/proxy]
+proxy_max_children = int, None, false
 
 [provider/proxy/id]
 proxy_lib_name = str, None, true
diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index 4f138e21940b1f13d864dd7c461dd981093ed2db..a76b19f447d4e1a441b64f2a4b3b99941b8bf9cd 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -2325,6 +2325,23 @@ pam_account_locked_message = Account locked, please contact help desk.
 
 
 
+
+
+proxy_max_children (integer)
+
+
+This option specifies the number of pre-forked
+proxy children. It is useful for high-load SSSD
+environments where sssd may run out of available
+child slots, which would cause some issues due to
+the requests being queued.
+
+
+Default: 10
+
+
+
+
 
 
 
diff --git a/src/providers/proxy/proxy_init.c b/src/providers/proxy/proxy_init.c
index 0a6b11d4a3f102782322c913d280b26fe47aecab..275a92c47e1009f36b2db51323a7d06858e31692 100644
--- a/src/providers/proxy/proxy_init.c
+++ b/src/providers/proxy/proxy_init.c
@@ -27,6 +27,8 @@
 #include "util/sss_format.h"
 #include "providers/proxy/proxy.h"
 
+#define OPT_MAX_CHILDREN_DEFAULT 10
+
 static int client_registration(struct sbus_request *dbus_req, void *data);
 
 static struct data_provider_iface proxy_methods = {
@@ -478,6 +480,7 @@ int sssm_proxy_auth_init(struct be_ctx *bectx,
 int ret;
 int hret;
 char *sbus_address;
+int max_children;
 
 /* If we're already set up, just return that */
 if(bectx->bet_info[BET_AUTH].mod_name &&
@@ -523,8 +526,22 @@ int sssm_proxy_auth_init(struct be_ctx *bectx,
 }
 
 /* Set up request hash table */
-

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-09-13 Thread Lukas Slebodnik 
On (13/09/16 16:24), Lukas Slebodnik wrote:
>On (13/09/16 14:11), Fabiano Fidêncio wrote:
>>On Mon, Sep 12, 2016 at 9:40 AM, Petr Cech  wrote:
>>> Bump.
>>>
>>>
>>> --
>>> Petr^4 Čech
>>> ___
>>> sssd-devel mailing list
>>> sssd-devel@lists.fedorahosted.org
>>> https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
>>
>>Patch looks good and all the requested changed were done.
>>I haven't done any tests with the patch, but the changes themselves
>>look good to me.
>>
>master:
>* aef0171e0bdc9a683958d69c7ee984fb10cd5de7
>
>http://sssd-ci.duckdns.org/logs/job/53/30/summary.html
>
Could you also prepare patch for 1.13 branch?

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-09-13 Thread Lukas Slebodnik 
On (13/09/16 14:11), Fabiano Fidêncio wrote:
>On Mon, Sep 12, 2016 at 9:40 AM, Petr Cech  wrote:
>> Bump.
>>
>>
>> --
>> Petr^4 Čech
>> ___
>> sssd-devel mailing list
>> sssd-devel@lists.fedorahosted.org
>> https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
>
>Patch looks good and all the requested changed were done.
>I haven't done any tests with the patch, but the changes themselves
>look good to me.
>
master:
* aef0171e0bdc9a683958d69c7ee984fb10cd5de7

http://sssd-ci.duckdns.org/logs/job/53/30/summary.html

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-09-13 Thread Fabiano Fidêncio 
On Mon, Sep 12, 2016 at 9:40 AM, Petr Cech  wrote:
> Bump.
>
>
> --
> Petr^4 Čech
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

Patch looks good and all the requested changed were done.
I haven't done any tests with the patch, but the changes themselves
look good to me.

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-09-12 Thread Petr Cech 

Bump.

--
Petr^4 Čech
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-09-06 Thread Petr Cech 

On 09/06/2016 05:11 PM, Justin Stephenson wrote:

On 09/06/2016 10:57 AM, Petr Cech wrote:

On 09/06/2016 04:17 PM, Justin Stephenson wrote:



On 09/05/2016 10:20 AM, Petr Cech wrote:

On 09/05/2016 04:05 PM, Fabiano Fidêncio wrote:

Petr,

On Mon, Sep 5, 2016 at 3:43 PM, Petr Cech  wrote:

On 09/05/2016 09:57 AM, Fabiano Fidêncio wrote:


Petr,

I see you have updated the OPT_MAX_CHILDREN_DEFAULT to 10 instead of
50. However, you haven't update the value on sssd.conf.5.xml:

diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index
ae291e0fc8f2f9afabcdf32f18a5ec12252f..1bf3e799047d9c722487be8657bbee5cfd479cdd




100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -2464,6 +2464,18 @@ subdomain_inherit = ldap_purge_cache_timeout
 
 

+
+proxy_max_children (integer)
+
+
+The number of preforked proxy children.
+
+
+Default: 50
   here: ^^^

+
+
+
+
 
 

Apart from this minor the patch seems to be following everything
that
was requested during the review process. However, I'm not
comfortable
with the text used to describe the new option, so adding there a bit
more information would be super. Like, I don't know what's the
influence of the preforked proxy children to the rest of the code
(probably because I'm a newbie here ;-)), but would be nice to
have it
clear in the documentation (for newbies like myself ;-)).

Best Regards,
--
Fabiano Fidêncio



Hi Fabiano,

thanks for code review. I fixed the default value in man page and I
reformulated description. Is it better?

Regards

--
Petr^4 Čech

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org







Looking at the changes in the manual ...

+In busy environments it is possible sssd
runs out of
+available child slots and starts queuing
requests
+in proxy mode. This option introduces the
number of
+preforked proxy children.

I personally go for something like:

"This option introduces the number of pre-forked proxy children. It's
useful for busy environments* where sssd may run out of available
child slots, which would cause some issues due to the requests being
queued".

*: Not sure whether busy environments is something clear for everyone
...

IMO the patch is good to go as soon as we have this part done/reviewed
by a native speaker.
Maybe Justin can help us here?

Best Regards,
--
Fabiano Fidêncio


Fabiano,

I took your suggestion, thanks. I don't know right term for 'busy
environments'. I will be glad if native speaker help me with the right
formulation of description.


Something like "it is useful for high-load SSSD environments" or "it is
useful for larger environments" may work - whichever you prefer.

Kind regards,
Justin Stephenson


Hello,

new version attached. Description is:

This option specifies the number of pre-forked
proxy children. It is useful for high-load SSSD
environments where sssd may run out of available
child slots, which would cause some issues due to
the requests being queued.

After hint from Michal I replaced introduces by specifies.

Is this version linguistic right?


This description looks good to me.

-Justin


Thanks, Justin.

--
Petr^4 Čech
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-09-06 Thread Justin Stephenson 

On 09/06/2016 10:57 AM, Petr Cech wrote:

On 09/06/2016 04:17 PM, Justin Stephenson wrote:



On 09/05/2016 10:20 AM, Petr Cech wrote:

On 09/05/2016 04:05 PM, Fabiano Fidêncio wrote:

Petr,

On Mon, Sep 5, 2016 at 3:43 PM, Petr Cech  wrote:

On 09/05/2016 09:57 AM, Fabiano Fidêncio wrote:


Petr,

I see you have updated the OPT_MAX_CHILDREN_DEFAULT to 10 instead of
50. However, you haven't update the value on sssd.conf.5.xml:

diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index
ae291e0fc8f2f9afabcdf32f18a5ec12252f..1bf3e799047d9c722487be8657bbee5cfd479cdd



100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -2464,6 +2464,18 @@ subdomain_inherit = ldap_purge_cache_timeout
 
 

+
+proxy_max_children (integer)
+
+
+The number of preforked proxy children.
+
+
+Default: 50
   here: ^^^

+
+
+
+
 
 

Apart from this minor the patch seems to be following everything that
was requested during the review process. However, I'm not comfortable
with the text used to describe the new option, so adding there a bit
more information would be super. Like, I don't know what's the
influence of the preforked proxy children to the rest of the code
(probably because I'm a newbie here ;-)), but would be nice to
have it
clear in the documentation (for newbies like myself ;-)).

Best Regards,
--
Fabiano Fidêncio



Hi Fabiano,

thanks for code review. I fixed the default value in man page and I
reformulated description. Is it better?

Regards

--
Petr^4 Čech

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org






Looking at the changes in the manual ...

+In busy environments it is possible sssd
runs out of
+available child slots and starts queuing
requests
+in proxy mode. This option introduces the
number of
+preforked proxy children.

I personally go for something like:

"This option introduces the number of pre-forked proxy children. It's
useful for busy environments* where sssd may run out of available
child slots, which would cause some issues due to the requests being
queued".

*: Not sure whether busy environments is something clear for everyone
...

IMO the patch is good to go as soon as we have this part done/reviewed
by a native speaker.
Maybe Justin can help us here?

Best Regards,
--
Fabiano Fidêncio


Fabiano,

I took your suggestion, thanks. I don't know right term for 'busy
environments'. I will be glad if native speaker help me with the right
formulation of description.


Something like "it is useful for high-load SSSD environments" or "it is
useful for larger environments" may work - whichever you prefer.

Kind regards,
Justin Stephenson


Hello,

new version attached. Description is:

This option specifies the number of pre-forked
proxy children. It is useful for high-load SSSD
environments where sssd may run out of available
child slots, which would cause some issues due to
the requests being queued.

After hint from Michal I replaced introduces by specifies.

Is this version linguistic right?


This description looks good to me.

-Justin



Regards



___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org


___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-09-06 Thread Justin Stephenson 



On 09/05/2016 10:20 AM, Petr Cech wrote:

On 09/05/2016 04:05 PM, Fabiano Fidêncio wrote:

Petr,

On Mon, Sep 5, 2016 at 3:43 PM, Petr Cech  wrote:

On 09/05/2016 09:57 AM, Fabiano Fidêncio wrote:


Petr,

I see you have updated the OPT_MAX_CHILDREN_DEFAULT to 10 instead of
50. However, you haven't update the value on sssd.conf.5.xml:

diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index
ae291e0fc8f2f9afabcdf32f18a5ec12252f..1bf3e799047d9c722487be8657bbee5cfd479cdd

100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -2464,6 +2464,18 @@ subdomain_inherit = ldap_purge_cache_timeout
 
 

+
+proxy_max_children (integer)
+
+
+The number of preforked proxy children.
+
+
+Default: 50
   here: ^^^

+
+
+
+
 
 

Apart from this minor the patch seems to be following everything that
was requested during the review process. However, I'm not comfortable
with the text used to describe the new option, so adding there a bit
more information would be super. Like, I don't know what's the
influence of the preforked proxy children to the rest of the code
(probably because I'm a newbie here ;-)), but would be nice to have it
clear in the documentation (for newbies like myself ;-)).

Best Regards,
--
Fabiano Fidêncio



Hi Fabiano,

thanks for code review. I fixed the default value in man page and I
reformulated description. Is it better?

Regards

--
Petr^4 Čech

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org




Looking at the changes in the manual ...

+In busy environments it is possible sssd
runs out of
+available child slots and starts queuing
requests
+in proxy mode. This option introduces the
number of
+preforked proxy children.

I personally go for something like:

"This option introduces the number of pre-forked proxy children. It's
useful for busy environments* where sssd may run out of available
child slots, which would cause some issues due to the requests being
queued".

*: Not sure whether busy environments is something clear for everyone ...

IMO the patch is good to go as soon as we have this part done/reviewed
by a native speaker.
Maybe Justin can help us here?

Best Regards,
--
Fabiano Fidêncio


Fabiano,

I took your suggestion, thanks. I don't know right term for 'busy
environments'. I will be glad if native speaker help me with the right
formulation of description.


Something like "it is useful for high-load SSSD environments" or "it is 
useful for larger environments" may work - whichever you prefer.


Kind regards,
Justin Stephenson


Regards



___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org


___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-09-05 Thread Petr Cech 

On 09/05/2016 04:05 PM, Fabiano Fidêncio wrote:

Petr,

On Mon, Sep 5, 2016 at 3:43 PM, Petr Cech  wrote:

On 09/05/2016 09:57 AM, Fabiano Fidêncio wrote:


Petr,

I see you have updated the OPT_MAX_CHILDREN_DEFAULT to 10 instead of
50. However, you haven't update the value on sssd.conf.5.xml:

diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index
ae291e0fc8f2f9afabcdf32f18a5ec12252f..1bf3e799047d9c722487be8657bbee5cfd479cdd
100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -2464,6 +2464,18 @@ subdomain_inherit = ldap_purge_cache_timeout
 
 

+
+proxy_max_children (integer)
+
+
+The number of preforked proxy children.
+
+
+Default: 50
   here: ^^^

+
+
+
+
 
 

Apart from this minor the patch seems to be following everything that
was requested during the review process. However, I'm not comfortable
with the text used to describe the new option, so adding there a bit
more information would be super. Like, I don't know what's the
influence of the preforked proxy children to the rest of the code
(probably because I'm a newbie here ;-)), but would be nice to have it
clear in the documentation (for newbies like myself ;-)).

Best Regards,
--
Fabiano Fidêncio



Hi Fabiano,

thanks for code review. I fixed the default value in man page and I
reformulated description. Is it better?

Regards

--
Petr^4 Čech

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org



Looking at the changes in the manual ...

+In busy environments it is possible sssd
runs out of
+available child slots and starts queuing requests
+in proxy mode. This option introduces the number of
+preforked proxy children.

I personally go for something like:

"This option introduces the number of pre-forked proxy children. It's
useful for busy environments* where sssd may run out of available
child slots, which would cause some issues due to the requests being
queued".

*: Not sure whether busy environments is something clear for everyone ...

IMO the patch is good to go as soon as we have this part done/reviewed
by a native speaker.
Maybe Justin can help us here?

Best Regards,
--
Fabiano Fidêncio


Fabiano,

I took your suggestion, thanks. I don't know right term for 'busy 
environments'. I will be glad if native speaker help me with the right 
formulation of description.


Regards

--
Petr^4 Čech
>From 0f700afa2a18c6afae876fce12dc7e83ba22f605 Mon Sep 17 00:00:00 2001
From: Petr Cech 
Date: Wed, 24 Aug 2016 14:41:09 +0200
Subject: [PATCH] PROXY: Adding proxy_max_children option

The new option 'proxy_max_children' is applicable
in domain section. Default value is 10.

Resolves:
https://fedorahosted.org/sssd/ticket/3153
---
 src/confdb/confdb.h   |  1 +
 src/config/SSSDConfig/__init__.py.in  |  3 +++
 src/config/cfg_rules.ini  |  1 +
 src/config/etc/sssd.api.d/sssd-proxy.conf |  1 +
 src/man/sssd.conf.5.xml   | 16 
 src/providers/proxy/proxy_init.c  | 21 +++--
 6 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index 401e5fbf7ed6bb9e8d7158dfab378c8159aa03db..9b5c7bc04bb8297842aa9a0ef50f239c50302757 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -218,6 +218,7 @@
 #define CONFDB_PROXY_LIBNAME "proxy_lib_name"
 #define CONFDB_PROXY_PAM_TARGET "proxy_pam_target"
 #define CONFDB_PROXY_FAST_ALIAS "proxy_fast_alias"
+#define CONFDB_PROXY_MAX_CHILDREN "proxy_max_children"
 
 /* Secrets Service */
 #define CONFDB_SEC_CONF_ENTRY "config/secrets"
diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
index 0191920f93ab9016508e08785c25dd043c180c0b..8ba006fdfe710fbfba82b40fe9b20461813ef3c7 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -428,6 +428,9 @@ option_strings = {
 'default_shell' : _('Default shell, /bin/bash'),
 'base_directory' : _('Base for home directories'),
 
+# [provider/proxy]
+'proxy_max_children' : _('The number of preforked proxy children.'),
+
 # [provider/proxy/id]
 'proxy_lib_name' : _('The name of the NSS library to use'),
 'proxy_fast_alias' : _('Whether to look up canonical group name from cache if possible'),
diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-09-05 Thread Fabiano Fidêncio 
Petr,

On Mon, Sep 5, 2016 at 3:43 PM, Petr Cech  wrote:
> On 09/05/2016 09:57 AM, Fabiano Fidêncio wrote:
>>
>> Petr,
>>
>> I see you have updated the OPT_MAX_CHILDREN_DEFAULT to 10 instead of
>> 50. However, you haven't update the value on sssd.conf.5.xml:
>>
>> diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
>> index
>> ae291e0fc8f2f9afabcdf32f18a5ec12252f..1bf3e799047d9c722487be8657bbee5cfd479cdd
>> 100644
>> --- a/src/man/sssd.conf.5.xml
>> +++ b/src/man/sssd.conf.5.xml
>> @@ -2464,6 +2464,18 @@ subdomain_inherit = ldap_purge_cache_timeout
>>  
>>  
>>
>> +
>> +proxy_max_children (integer)
>> +
>> +
>> +The number of preforked proxy children.
>> +
>> +
>> +Default: 50
>>here: ^^^
>>
>> +
>> +
>> +
>> +
>>  
>>  
>>
>> Apart from this minor the patch seems to be following everything that
>> was requested during the review process. However, I'm not comfortable
>> with the text used to describe the new option, so adding there a bit
>> more information would be super. Like, I don't know what's the
>> influence of the preforked proxy children to the rest of the code
>> (probably because I'm a newbie here ;-)), but would be nice to have it
>> clear in the documentation (for newbies like myself ;-)).
>>
>> Best Regards,
>> --
>> Fabiano Fidêncio
>
>
> Hi Fabiano,
>
> thanks for code review. I fixed the default value in man page and I
> reformulated description. Is it better?
>
> Regards
>
> --
> Petr^4 Čech
>
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
>

Looking at the changes in the manual ...

+In busy environments it is possible sssd
runs out of
+available child slots and starts queuing requests
+in proxy mode. This option introduces the number of
+preforked proxy children.

I personally go for something like:

"This option introduces the number of pre-forked proxy children. It's
useful for busy environments* where sssd may run out of available
child slots, which would cause some issues due to the requests being
queued".

*: Not sure whether busy environments is something clear for everyone ...

IMO the patch is good to go as soon as we have this part done/reviewed
by a native speaker.
Maybe Justin can help us here?

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-09-05 Thread Petr Cech 

On 09/05/2016 09:57 AM, Fabiano Fidêncio wrote:

Petr,

I see you have updated the OPT_MAX_CHILDREN_DEFAULT to 10 instead of
50. However, you haven't update the value on sssd.conf.5.xml:

diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index 
ae291e0fc8f2f9afabcdf32f18a5ec12252f..1bf3e799047d9c722487be8657bbee5cfd479cdd
100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -2464,6 +2464,18 @@ subdomain_inherit = ldap_purge_cache_timeout
 
 

+
+proxy_max_children (integer)
+
+
+The number of preforked proxy children.
+
+
+Default: 50
   here: ^^^

+
+
+
+
 
 

Apart from this minor the patch seems to be following everything that
was requested during the review process. However, I'm not comfortable
with the text used to describe the new option, so adding there a bit
more information would be super. Like, I don't know what's the
influence of the preforked proxy children to the rest of the code
(probably because I'm a newbie here ;-)), but would be nice to have it
clear in the documentation (for newbies like myself ;-)).

Best Regards,
--
Fabiano Fidêncio


Hi Fabiano,

thanks for code review. I fixed the default value in man page and I 
reformulated description. Is it better?


Regards

--
Petr^4 Čech
>From 4645b8a9f2c3b98fe92343135aa09e70b8a019d3 Mon Sep 17 00:00:00 2001
From: Petr Cech 
Date: Wed, 24 Aug 2016 14:41:09 +0200
Subject: [PATCH] PROXY: Adding proxy_max_children option

The new option 'proxy_max_children' is applicable
in domain section. Default value is 10.

Resolves:
https://fedorahosted.org/sssd/ticket/3153
---
 src/confdb/confdb.h   |  1 +
 src/config/SSSDConfig/__init__.py.in  |  3 +++
 src/config/cfg_rules.ini  |  1 +
 src/config/etc/sssd.api.d/sssd-proxy.conf |  1 +
 src/man/sssd.conf.5.xml   | 15 +++
 src/providers/proxy/proxy_init.c  | 21 +++--
 6 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index 401e5fbf7ed6bb9e8d7158dfab378c8159aa03db..9b5c7bc04bb8297842aa9a0ef50f239c50302757 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -218,6 +218,7 @@
 #define CONFDB_PROXY_LIBNAME "proxy_lib_name"
 #define CONFDB_PROXY_PAM_TARGET "proxy_pam_target"
 #define CONFDB_PROXY_FAST_ALIAS "proxy_fast_alias"
+#define CONFDB_PROXY_MAX_CHILDREN "proxy_max_children"
 
 /* Secrets Service */
 #define CONFDB_SEC_CONF_ENTRY "config/secrets"
diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
index 0191920f93ab9016508e08785c25dd043c180c0b..8ba006fdfe710fbfba82b40fe9b20461813ef3c7 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -428,6 +428,9 @@ option_strings = {
 'default_shell' : _('Default shell, /bin/bash'),
 'base_directory' : _('Base for home directories'),
 
+# [provider/proxy]
+'proxy_max_children' : _('The number of preforked proxy children.'),
+
 # [provider/proxy/id]
 'proxy_lib_name' : _('The name of the NSS library to use'),
 'proxy_fast_alias' : _('Whether to look up canonical group name from cache if possible'),
diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index 5e248066bd554d2a654a764f406f6b33c4d66733..5213ce4c7e623899edd305c43137a1dbdd7aac7e 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -304,6 +304,7 @@ option = base_directory
 option = proxy_lib_name
 option = proxy_fast_alias
 option = proxy_pam_target
+option = proxy_max_children
 
 # simple access provider specific options
 option = simple_allow_users
diff --git a/src/config/etc/sssd.api.d/sssd-proxy.conf b/src/config/etc/sssd.api.d/sssd-proxy.conf
index 89a6503f9b84b7eab5fb3b0dd591dea905b43adb..09bf82affcb4263de3abbb67d1d484f6b01a1824 100644
--- a/src/config/etc/sssd.api.d/sssd-proxy.conf
+++ b/src/config/etc/sssd.api.d/sssd-proxy.conf
@@ -1,4 +1,5 @@
 [provider/proxy]
+proxy_max_children = int, None, false
 
 [provider/proxy/id]
 proxy_lib_name = str, None, true
diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index ae291e0fc8f2f9afabcdf32f18a5ec12252f..9ea331e721f8f1d35b1f891303c519033749bc8e 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -2464,6 +2464,21 @@ subdomain_inherit = ldap_purge_cache_timeout
 
 
 
+
+proxy_max_children (integer)
+
+
+In busy environments it is possible sssd runs out of
+available child slots and starts

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-09-05 Thread Fabiano Fidêncio 
Petr,

I see you have updated the OPT_MAX_CHILDREN_DEFAULT to 10 instead of
50. However, you haven't update the value on sssd.conf.5.xml:

diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index 
ae291e0fc8f2f9afabcdf32f18a5ec12252f..1bf3e799047d9c722487be8657bbee5cfd479cdd
100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -2464,6 +2464,18 @@ subdomain_inherit = ldap_purge_cache_timeout
 
 

+
+proxy_max_children (integer)
+
+
+The number of preforked proxy children.
+
+
+Default: 50
   here: ^^^

+
+
+
+
 
 

Apart from this minor the patch seems to be following everything that
was requested during the review process. However, I'm not comfortable
with the text used to describe the new option, so adding there a bit
more information would be super. Like, I don't know what's the
influence of the preforked proxy children to the rest of the code
(probably because I'm a newbie here ;-)), but would be nice to have it
clear in the documentation (for newbies like myself ;-)).

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-09-05 Thread Petr Cech 

Bump.

--
Petr^4 Čech
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-08-30 Thread Petr Cech 

On 08/30/2016 01:21 PM, Pavel Březina wrote:

On 08/30/2016 01:06 PM, Lukas Slebodnik wrote:

On (30/08/16 13:03), Petr Cech wrote:

On 08/30/2016 12:42 PM, Pavel Březina wrote:

On 08/25/2016 01:43 PM, Petr Cech wrote:

-/* FIXME: get max_children from configuration file */
-auth_ctx->max_children = 10;
+ret = confdb_get_int(be_ctx->cdb, be_ctx->conf_path,
+ CONFDB_PROXY_MAX_CHILDREN, 10,
+ _children);
+if (ret != EOK) {
+DEBUG(SSSDBG_CRIT_FAILURE, "Unable to read confdb [%d]:
%s\n",
+   ret, sss_strerror(ret));
+goto done;
+}
+if (max_children < 1) {
+DEBUG(SSSDBG_CRIT_FAILURE, "Option %s must be bigger then
1\n",
+   CONFDB_PROXY_MAX_CHILDREN);
+goto done;
+}


You need to either set ret here, or set max_children to some reasonable
value (10?).


Hello Pavel,

max_children is set on the next line as:
auth_ctx->max_children = max_children;


No it is not.


+if (ret != EOK) {
+DEBUG(SSSDBG_CRIT_FAILURE, "Unable to read confdb [%d]: %s\n",
+   ret, sss_strerror(ret));
+goto done;
+}
+if (max_children < 1) {
+DEBUG(SSSDBG_CRIT_FAILURE, "Option %s must be bigger then 1\n",
+   CONFDB_PROXY_MAX_CHILDREN);
+goto done;
+}
+auth_ctx->max_children = max_children;


if max_children < 1, you report an error and goto done, keeping ret =
EOK from previous successful call. Either we consider it as an error,
and set ret to e.g. EINVAL, or we set default value and continue.


I see, good point. Thank you.

I chose go to done. It prevents our users to have wrong configuration.



I use temporary variable max_children,
because there is issue with signed/unsigned
integer value.

10 was original value. I increase it to 50.
And I use constant OPT_MAX_CHILDREN_DEFAULT now.


10 is a reasonable default and works for most of users.
I do not think we need to increase default value.
This is a purpose of the new option


Agree.


--
Petr^4 Čech
>From 3783aed0c7127358e1708d5657773af50fd136a0 Mon Sep 17 00:00:00 2001
From: Petr Cech 
Date: Wed, 24 Aug 2016 14:41:09 +0200
Subject: [PATCH] PROXY: Adding proxy_max_children option

The new option 'proxy_max_children' is applicable
in domain section. Default value is 10.

Resolves:
https://fedorahosted.org/sssd/ticket/3153
---
 src/confdb/confdb.h   |  1 +
 src/config/SSSDConfig/__init__.py.in  |  3 +++
 src/config/cfg_rules.ini  |  1 +
 src/config/etc/sssd.api.d/sssd-proxy.conf |  1 +
 src/man/sssd.conf.5.xml   | 12 
 src/providers/proxy/proxy_init.c  | 21 +++--
 6 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index 401e5fbf7ed6bb9e8d7158dfab378c8159aa03db..9b5c7bc04bb8297842aa9a0ef50f239c50302757 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -218,6 +218,7 @@
 #define CONFDB_PROXY_LIBNAME "proxy_lib_name"
 #define CONFDB_PROXY_PAM_TARGET "proxy_pam_target"
 #define CONFDB_PROXY_FAST_ALIAS "proxy_fast_alias"
+#define CONFDB_PROXY_MAX_CHILDREN "proxy_max_children"
 
 /* Secrets Service */
 #define CONFDB_SEC_CONF_ENTRY "config/secrets"
diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
index b3f04ac26309bb5b518fb87cd0dae2962e853179..9076dd2c4bf630626d6d8eaef0e7ab67c0ac93f5 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -430,6 +430,9 @@ option_strings = {
 'default_shell' : _('Default shell, /bin/bash'),
 'base_directory' : _('Base for home directories'),
 
+# [provider/proxy]
+'proxy_max_children' : _('The number of preforked proxy children.'),
+
 # [provider/proxy/id]
 'proxy_lib_name' : _('The name of the NSS library to use'),
 'proxy_fast_alias' : _('Whether to look up canonical group name from cache if possible'),
diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index df10538dee4a547a1b1af62a4cfe37b89e236b18..1b3c840199d64fe1a9088147c9c5c836216b25eb 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -323,6 +323,7 @@ option = base_directory
 option = proxy_lib_name
 option = proxy_fast_alias
 option = proxy_pam_target
+option = proxy_max_children
 
 # simple access provider specific options
 option = simple_allow_users
diff --git a/src/config/etc/sssd.api.d/sssd-proxy.conf b/src/config/etc/sssd.api.d/sssd-proxy.conf
index 89a6503f9b84b7eab5fb3b0dd591dea905b43adb..09bf82affcb4263de3abbb67d1d484f6b01a1824 100644
--- a/src/config/etc/sssd.api.d/sssd-proxy.conf
+++ b/src/config/etc/sssd.api.d/sssd-proxy.conf
@@ -1,4 +1,5 @@
 [provider/proxy]
+proxy_max_children = int, None, false
 
 [provider/proxy/id]
 proxy_lib_name = str, None, true
diff --git a/src/man/sssd.conf.5.xml

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-08-30 Thread Lukas Slebodnik 
On (30/08/16 13:03), Petr Cech wrote:
>On 08/30/2016 12:42 PM, Pavel Březina wrote:
>> On 08/25/2016 01:43 PM, Petr Cech wrote:
>> > -/* FIXME: get max_children from configuration file */
>> > -auth_ctx->max_children = 10;
>> > +ret = confdb_get_int(be_ctx->cdb, be_ctx->conf_path,
>> > + CONFDB_PROXY_MAX_CHILDREN, 10,
>> > + _children);
>> > +if (ret != EOK) {
>> > +DEBUG(SSSDBG_CRIT_FAILURE, "Unable to read confdb [%d]: %s\n",
>> > +   ret, sss_strerror(ret));
>> > +goto done;
>> > +}
>> > +if (max_children < 1) {
>> > +DEBUG(SSSDBG_CRIT_FAILURE, "Option %s must be bigger then 1\n",
>> > +   CONFDB_PROXY_MAX_CHILDREN);
>> > +goto done;
>> > +}
>> 
>> You need to either set ret here, or set max_children to some reasonable
>> value (10?).
>
>Hello Pavel,
>
>max_children is set on the next line as:
>auth_ctx->max_children = max_children;
>
>I use temporary variable max_children,
>because there is issue with signed/unsigned
>integer value.
>
>10 was original value. I increase it to 50.
>And I use constant OPT_MAX_CHILDREN_DEFAULT now.
>
10 is a reasonable default and works for most of users.
I do not think we need to increase default value.
This is a purpose of the new option

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-08-30 Thread Petr Cech 

On 08/30/2016 12:42 PM, Pavel Březina wrote:

On 08/25/2016 01:43 PM, Petr Cech wrote:

-/* FIXME: get max_children from configuration file */
-auth_ctx->max_children = 10;
+ret = confdb_get_int(be_ctx->cdb, be_ctx->conf_path,
+ CONFDB_PROXY_MAX_CHILDREN, 10,
+ _children);
+if (ret != EOK) {
+DEBUG(SSSDBG_CRIT_FAILURE, "Unable to read confdb [%d]: %s\n",
+   ret, sss_strerror(ret));
+goto done;
+}
+if (max_children < 1) {
+DEBUG(SSSDBG_CRIT_FAILURE, "Option %s must be bigger then 1\n",
+   CONFDB_PROXY_MAX_CHILDREN);
+goto done;
+}


You need to either set ret here, or set max_children to some reasonable
value (10?).


Hello Pavel,

max_children is set on the next line as:
auth_ctx->max_children = max_children;

I use temporary variable max_children,
because there is issue with signed/unsigned
integer value.

10 was original value. I increase it to 50.
And I use constant OPT_MAX_CHILDREN_DEFAULT now.

Fixed patch is attached.

Regards

--
Petr^4 Čech
>From fb5818026adf0bb8dde45ccae5d94a6cb6331bec Mon Sep 17 00:00:00 2001
From: Petr Cech 
Date: Wed, 24 Aug 2016 14:41:09 +0200
Subject: [PATCH] PROXY: Adding proxy_max_children option

The new option 'proxy_max_children' is applicable
in domain section. Default value is 10.

Resolves:
https://fedorahosted.org/sssd/ticket/3153
---
 src/confdb/confdb.h   |  1 +
 src/config/SSSDConfig/__init__.py.in  |  3 +++
 src/config/cfg_rules.ini  |  1 +
 src/config/etc/sssd.api.d/sssd-proxy.conf |  1 +
 src/man/sssd.conf.5.xml   | 12 
 src/providers/proxy/proxy_init.c  | 20 ++--
 6 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index 401e5fbf7ed6bb9e8d7158dfab378c8159aa03db..9b5c7bc04bb8297842aa9a0ef50f239c50302757 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -218,6 +218,7 @@
 #define CONFDB_PROXY_LIBNAME "proxy_lib_name"
 #define CONFDB_PROXY_PAM_TARGET "proxy_pam_target"
 #define CONFDB_PROXY_FAST_ALIAS "proxy_fast_alias"
+#define CONFDB_PROXY_MAX_CHILDREN "proxy_max_children"
 
 /* Secrets Service */
 #define CONFDB_SEC_CONF_ENTRY "config/secrets"
diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
index b3f04ac26309bb5b518fb87cd0dae2962e853179..9076dd2c4bf630626d6d8eaef0e7ab67c0ac93f5 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -430,6 +430,9 @@ option_strings = {
 'default_shell' : _('Default shell, /bin/bash'),
 'base_directory' : _('Base for home directories'),
 
+# [provider/proxy]
+'proxy_max_children' : _('The number of preforked proxy children.'),
+
 # [provider/proxy/id]
 'proxy_lib_name' : _('The name of the NSS library to use'),
 'proxy_fast_alias' : _('Whether to look up canonical group name from cache if possible'),
diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index df10538dee4a547a1b1af62a4cfe37b89e236b18..1b3c840199d64fe1a9088147c9c5c836216b25eb 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -323,6 +323,7 @@ option = base_directory
 option = proxy_lib_name
 option = proxy_fast_alias
 option = proxy_pam_target
+option = proxy_max_children
 
 # simple access provider specific options
 option = simple_allow_users
diff --git a/src/config/etc/sssd.api.d/sssd-proxy.conf b/src/config/etc/sssd.api.d/sssd-proxy.conf
index 89a6503f9b84b7eab5fb3b0dd591dea905b43adb..09bf82affcb4263de3abbb67d1d484f6b01a1824 100644
--- a/src/config/etc/sssd.api.d/sssd-proxy.conf
+++ b/src/config/etc/sssd.api.d/sssd-proxy.conf
@@ -1,4 +1,5 @@
 [provider/proxy]
+proxy_max_children = int, None, false
 
 [provider/proxy/id]
 proxy_lib_name = str, None, true
diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index ae291e0fc8f2f9afabcdf32f18a5ec12252f..1bf3e799047d9c722487be8657bbee5cfd479cdd 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -2464,6 +2464,18 @@ subdomain_inherit = ldap_purge_cache_timeout
 
 
 
+
+proxy_max_children (integer)
+
+
+The number of preforked proxy children.
+
+
+Default: 50
+
+
+
+
 
 
 
diff --git a/src/providers/proxy/proxy_init.c b/src/providers/proxy/proxy_init.c
index 1edf4fd64e54f4f0df7a78a9e56eb232a1d3e948..ed5e825a730c2db9072de4a47f5d91720f4d 100644
--- a/src/providers/proxy/proxy_init.c
+++ b/src/providers/proxy/proxy_init.c
@@ -29,6 +29,8 @@
 
 #define NSS_FN_NAME "_nss_%s_%s"
 
+#define

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-08-30 Thread Pavel Březina 

On 08/25/2016 01:43 PM, Petr Cech wrote:

-/* FIXME: get max_children from configuration file */
-auth_ctx->max_children = 10;
+ret = confdb_get_int(be_ctx->cdb, be_ctx->conf_path,
+ CONFDB_PROXY_MAX_CHILDREN, 10,
+ _children);
+if (ret != EOK) {
+DEBUG(SSSDBG_CRIT_FAILURE, "Unable to read confdb [%d]: %s\n",
+   ret, sss_strerror(ret));
+goto done;
+}
+if (max_children < 1) {
+DEBUG(SSSDBG_CRIT_FAILURE, "Option %s must be bigger then 1\n",
+   CONFDB_PROXY_MAX_CHILDREN);
+goto done;
+}


You need to either set ret here, or set max_children to some reasonable 
value (10?).

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-08-25 Thread Petr Cech 

On 08/24/2016 05:25 PM, Fabiano Fidêncio wrote:

Petr,

On Wed, Aug 24, 2016 at 4:22 PM, Petr Cech  wrote:

Hello,

I am fighting with adding new option to sssd.conf.
I slowly running out of breath.

I know proxy could be id, auth or chpass provider. I don't know
where is the right place for my option. And the second issue is
it breaks test for SSSD config. :-(

Is there anyone who would like to join to the fight? Please,
see attached patch.


I could spot 1 issue and one possible issue with your patch.
Let me paste the (possible) problematic parts here:


diff --git a/src/config/SSSDConfig/__init__.py.in 
b/src/config/SSSDConfig/__init__.py.in
index 
b3f04ac26309bb5b518fb87cd0dae2962e853179..50917322da74211a54db69fee05589bdddaebd33
 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -435,6 +435,7 @@ option_strings = {
 'proxy_fast_alias' : _('Whether to look up canonical group name from cache 
if possible'),

 # [provider/proxy/auth]
+'proxy_max_children' : _('The number of preforked proxy children.'),
 'proxy_pam_target' : _('PAM stack to use')
 }


As far as I understand from the ticket, proxy_max_children should be a
global option for proxy, so you should put it ander [provider/proxy]
and not under [provider/proxy/auth].


diff --git a/src/config/etc/sssd.api.d/sssd-proxy.conf 
b/src/config/etc/sssd.api.d/sssd-proxy.conf
index 89a6503..96e2d4a 100644
--- a/src/config/etc/sssd.api.d/sssd-proxy.conf
+++ b/src/config/etc/sssd.api.d/sssd-proxy.conf
@@ -1,11 +1,9 @@
-[provider/proxy]
-
 [provider/proxy/i
 proxy_lib_name = str, None, true
 proxy_fast_alias = bool, None, true

 [provider/proxy/auth]
  proxy_pam_target = str, None, true
+proxy_max_children = int, None, false

 [provider/proxy/chpass]
-


On this part I'm pretty sure you don't want to remove [provider/proxy]
neither the last line of the file.
And as far as I understand from the option you're trying to add, it
should be added under [provider/proxy].

With these 2 changes "make check" passes again and I guess it's enough
for what you're trying to achieve.

Here you can see my changes on top of your changes:
[ffidenci@cat x86_64]$ git diff
diff --git a/src/config/SSSDConfig/__init__.py.in
b/src/config/SSSDConfig/__init__.py.in
index 5091732..9076dd2 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -430,12 +430,14 @@ option_strings = {
 'default_shell' : _('Default shell, /bin/bash'),
 'base_directory' : _('Base for home directories'),

+# [provider/proxy]
+'proxy_max_children' : _('The number of preforked proxy children.'),
+
 # [provider/proxy/id]
 'proxy_lib_name' : _('The name of the NSS library to use'),
 'proxy_fast_alias' : _('Whether to look up canonical group name
from cache if possible'),

 # [provider/proxy/auth]
-'proxy_max_children' : _('The number of preforked proxy children.'),
 'proxy_pam_target' : _('PAM stack to use')
 }

diff --git a/src/config/etc/sssd.api.d/sssd-proxy.conf
b/src/config/etc/sssd.api.d/sssd-proxy.conf
index 96e2d4a..09bf82a 100644
--- a/src/config/etc/sssd.api.d/sssd-proxy.conf
+++ b/src/config/etc/sssd.api.d/sssd-proxy.conf
@@ -1,9 +1,12 @@
+[provider/proxy]
+proxy_max_children = int, None, false
+
 [provider/proxy/id]
 proxy_lib_name = str, None, true
 proxy_fast_alias = bool, None, true

 [provider/proxy/auth]
 proxy_pam_target = str, None, true
-proxy_max_children = int, None, false

 [provider/proxy/chpass]
+

I hope it helps!

Best Regards,
--
Fabiano Fidêncio


Hi Fabiano,

thank you for help. You was right with section
[provider/proxy].

And I hit little issue with last white line in file.
My IDE did some automagic.

The entry in man page is really short. Please, if you have any idea, 
share it.


Regards

--
Petr^4 Čech
>From b608fb4e3968758cc9f3aaa00cedba7561f33dbe Mon Sep 17 00:00:00 2001
From: Petr Cech 
Date: Wed, 24 Aug 2016 14:41:09 +0200
Subject: [PATCH] PROXY: Adding proxy_max_children option

The new option 'proxy_max_children' is applicable
in domain section. Default value is 10.

Resolves:
https://fedorahosted.org/sssd/ticket/3153
---
 src/confdb/confdb.h   |  1 +
 src/config/SSSDConfig/__init__.py.in  |  3 +++
 src/config/cfg_rules.ini  |  1 +
 src/config/etc/sssd.api.d/sssd-proxy.conf |  1 +
 src/man/sssd.conf.5.xml   | 12 
 src/providers/proxy/proxy_init.c  | 17 +++--
 6 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index 72adbd80ea534eb0becd3e517c00b0c26d00444c..dddf3e94fc4a083dfe23549c50c75b8fe1e47c9f 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -220,6 +220,7 @@
 #define CONFDB_PROXY_LIBNAME "proxy_lib_name"
 #define CONFDB_PROXY_PAM_TARGET "proxy_pam_target"
 #define CONFDB_PROXY_FAST_ALIAS "proxy_fast_alias"
+#define CONFDB_PROXY_MAX_CHILDREN