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. > > > _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
