Hi,

On 11.04.2020 10:56, Michael Tremer wrote:
> Hey,
> 
>> On 10 Apr 2020, at 18:17, Matthias Fischer <matthias.fischer(a)ipfire.org> wrote:
>> 
>> Added the  new 'vnstatd' daemon to 'start' and 'stop' section.
>> 
>> Signed-off-by: Matthias Fischer <matthias.fischer(a)ipfire.org>
>> ---
>> src/initscripts/system/vnstat | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>> 
>> diff --git a/src/initscripts/system/vnstat b/src/initscripts/system/vnstat
>> index 363307013..bcc19c3ab 100755
>> --- a/src/initscripts/system/vnstat
>> +++ b/src/initscripts/system/vnstat
>> @@ -17,15 +17,28 @@ case "$1" in
>> 			mount_ramdisk "${VNSTATLOG}"
>> 			evaluate_retval
>> 		fi
>> +
>> +		boot_mesg "Starting vnstatd..."
>> +		loadproc /usr/sbin/vnstatd -d --alwaysadd
>> +		sleep 2
>> +		evaluate_retval
> 
> What is the sleep operation necessary for?

Thinking about it, I thought it could be necessary to give the script a
little time to complete...

> Also, evaluate_retval will now always consider the return code of “sleep” which is probably not what we want here. We want to see when launching vnstatd fails.

I see. Ok.

> Also, loadproc will already take care of printing “OK” or “ERROR”.

Changed => deleted 'sleep 2' => https://patchwork.ipfire.org/patch/2935/

Best,
Matthias

> 
>> 		;;
>> +
>> 	stop)
>> +		boot_mesg "Stopping vnstatd..."
>> +		killproc /usr/sbin/vnstatd
>> +		sleep 2
>> +		evaluate_retval
> 
> Same here. It delays the shutdown operation unnecessarily.
> 
> Best,
> -Michael
> 
>> +
>> 		umount_ramdisk "${VNSTATLOG}"
>> 		;;
>> +
>> 	restart)
>> 		${0} stop
>> 		sleep 1
>> 		${0} start
>> 		;;
>> +
>> 	backup)
>> 		# Backup all data if ramdisk is used
>> 		if mountpoint "${RRDLOG}" &>/dev/null; then
>> -- 
>> 2.18.0
>> 
>