[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders lslebodn commented: """ * 560daa14ef013aa14e2aedeea10b07f623d84ec8 * 151a6de4793e0045a7085d4d72b975947662e566 * 32c76642250b3ba3b173d0576c0d00b0190320a9 *

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ New patch set pushed! """ See the full comment at https://github.com/SSSD/sssd/pull/94#issuecomment-274498903 ___ sssd-devel

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders lslebodn commented: """ On (23/01/17 03:30), Pavel Březina wrote: >pbrezina commented on this pull request. >> @@ -304,16 +239,6 @@ static errno_t dp_target_special(struct be_ctx *be_ctx, >

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders pbrezina commented: """ > > if (conn->last_request_time != NULL) { > > time_t *last_request_time = conn->last_request_time; > > > > *last_request_time = time(NULL); > > }

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ New patch set pushed addressing @lslebodn's review. @lslebodn, @pbrezina: I'm wondering whether we should drop this patch [0] and make it part of a different series.

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ New patch set pushed addressing @lslebodn's review. @lslebodn, @pbrezina: I'm wondering whether we should drop this patch [0] and make it part of a different series.

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ Patchset updated! I had to do some changes in a few commits in order to fix a regression introduced by the previous series. Let me try to sum up the changes: - SBUS:

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ On Thu, Jan 19, 2017 at 1:51 PM, Pavel Březina wrote: > if (conn->last_request_time != NULL) { > time_t *last_request_time =

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ @pbrezina: Oh! Okay, I'll fix it before submitting a new version (as it doesn't affect the functionality). Thanks for the review. """ See the full comment at

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders pbrezina commented: """ ```c if (conn->last_request_time != NULL) { time_t *last_request_time = conn->last_request_time; *last_request_time = time(NULL); } ``` This

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ @lslebodn: Okay, I was able to reproduce both issues, they have been fixed and a new series has been updated. I do believe the crash you saw (even I'm not able to

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ @lslebodn: About kill -9 not restarting the responder ... it's already fixed locally by the my patches covering @jhrozek review. About the second one, I'll dig into it

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ @lslebodn: About kill -9 not restarting the responder ... it's already fixed locally by the my patches covering @jhrozek review. About the second one, I'll dig into it

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders lslebodn commented: """ And it seems that is also possible to add invalid service to this option. Previously it was not possible e.g. ``` sed -i 's/services = nss, pam/services = nss, pam,

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders lslebodn commented: """ As part of different testing I found out that services are not restarted if they die. There send `kill -9` to some responder which is listed in "services" option.

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ @lslebodn: Can you gimme a reproducer (even if it's not reliable)? """ See the full comment at https://github.com/SSSD/sssd/pull/94#issuecomment-273052238

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders lslebodn commented: """ I still can see the crash in latest version. It was not related to the quoted change """ See the full comment at

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders lslebodn commented: """ I still can see the crash in latest version. It was not related to the quoted change """ See the full comment at

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders lslebodn commented: """ On (15/01/17 14:54), fidencio wrote: >fidencio commented on this pull request. > > > >> +} >+ >+/* As the service is a responder and wasn't part of the services'

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ @jhrozek , thanks for the review. A new series will be update in a few and I hope all the issues raised are addressed. """ See the full comment at

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ @jhrozek , thanks for the review. A new series will be update in a few and I hope all the issues raised are addressed. """ See the full comment at

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders jhrozek commented: """ oh, and just so that it's known what I tested, I removed the services line altogether, then went one-by-one through all the responders and tested them by enabling the

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders jhrozek commented: """ OK, I don't have any more comments than those inline. Thank you for the patches, really nice work. I'm just setting the Changes Requested label so that it's clear I'm

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders jhrozek commented: """ On Fri, Jan 13, 2017 at 02:52:49AM -0800, fidencio wrote: > On Fri, Jan 13, 2017 at 11:42 AM, Jakub Hrozek > wrote: > > > *@jhrozek* commented

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ On Fri, Jan 13, 2017 at 11:42 AM, Jakub Hrozek wrote: > *@jhrozek* commented on this pull request. > -- > > In

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ Pushed a new version. """ See the full comment at https://github.com/SSSD/sssd/pull/94#issuecomment-272136455 ___ sssd-devel

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ Pushed a new version. """ See the full comment at https://github.com/SSSD/sssd/pull/94#issuecomment-272136455 ___ sssd-devel

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders lslebodn commented: """ Test are broken on all platforms http://sssd-ci.duckdns.org/logs/job/59/96/summary.html """ See the full comment at

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ New patchset has been pushed. Hopefully all comments have been addressed. """ See the full comment at https://github.com/SSSD/sssd/pull/94#issuecomment-272042851

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ On Wed, Dec 21, 2016 at 11:32 AM, Fabiano Fidêncio wrote: > > > On Wed, Dec 21, 2016 at 10:59 AM, Jakub Hrozek > wrote:

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ On Wed, Dec 21, 2016 at 10:59 AM, Jakub Hrozek wrote: > Coverity seems to have detected a warning: > > Error: CHECKED_RETURN (CWE-252): >

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders jhrozek commented: """ Coverity seems to have detected a warning: Error: CHECKED_RETURN (CWE-252): sssd-1.14.90/src/responder/autofs/autofssrv_cmd.c:323: check_return: Calling

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ Okay, I'm about to push a new version of the series ... just some small tips for testing: - Use SELinux as permissive or disabled, at least till we have the SELinux

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ Okay, I'll about to push a new version of the series ... just some small tips for testing: - Use SELinux as permissive or disabled, at least till we have the SELinux

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders pbrezina commented: """ > *sudo* Sudo is a little bit special responder. Since it requires some periodic task in the backend, we decided a long time ago to disable sudo in backend unless it

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders pbrezina commented: """ > ../src/sbus/sssd_dbus_connection.c: In function ‘sbus_init_connection’: > ../src/sbus/sssd_dbus_connection.c:181:9: warning: initialization from > incompatible

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ On Mon, Dec 12, 2016 at 7:13 PM, Fabiano Fidêncio wrote: > > > On Mon, Dec 12, 2016 at 2:26 PM, Fabiano Fidêncio > wrote: >

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ On Mon, Dec 12, 2016 at 2:26 PM, Fabiano Fidêncio wrote: > > > On Mon, Dec 12, 2016 at 1:34 PM, Pavel Březina > wrote: >

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders jhrozek commented: """ On Mon, Dec 12, 2016 at 05:19:18AM -0800, fidencio wrote: > >Would it make any sense to own the sockets by the sssd user as well? > >Currently it seems the sockets

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ On Mon, Dec 12, 2016 at 1:34 PM, Pavel Březina wrote: > *Unit files* > I wonder if the user and group in unit files should take

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ On Mon, Dec 12, 2016 at 12:40 PM, Jakub Hrozek wrote: > Hi, > I read the patches, so far I didn't do a whole lot of testing, so maybe > some

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders lslebodn commented: """ On (12/12/16 03:40), Jakub Hrozek wrote: >1. There were some SELinux denials on my test VM, but granted, I run F-24 >there. We need to make sure that no SELinux AVC

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders pbrezina commented: """ **Unit files** I wonder if the user and group in unit files should take --with-sssd-user value from configure. **MAN: Mention that the services' list is optional** It

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders jhrozek commented: """ Hi, I read the patches, so far I didn't do a whole lot of testing, so maybe some questions here are invalid, but nonetheless, here is my take on the patchset: 1.

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ Just pushed a new series. Checks have failed but the reason seems to be the CentOS CI (https://ci.centos.org/job/sssd-CentOS6/218/consoleText). """ See the full

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders pbrezina commented: """ Squashing is fine, of course. BTW I'm not quite sure if this version contains the uid/gid changes as I see it as parameters in all systemd unit files. In my opinion,

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ On Thu, Dec 1, 2016 at 1:03 PM, Pavel Březina wrote: > Hi, > > if (ret != EOK) { > #ifdef HAVE_SYSTEMD > if (ret != ENOENT) > #endif >

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders pbrezina commented: """ Sounds good to me. """ See the full comment at https://github.com/SSSD/sssd/pull/94#issuecomment-263818341 ___ sssd-devel

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ On Tue, Nov 29, 2016 at 10:05 AM, Fabiano Fidêncio wrote: > > > On Tue, Nov 29, 2016 at 9:33 AM, Pavel Březina > wrote:

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders fidencio commented: """ On Tue, Nov 29, 2016 at 9:33 AM, Pavel Březina wrote: > One thing I don't like about those patches is that we always recreate the > idle

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders pbrezina commented: """ One thing I don't like about those patches is that we always recreate the idle timer. Can we get around this (maybe also for the client idle timeout)? What I have i

[SSSD] [sssd PR#94][comment] Enable {socket,dbus}-activation for responders

URL: https://github.com/SSSD/sssd/pull/94 Title: #94: Enable {socket,dbus}-activation for responders pbrezina commented: """ Ok, I read those commits and I so far have none code issues. Haven't tried it yet. """ See the full comment at