[LEDE-DEV] [PATCH 2/3][RFC] base-files: add option to rc.common to disable default reload behavior

[Alexandru Ardelean]

On Tue, Mar 28, 2017 at 11:36 PM, Hans Dedecker <dedeckeh at gmail.com> wrote:
> On Mon, Mar 20, 2017 at 3:08 PM, Alexandru Ardelean
> <ardeleanalex at gmail.com> wrote:
>> From: Alexandru Ardelean <ardeleanalex at gmail.com>
>>
>> Traditionally if a reload script fails, it will fallback to restart.
>>
>> That seems to be the default behavior in case no reload
>> handler has been specified, and `reload` will return 1.
>>
>> That also has the disadvantage of masking reload errors
>> from `/etc/init.d/<service> reload`.
>>
>> Still, it's a pretty old behavior, and in most cases
>> it should be fine.
>>
>> For other cases, the `RESTART_ON_RELOAD_ERR=0` can
>> be specified to override this.
>>
>> Not sure about the correctness of this approach,
>> so this patch is RFC.
> Discussed the restart-on-reload-fail behaviour with Felix and
> Matthias; general consensus is to remove the restart-on-reload-fail
> behaviour in rc.common. Packages do not seem to depend on this
> restart-on-reload fail behaviour and it's more logical to handle
> reload failure in the packages itself if extra logic is required.

Thanks for the consideration.

I'll submit another patch series just for this restart-on-reload fail.

I feel this mechanism was maybe intended to act as default reload
implementation.
In the sense, that a default reload hook is implemented returning
non-zero, and if no specific reload is implemented, this logic [
restart on reload fail ] would work.

Will think about it a bit.

Thanks
Alex

>
> Hans
>>
>> Signed-off-by: Alexandru Ardelean <ardeleanalex at gmail.com>
>> ---
>>  package/base-files/files/etc/rc.common | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/package/base-files/files/etc/rc.common b/package/base-files/files/etc/rc.common
>> index 95cf956..a893b09 100755
>> --- a/package/base-files/files/etc/rc.common
>> +++ b/package/base-files/files/etc/rc.common
>> @@ -139,7 +139,13 @@ ${INIT_TRACE:+set -x}
>>         }
>>  }
>>
>> +RESTART_ON_RELOAD_ERR=${RESTART_ON_RELOAD_ERR:-1}
>> +
>>  ALL_COMMANDS="start stop reload restart boot shutdown enable disable enabled depends ${EXTRA_COMMANDS}"
>>  list_contains ALL_COMMANDS "$action" || action=help
>> -[ "$action" = "reload" ] && action='eval reload "$@" || restart "$@" && :'
>> +[ "$action" = "reload" ] && {
>> +       if [ "$RESTART_ON_RELOAD_ERR" == "1" ] ; then
>> +               action='eval reload "$@" || restart "$@" && :'
>> +       fi
>> +}
>>  $action "$@"
>> --
>> 2.7.4
>>