Re: [LTP] [PATCH] cron_tests.sh: fix a bug of syslog daemon is disabled

[chrubis@suse.cz]

Hi!
> ...
> >> +if [ -n "$SYSLOG_NAME" ]; then
> >> +	if [ $HAVE_SYSTEMCTL == 1 ]; then
> >> +		status_daemon $SYSLOG_NAME
> >> +		if [ $? -ne 0 ]; then
> >> +			restart_daemon $SYSLOG_NAME
> >> +			FLAG=1
> >> +		fi
> ...
> >> +if [ $FLAG -eq 1 ]; then
> >> +	if [ $HAVE_SYSTEMCTL == 1 ]; then
> >> +		stop_daemon $SYSLOG_NAME
> >> +	else
> >> +		$SYSLOG_NAME stop >/dev/null 2>&1
> >> +	fi
> >> +fi
> >> +
> > 
> > We have similar code to restart cron in cron/cron02 and to restart nscd
> > in sssd/sssd-libs.sh.
> > 
> > What about we create a short library with a common code to restart
> > system daemons? So that any new testcase would just need to source the
> 
> A short library is in cmdlib.sh.
> stop_daemon, status_daemon and restart_daemon is used in the patch.

Then I do not understand why the patch adds systemctl detection
code to the cron_tests.sh. It is allready present in the cmdlib.sh

> The syslog daemon may be syslog, rsyslog, syslog-ng etc, 
> so I make the patch which is similar to syslog-lib.sh.

Which is why I object to the patch, it copies code from library to the
testcase, which is wrong in terms of longterm maintenance.

Ideal solution would be to move the syslog detection to the cmdlib.sh
and make a canonical name for syslog so that if you request to restart
the daemon in test with:

daemon_restart syslog

Everything is detected automatically, the syslog binary, systemd vs
system V init, etc. And everythig is kept in one file, no code
duplications.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list