diff mbox series

[3/4] vnstat 2.6: New initscript

Message ID 20200410171711.3902-3-matthias.fischer@ipfire.org
State Accepted
Commit 5a5de3f0269983bec1d7ffc9f4019b96b1c560a0
Headers
Series [1/4] vnstat: Update to 2.6 |

Commit Message

Matthias Fischer April 10, 2020, 5:17 p.m. UTC 
  Added the  new 'vnstatd' daemon to 'start' and 'stop' section.

Signed-off-by: Matthias Fischer <matthias.fischer@ipfire.org>
---
 src/initscripts/system/vnstat | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Michael Tremer April 11, 2020, 8:56 a.m. UTC | #1 
Hey,

> On 10 Apr 2020, at 18:17, Matthias Fischer <matthias.fischer@ipfire.org> wrote:
> 
> Added the  new 'vnstatd' daemon to 'start' and 'stop' section.
> 
> Signed-off-by: Matthias Fischer <matthias.fischer@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?

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.

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

> 		;;
> +
> 	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
>
Matthias Fischer April 11, 2020, 2:42 p.m. UTC | #2 
Hi,

On 11.04.2020 10:56, Michael Tremer wrote:
> Hey,
> 
>> On 10 Apr 2020, at 18:17, Matthias Fischer <matthias.fischer@ipfire.org> wrote:
>> 
>> Added the  new 'vnstatd' daemon to 'start' and 'stop' section.
>> 
>> Signed-off-by: Matthias Fischer <matthias.fischer@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
>> 
>
diff mbox series

Patch

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
 		;;
+
 	stop)
+		boot_mesg "Stopping vnstatd..."
+		killproc /usr/sbin/vnstatd
+		sleep 2
+		evaluate_retval
+
 		umount_ramdisk "${VNSTATLOG}"
 		;;
+
 	restart)
 		${0} stop
 		sleep 1
 		${0} start
 		;;
+
 	backup)
 		# Backup all data if ramdisk is used
 		if mountpoint "${RRDLOG}" &>/dev/null; then