[PATCH 1/5] zone: fix zone_config()

[PATCH 1/5] zone: fix zone_config()

[Jonatan Schlag]

[-- Attachment #1: Type: text/plain, Size: 785 bytes --]

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 src/functions/functions.zone | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/functions/functions.zone b/src/functions/functions.zone
index acf68f5..11a8dc2 100644
--- a/src/functions/functions.zone
+++ b/src/functions/functions.zone
@@ -565,9 +565,9 @@ zone_config() {
 			# TODO This could be also a valid hid
 			local id=${cmd}
 
-			if zone_config_id_is valid ${zone} ${id} && [[ ${1} == "edit" ]]; then
+			if zone_config_id_is_valid ${zone} ${id} && [[ ${1} == "edit" ]]; then
 				shift 1
-				zone_config_edit "${zone}" "${id}""$@"
+				zone_config_edit "${zone}" "${id}" "$@"
 			else
 				error "Unrecognized argument: ${cmd}"
 				cli_usage root-zone-config-subcommands
-- 
2.6.3

[PATCH 2/5] header-zone fix hook_config_edit()

[Jonatan Schlag]

[-- Attachment #1: Type: text/plain, Size: 568 bytes --]

This function accepted only two arguments so new cmd coudl be passed.
We accept now more then 2 arguments.

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 src/header-zone | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/header-zone b/src/header-zone
index 2e3fa09..98b0ce7 100644
--- a/src/header-zone
+++ b/src/header-zone
@@ -225,7 +225,7 @@ hook_config_destroy() {
 }
 
 hook_config_edit() {
-	assert [ $# -eq 2 ]
+	assert [ $# -ge 2 ]
 	local zone=${1}
 	# The id must be the id and not the hid.
 	local id=${2}
-- 
2.6.3

[PATCH 3/5] dhcp: check the config indide the hook_parse_cmdline() function

[Jonatan Schlag]

[-- Attachment #1: Type: text/plain, Size: 1083 bytes --]

We now check the config inside the hook_parse_cmdline function.
This mae it possible ti use this function in a generic edit function.

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 src/hooks/configs/dhcp | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/src/hooks/configs/dhcp b/src/hooks/configs/dhcp
index 8bb6aa9..7f6780b 100644
--- a/src/hooks/configs/dhcp
+++ b/src/hooks/configs/dhcp
@@ -55,6 +55,12 @@ hook_parse_cmdline() {
 		esac
 		shift
 	done
+
+	# Check if the user disabled ipv6 and ipv4
+	if ! enabled ENABLE_IPV6 && ! enabled ENABLE_IPV4; then
+		log ERROR "You disabled IPv6 and IPv4. At least one must be enabled"
+		return ${EXIT_ERROR}
+	fi
 }
 
 hook_new() {
@@ -71,13 +77,6 @@ hook_new() {
 		return ${EXIT_ERROR}
 	fi
 
-	# Check if the user disabled ipv4 and ipv6
-
-	if ! enabled ENABLE_IPV6 && ! enabled ENABLE_IPV4; then
-		log ERROR "You disabled IPv6 and IPv4. At least one must be enabled"
-		return ${EXIT_ERROR}
-	fi
-
 	zone_config_settings_write "${zone}" "${HOOK}"
 
 	exit ${EXIT_OK}
-- 
2.6.3

[PATCH 4/5] config: add new functions

[Jonatan Schlag]

[-- Attachment #1: Type: text/plain, Size: 986 bytes --]

This patch add two new functions:
config_get_id_from_config()
config_get_hook_from_config

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 src/functions/functions.config | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/src/functions/functions.config b/src/functions/functions.config
index 854f490..e11a1c2 100644
--- a/src/functions/functions.config
+++ b/src/functions/functions.config
@@ -51,3 +51,23 @@ config_domainname() {
 	# the domain part.
 	print "${hostname#*.}"
 }
+
+config_get_id_from_config() {
+	# This function returns the id for a given config name
+	# Example 'dhcp.0' => 0
+	assert [ $# -eq 1 ]
+	local config=${1}
+
+	local hook=$(config_get_hook_from_config ${config})
+	echo "${config//"${hook}."/}"
+
+}
+
+config_get_hook_from_config() {
+	# This function returns the hook for a given config name
+	# Example 'dhcp.0' => dhcp
+	assert [ $# -eq 1 ]
+	local config=${1}
+
+	echo "${config//.*[[:digit:]]/}"
+}
-- 
2.6.3

[PATCH 5/5] header-config: add generic hook_edit function

[Jonatan Schlag]

[-- Attachment #1: Type: text/plain, Size: 1855 bytes --]

If a hook_parse-cmdline function exists
this functions allows it do edit the hook safely.

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 src/header-config | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/src/header-config b/src/header-config
index b697797..e3c6423 100644
--- a/src/header-config
+++ b/src/header-config
@@ -22,3 +22,57 @@
 hook_new() {
 	cmd_not_implemented
 }
+
+hook_edit() {
+	local zone=${1}
+	local config=${2}
+	shift 2
+
+	local hook=$(config_get_hook_from_config ${config})
+	local id=$(config_get_id_from_config ${config})
+
+	assert isset hook
+	assert isset id
+
+	if ! zone_exists ${zone}; then
+		error "Zone '${zone}' doesn't exist."
+		exit ${EXIT_ERROR}
+	fi
+
+	# Bring the config down
+	if device_exists ${zone}; then
+		if ! hook_config_cmd "down" "${zone}" "${hook}" "${hook}.${id}"; then
+			log ERROR "Could not bring config ${config} down for zone ${zone}"
+			return ${EXIT_ERROR}
+		fi
+	fi
+
+	local ${HOOK_CONFIG_SETTINGS}
+
+	# If reading the config fails we cannot go on
+	if ! zone_config_settings_read "${zone}" "${config}"; then
+		log ERROR "Could read the config ${config} for zone ${zone}"
+		return ${EXIT_ERROR}
+	fi
+
+	if ! hook_parse_cmdline $@; then
+		# Return an error if the parsing of the cmd line fails
+		return ${EXIT_ERROR}
+	fi
+
+	# Write the settings to the config file
+	if ! zone_config_settings_write "${zone}" "${hook}" ${id}; then
+		log ERROR "Could not write config settings file ${config} for ${zone}"
+		return ${EXIT_ERROR}
+	fi
+
+	# Bring the config up
+	if device_exists ${zone}; then
+		if ! hook_config_cmd "up" "${zone}" "${hook}" "${hook}.${id}"; then
+			log ERROR "Could not bring config ${config} up for zone ${zone}"
+			return ${EXIT_ERROR}
+		fi
+	fi
+
+	exit ${EXIT_OK}
+}
-- 
2.6.3

Re: [PATCH 1/5] zone: fix zone_config()

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 1086 bytes --]

Hi,

I merged this patchset, but there is a few comments left.

Maybe you can have a look at them and if you think it suits, please
send some patches to get rid of any problems.

-Michael

On Mon, 2017-07-17 at 17:24 +0200, Jonatan Schlag wrote:
> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> ---
>  src/functions/functions.zone | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/functions/functions.zone
> b/src/functions/functions.zone
> index acf68f5..11a8dc2 100644
> --- a/src/functions/functions.zone
> +++ b/src/functions/functions.zone
> @@ -565,9 +565,9 @@ zone_config() {
>  			# TODO This could be also a valid hid
>  			local id=${cmd}
>  
> -			if zone_config_id_is valid ${zone} ${id} &&
> [[ ${1} == "edit" ]]; then
> +			if zone_config_id_is_valid ${zone} ${id} &&
> [[ ${1} == "edit" ]]; then
>  				shift 1
> -				zone_config_edit "${zone}"
> "${id}""$@"
> +				zone_config_edit "${zone}" "${id}"
> "$@"
>  			else
>  				error "Unrecognized argument:
> ${cmd}"
>  				cli_usage root-zone-config-
> subcommands

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/5] header-zone fix hook_config_edit()

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 712 bytes --]

On Mon, 2017-07-17 at 17:24 +0200, Jonatan Schlag wrote:
> This function accepted only two arguments so new cmd coudl be passed.
> We accept now more then 2 arguments.
> 
> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> ---
>  src/header-zone | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/header-zone b/src/header-zone
> index 2e3fa09..98b0ce7 100644
> --- a/src/header-zone
> +++ b/src/header-zone
> @@ -225,7 +225,7 @@ hook_config_destroy() {
>  }
>  
>  hook_config_edit() {
> -	assert [ $# -eq 2 ]
> +	assert [ $# -ge 2 ]
>  	local zone=${1}
>  	# The id must be the id and not the hid.
>  	local id=${2}

Please have a blank line after any assertions.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 4/5] config: add new functions

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 1403 bytes --]

Hi,

On Mon, 2017-07-17 at 17:24 +0200, Jonatan Schlag wrote:
> This patch add two new functions:
> config_get_id_from_config()
> config_get_hook_from_config
> 
> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> ---
>  src/functions/functions.config | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/src/functions/functions.config
> b/src/functions/functions.config
> index 854f490..e11a1c2 100644
> --- a/src/functions/functions.config
> +++ b/src/functions/functions.config
> @@ -51,3 +51,23 @@ config_domainname() {
>  	# the domain part.
>  	print "${hostname#*.}"
>  }
> +
> +config_get_id_from_config() {
> +	# This function returns the id for a given config name
> +	# Example 'dhcp.0' => 0
> +	assert [ $# -eq 1 ]
> +	local config=${1}
> +
> +	local hook=$(config_get_hook_from_config ${config})
> +	echo "${config//"${hook}."/}"
> +
> +}

There is an extra empty line.

And calling config_get_hook_from_config() is an expensive call. It
forks a subshell and loads the hook and so on.

I am sure that you can find the ID without knowing what is coming
before. That would increase performance of this function tremendously.

> +
> +config_get_hook_from_config() {
> +	# This function returns the hook for a given config name
> +	# Example 'dhcp.0' => dhcp
> +	assert [ $# -eq 1 ]
> +	local config=${1}
> +
> +	echo "${config//.*[[:digit:]]/}"
> +}

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 4/5] config: add new functions

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 1655 bytes --]

On Mon, 2017-07-17 at 16:22 -0400, Michael Tremer wrote:
> Hi,
> 
> On Mon, 2017-07-17 at 17:24 +0200, Jonatan Schlag wrote:
> > This patch add two new functions:
> > config_get_id_from_config()
> > config_get_hook_from_config
> > 
> > Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> > ---
> >  src/functions/functions.config | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/src/functions/functions.config
> > b/src/functions/functions.config
> > index 854f490..e11a1c2 100644
> > --- a/src/functions/functions.config
> > +++ b/src/functions/functions.config
> > @@ -51,3 +51,23 @@ config_domainname() {
> >  	# the domain part.
> >  	print "${hostname#*.}"
> >  }
> > +
> > +config_get_id_from_config() {
> > +	# This function returns the id for a given config name
> > +	# Example 'dhcp.0' => 0
> > +	assert [ $# -eq 1 ]
> > +	local config=${1}
> > +
> > +	local hook=$(config_get_hook_from_config ${config})
> > +	echo "${config//"${hook}."/}"
> > +
> > +}
> 
> There is an extra empty line.
> 
> And calling config_get_hook_from_config() is an expensive call. It
> forks a subshell and loads the hook and so on.
> 
> I am sure that you can find the ID without knowing what is coming
> before. That would increase performance of this function
> tremendously.

Actually I am wrong here. This is the function below :)

Still you can avoid calling it...

> 
> > +
> > +config_get_hook_from_config() {
> > +	# This function returns the hook for a given config name
> > +	# Example 'dhcp.0' => dhcp
> > +	assert [ $# -eq 1 ]
> > +	local config=${1}
> > +
> > +	echo "${config//.*[[:digit:]]/}"
> > +}

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]