Commit Message
We now only start radvd when we write a config fr a zone into the config
file and when no errors happen.
Fixes: #11450
Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
---
src/functions/functions.radvd | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
Comments
Hi,
On Sat, 2018-01-27 at 17:06 +0000, Jonatan Schlag wrote:
> We now only start radvd when we write a config fr a zone into the config
> file and when no errors happen.
>
> Fixes: #11450
>
> Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
> ---
> src/functions/functions.radvd | 31 +++++++++++++++++++------------
> 1 file changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/src/functions/functions.radvd b/src/functions/functions.radvd
> index 4e41160..bd016a0 100644
> --- a/src/functions/functions.radvd
> +++ b/src/functions/functions.radvd
> @@ -23,16 +23,16 @@ RADVD_CONFIGFILE="/etc/radvd.conf"
>
> radvd_update() {
> # (Re-)write the configuration file
> - radvd_write_config
> -
> - # Reload the radvd service if it is already running
> - if service_is_active radvd; then
> - service_reload radvd
> - return ${EXIT_OK}
> + if radvd_write_config; then
> + # Reload the radvd service if it is already running
> + if service_is_active radvd; then
> + service_reload radvd
> + return ${EXIT_OK}
> + fi
> +
> + # Start the radvd service
> + service_start radvd
> fi
> -
> - # Start the radvd service
> - service_start radvd
> }
This part is okay assuming that radvd_write_config is returning an error when
the configuration file is empty.
>
> radvd_clear_config() {
> @@ -48,12 +48,19 @@ radvd_write_config() {
>
> # Write the configuration for all zones.
> local zone
> - for zone in $(zones_get_local); do
> - __radvd_config_interface ${zone}
> + # The return value determine if radvd is started or not
> + local return_value=${EXIT_FALSE}
>
> + for zone in $(zones_get_local); do
> + if __radvd_config_interface ${zone} && [ ${return_value} =
> ${EXIT_TRUE} ]; then
> + # We return TRUE when __radvd_config_interface
> succeed and no errors happend till now
> + return_value=${EXIT_TRUE}
> + else
> + return_value=${EXIT_FALSE}
> + fi
> done >> ${RADVD_CONFIGFILE}
>
> - return ${EXIT_OK}
> + return ${return_value}
> }
This is a bit spaghetti and can be written better. Like:
r = TRUE
for zone in ...; do
if ! __radvd_config_interface "${zone}"; then
r = FALSE
fi
done
return r
Of course that is just pseudo-code, but it spares the else clause and the
comparison which isn't needed.
You will always run radvd even when you have interfaces that don't have IPv6. I
don't see where this is now being skipped.
-Michael
@@ -23,16 +23,16 @@ RADVD_CONFIGFILE="/etc/radvd.conf"
radvd_update() {
# (Re-)write the configuration file
- radvd_write_config
-
- # Reload the radvd service if it is already running
- if service_is_active radvd; then
- service_reload radvd
- return ${EXIT_OK}
+ if radvd_write_config; then
+ # Reload the radvd service if it is already running
+ if service_is_active radvd; then
+ service_reload radvd
+ return ${EXIT_OK}
+ fi
+
+ # Start the radvd service
+ service_start radvd
fi
-
- # Start the radvd service
- service_start radvd
}
radvd_clear_config() {
@@ -48,12 +48,19 @@ radvd_write_config() {
# Write the configuration for all zones.
local zone
- for zone in $(zones_get_local); do
- __radvd_config_interface ${zone}
+ # The return value determine if radvd is started or not
+ local return_value=${EXIT_FALSE}
+ for zone in $(zones_get_local); do
+ if __radvd_config_interface ${zone} && [ ${return_value} = ${EXIT_TRUE} ]; then
+ # We return TRUE when __radvd_config_interface succeed and no errors happend till now
+ return_value=${EXIT_TRUE}
+ else
+ return_value=${EXIT_FALSE}
+ fi
done >> ${RADVD_CONFIGFILE}
- return ${EXIT_OK}
+ return ${return_value}
}
__radvd_config_interface() {