Hello,
for this entire patchset I have a few coding style comments on top of a few questions to clarify a few of the functions for me.
On Fri, 2017-06-09 at 12:17 +0200, Jonatan Schlag wrote:
The configuration of a port was stored in a file called: /etc/network/${port} This is bad because it is very hard to add further information which belong primary not to the configuration to this file.
So we change the settings file to /etc/network/${port}/settings like for the zones.
This make it possible to store other configurations like the color in other files in the directory /etc/network/${port}.
So this is quite a huge change. Ideally we would need to migrate configuration, but given that we are only at alpha stage, it might be okay to break compatibility a bit. I hope I won't regret saying any of this soon.
A workaround to move the config file into the new directory scheme is: port=p1 && mv /etc/network/ports/${port} /etc/network/ports/${port}- save \ && mkdir -p /etc/network/ports/${port} \ && mv /etc/network/ports/${port}-save /etc/network/ports/${port}/settings
where port is the name of the port like p1 or p0.
Signed-off-by: Jonatan Schlag jonatan.schlag@ipfire.org
src/functions/functions.ports | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/functions/functions.ports b/src/functions/functions.ports index 40f4eae..b138238 100644 --- a/src/functions/functions.ports +++ b/src/functions/functions.ports @@ -20,12 +20,13 @@ #################################################################### ########### port_dir() {
- echo "${NETWORK_CONFIG_DIR}/ports"
- local port="${1}"
- echo "${NETWORK_CONFIG_DIR}/ports/${port}"
} port_list() { local port
- for port in $(port_dir)/*; do
- for port in $(port_dir)*; do
port="$(basename "${port}")" if port_exists "${port}"; then print "${port}"
Not really sure if this works as it should. I think the original version was okay. It would probably be better to not call the port_dir function here at all.
@@ -113,13 +114,13 @@ port_file() { local port="${1}" assert isset port
- echo "$(port_dir)/${port}"
- echo "$(port_dir ${port})/settings"
} port_exists() { local port=${1}
- [ -f "${NETWORK_CONFIG_DIR}/ports/${port}" ]
- [ -d "${NETWORK_CONFIG_DIR}/ports/${port}" ]
} port_get_hook() { @@ -207,7 +208,7 @@ port_destroy() { port_remove "${port}"
- rm -f $(port_file ${port})
- rm -rf $(port_dir ${port})
} port_create() { @@ -263,7 +264,7 @@ port_cmd() { ports_get() { local port
- for port in $(port_dir)/*; do
- for port in $(port_dir)*; do
port=$(basename ${port}) if port_exists ${port}; then echo "${port}"
Same as above.
-Michael