On Tue, Jun 21, 2016 at 04:15:06PM +0200, Pavel Březina wrote:
On 06/21/2016 03:50 PM, Jakub Hrozek wrote:
> On Mon, Jun 20, 2016 at 07:13:10PM +0200, Jakub Hrozek wrote:
> > Hi,
> >
> > Pavel's sssctl tool is at:
> >
https://fedorapeople.org/cgit/pbrezina/public_git/sssd.git/log/?h=sssctl
> >
> > I'll try to have a first pass at review today in the evening, but I
> > thought I'll pass on the branch in case anyone else is interested..
>
> Hi, I went through the patches today. I pushed some of my review
> comments to my branch:
>
https://github.com/jhrozek/sssd/tree/sssctl
> these are mostly trivial. The only important part is that sss_simpleifp
> changes are no longer backwards-incompatible to make enterprise distros
> happy.
>
> Other comments:
> 1) The sssctl tool restarts sssd with a command and only supports
> systemctl and service. For Beta, I would propose:
> - if sssd is built with systemd, use systemctl
> - otherwise, detect /sbin/service during configure time. If it's
> available, use /sbin/service
> - otherwise, don't support restarting sssd at all, but tell the
> admin to restart it
Agree. I was thinking whether to put it in or not but then again it's to
simplify life of our users but I didn't want to overthink it. All those
operations can be done manually without sssctl.
>
> For the next release, I would prefer to call systemd D-Bus
> interface to restart the service instead. This is cross-platform
> enough. Let other distributions submit patches for restarting the
> service via another service manager. This would be handled with a
> 1.14.0 ticket.
Do you mean this interface?
https://www.freedesktop.org/wiki/Software/systemd/dbus/
Yes.
>
> 2) We should open a ticket to merge sss_cache, sss_debuglevel and
> maybe others under sssctl. sss_cache would just be a shell wrapper
> that calls sssctl for some time and eventually removed (in 2.0?)
Agree.
>
> 3) the commands that remove the cache should remove the timestamps
> cache as well, if it exists (depends on which patches get to master
> first)
>
> 4) removing logs -- is there a reason to not remove *.log but
> special case child.log as well?
The same reason why I avoided to remove *.ldb and pinpointed the files
instead. So we do not remove anything that doesn't belong to SSSD even if it
is under our directory.
It is /our/ directory :-) I think a wildcard would be better, otherwise,
I'm sure we will forget to add a pattern when we add some new logfile.
>
> >
> > 5) CI fails. I haven't ran Coverity yet either.
> >
>