From ac694a6ab3af6fb2a7cce13e8cb933578db7b8af Mon Sep 17 00:00:00 2001
From: Michael Tremer <michael.tremer@ipfire.org>
Date: Sat, 27 Dec 2014 23:57:26 +0000
Subject: [PATCH] Make zone handling nicer

Zones are now automatically activated after they have been
created and attaching and detaching ports is now just called that.
---
 src/functions/functions.hook |   2 +-
 src/functions/functions.zone | 283 +++++++++++++++++++++--------------
 src/header-zone              |   6 +-
 src/hooks/zones/bridge       |  74 +++++----
 src/hooks/zones/pppoe        |  35 +++--
 src/network                  |  49 +++++-
 6 files changed, 282 insertions(+), 167 deletions(-)

diff --git a/src/functions/functions.hook b/src/functions/functions.hook
index a48a86d4..3d79ed22 100644
--- a/src/functions/functions.hook
+++ b/src/functions/functions.hook
@@ -248,7 +248,7 @@ function hook_valid_command_zone() {
 			;;
 
 		# Ports
-		port_add|port_edit|port_create|port_remove|port_show|port_status|port_up|port_down)
+		port_attach|port_detach|port_edit|port_create|port_remove|port_show|port_status|port_up|port_down)
 			return ${EXIT_TRUE}
 			;;
 
diff --git a/src/functions/functions.zone b/src/functions/functions.zone
index 04b034d6..e39248c6 100644
--- a/src/functions/functions.zone
+++ b/src/functions/functions.zone
@@ -250,6 +250,9 @@ function zone_new() {
 
 	# Automatically enable zone.
 	zone_enable "${zone}"
+
+	# Bring up the zone immediately after
+	zone_start "${zone}"
 }
 
 function zone_edit() {
@@ -411,105 +414,6 @@ function zone_status() {
 	fi
 }
 
-function zone_port() {
-	local zone=${1}
-	local action=${2}
-	shift 2
-
-	assert isset zone
-	assert isset action
-	assert zone_exists ${zone}
-
-	case "${action}" in
-		add|edit|remove)
-			zone_port_${action} ${zone} $@
-			;;
-		*)
-			error "Unrecognized argument: ${action}"
-			cli_usage root-zone-port-subcommands
-			exit ${EXIT_ERROR}
-			;;		
-	esac
-}
-
-function zone_port_add() {
-	local zone="${1}"
-	assert isset zone
-
-	local port="${2}"
-	assert isset port
-
-	shift 2
-
-	# Check if the port actually exists.
-	if ! port_exists "${port}"; then
-		error "Cannot add port '${port}' which does not exist"
-		return ${EXIT_ERROR}
-	fi
-
-	# Check if the port is already connected to this or any other zone.
-	local z
-	for z in $(zones_get_all); do
-		if zone_has_port "${z}" "${port}"; then
-			error "Port '${port}' is already assigned to zone '${z}'"
-			return ${EXIT_ERROR}
-		fi
-	done
-
-	local hook=$(zone_get_hook "${zone}")
-	assert isset hook
-
-	hook_zone_exec "${hook}" "port_add" "${zone}" "${port}" "$@"
-}
-
-function zone_port_edit() {
-	local zone="${1}"
-	assert isset zone
-
-	local port="${2}"
-	assert isset port
-
-	shift 2
-
-	# Check if the port actually exists.
-	if ! port_exists "${port}"; then
-		error "Port '${port}' does not exist"
-		return ${EXIT_ERROR}
-	fi
-
-	# Check if the zone actually has this port.
-	if ! zone_has_port "${zone}" "${port}"; then
-		error "Port '${port}' is not attached to zone '${zone}'"
-		return ${EXIT_ERROR}
-	fi
-
-	local hook=$(zone_get_hook "${zone}")
-	assert isset hook
-
-	hook_zone_exec "${hook}" "port_edit" "${zone}" "${port}" "$@"
-}
-
-function zone_port_remove() {
-	local zone="${1}"
-	assert isset zone
-
-	local port="${2}"
-	assert isset port
-
-	shift 2
-
-	# Check if the zone actually has this port.
-	if ! zone_has_port "${zone}" "${port}"; then
-		error "Port '${port}' is not attached to zone '${zone}'"
-		return ${EXIT_ERROR}
-	fi
-
-	local hook=$(zone_get_hook "${zone}")
-	assert isset hook
-
-	hook_zone_exec "${hook}" "port_remove" "${zone}" "${port}" "$@"
-}
-
 function zone_get_ports() {
 	local zone=${1}
 
@@ -712,21 +616,171 @@ function zone_ports_list() {
 	done
 }
 
-function zone_ports_cmd() {
-	local cmd=${1}
-	local zone=${2}
+function zone_port_attach() {
+	local zone="${1}"
+	assert isset zone
+
+	local port="${2}"
+	assert isset port
+
+	shift 2
+
+	# Check if the port actually exists.
+	if ! port_exists "${port}"; then
+		error "Cannot attach port '${port}' which does not exist"
+		return ${EXIT_ERROR}
+	fi
+
+	# Check if the port is already connected to this or any other zone.
+	local z
+	for z in $(zones_get_all); do
+		if zone_has_port "${z}" "${port}"; then
+			error "Port '${port}' is already attached to zone '${z}'"
+			return ${EXIT_ERROR}
+		fi
+	done
+
+	local hook="$(zone_get_hook "${zone}")"
+	assert isset hook
+
+	hook_zone_exec "${hook}" "port_attach" "${zone}" "${port}" "$@"
+	local ret="${?}"
+
+	case "${ret}" in
+		${EXIT_OK})
+			log INFO "${port} has been attached to ${zone}"
+
+			# Automatically connect the port
+			if zone_is_active "${zone}"; then
+				zone_port_create "${zone}" "${port}"
+			fi
+			;;
+		*)
+			log CRITICAL "${port} could not be attached to ${zone}"
+			;;
+	esac
+
+	return ${ret}
+}
+
+function zone_port_edit() {
+	local zone="${1}"
+	assert isset zone
+
+	local port="${2}"
+	assert isset port
+
+	shift 2
+
+	# Check if the port actually exists.
+	if ! port_exists "${port}"; then
+		error "Port '${port}' does not exist"
+		return ${EXIT_ERROR}
+	fi
+
+	# Check if the zone actually has this port.
+	if ! zone_has_port "${zone}" "${port}"; then
+		error "Port '${port}' is not attached to zone '${zone}'"
+		return ${EXIT_ERROR}
+	fi
+
+	local hook=$(zone_get_hook "${zone}")
+	assert isset hook
+
+	hook_zone_exec "${hook}" "port_edit" "${zone}" "${port}" "$@"
+}
+
+function zone_port_detach() {
+	local zone="${1}"
+	assert isset zone
+
+	local port="${2}"
+	assert isset port
+
 	shift 2
 
+	# Check if the zone actually has this port.
+	if ! zone_has_port "${zone}" "${port}"; then
+		error "Port '${port}' is not attached to zone '${zone}'"
+		return ${EXIT_ERROR}
+	fi
+
+	local hook=$(zone_get_hook "${zone}")
+	assert isset hook
+
+	hook_zone_exec "${hook}" "port_detach" "${zone}" "${port}" "$@"
+	local ret="${?}"
+
+	case "${ret}" in
+		${EXIT_OK})
+			log INFO "${port} has been detached from ${zone}"
+
+			# Bring down the port if needed
+			if zone_is_active "${zone}"; then
+				zone_port_remove "${zone}" "${port}"
+			fi
+			;;
+		*)
+			log CRITICAL "${port} could not be detached from ${zone}"
+			;;
+	esac
+
+	return ${ret}
+}
+
+function zone_port_cmd() {
+	local cmd="${1}"
 	assert isset cmd
+
+	local zone="${2}"
 	assert isset zone
 
-	assert zone_exists ${zone}
+	local port="${3}"
+	assert isset port
 
-	local hook=$(zone_get_hook ${zone})
+	shift 3
+
+	local hook="$(zone_get_hook "${zone}")"
+	assert isset hook
+
+	# Dispatch command to hook
+	hook_zone_exec "${hook}" "${cmd}" "${zone}" "${port}" $@
+}
+
+function zone_port_create() {
+	zone_port_cmd "port_create" $@
+}
+
+function zone_port_remove() {
+	zone_port_cmd "port_remove" $@
+}
+
+function zone_port_up() {
+	zone_port_cmd "port_up" $@
+}
+
+function zone_port_down() {
+	zone_port_cmd "port_down" $@
+}
+
+function zone_port_status() {
+	zone_port_cmd "port_status" $@
+}
+
+function zone_ports_cmd() {
+	local cmd="${1}"
+	assert isset cmd
+
+	local zone="${2}"
+	assert isset zone
+
+	shift 2
+
+	local hook="$(zone_get_hook "${zone}")"
 
 	local port
 	for port in $(zone_get_ports ${zone}); do
-		hook_zone_exec ${hook} ${cmd} ${zone} ${port} $@
+		hook_zone_exec "${hook}" "${cmd}" "${zone}" "${port}" $@
 	done
 }
 
@@ -739,15 +793,15 @@ function zone_ports_remove() {
 }
 
 function zone_ports_up() {
-	zone_ports_cmd port_up $@
+	zone_ports_cmd "port_up" $@
 }
 
 function zone_ports_down() {
-	zone_ports_cmd port_down $@
+	zone_ports_cmd "port_down" $@
 }
 
 function zone_ports_status() {
-	zone_ports_cmd port_status $@
+	zone_ports_cmd "port_status" $@
 }
 
 function zone_configs_cmd() {
@@ -943,14 +997,21 @@ function zone_config_settings_write() {
 }
 
 function zone_port_settings_read() {
-	assert [ $# -gt 2 ]
+	assert [ $# -ge 2 ]
 
 	local zone="${1}"
 	local port="${2}"
 	shift 2
 
+	local args
+	if [ $# -eq 0 ] && [ -n "${HOOK_PORT_SETTINGS}" ]; then
+		list_append args ${HOOK_PORT_SETTINGS}
+	else
+		list_append args $@
+	fi
+
 	local path="$(zone_dir "${zone}")/ports/${port}"
-	settings_read "${path}" "$@"
+	settings_read "${path}" ${args}
 }
 
 function zone_port_settings_write() {
@@ -964,7 +1025,7 @@ function zone_port_settings_write() {
 	if function_exists "hook_check_port_settings"; then
 		list_append args "--check=\"hook_check_port_settings\""
 	fi
-	list_append args $@
+	list_append args ${HOOK_PORT_SETTINGS}
 
 	local path="$(zone_dir "${zone}")/ports/${port}"
 	settings_write "${path}" ${args}
diff --git a/src/header-zone b/src/header-zone
index 1f3a6612..f8a06d84 100644
--- a/src/header-zone
+++ b/src/header-zone
@@ -102,15 +102,15 @@ function hook_port() {
 	exit ${ret}
 }
 
-function hook_port_add() {
+function hook_port_attach() {
 	return ${EXIT_NOT_SUPPORTED}
 }
 
-function hook_port_edit() {
+function hook_port_detach() {
 	return ${EXIT_NOT_SUPPORTED}
 }
 
-function hook_port_destroy() {
+function hook_port_edit() {
 	return ${EXIT_NOT_SUPPORTED}
 }
 
diff --git a/src/hooks/zones/bridge b/src/hooks/zones/bridge
index b29c9680..c672e737 100644
--- a/src/hooks/zones/bridge
+++ b/src/hooks/zones/bridge
@@ -240,23 +240,17 @@ function hook_status() {
 	exit ${EXIT_OK}
 }
 
-function __parse_cmdline_args() {
-	while [ $# -gt 0 ]; do
-		case "${1}" in
-			--priority=*)
-				PRIORITY="$(cli_get_val ${1})"
-				;;
-			--cost=*)
-				COST="$(cli_get_val ${1})"
-				;;
-		esac
-		shift
-	done
+function hook_check_port_settings() {
+	if isset COST; then
+		assert isinteger COST
+	fi
 
-	return ${EXIT_OK}
+	if isset PRIORITY; then
+		assert isinteger PRIORITY
+	fi
 }
 
-function hook_port_add() {
+function hook_port_attach() {
 	# Excepting at least two arguments here
 	assert [ $# -ge 2 ]
 
@@ -264,36 +258,31 @@ function hook_port_add() {
 	local port="${2}"
 	shift 2
 
-	__parse_cmdline_args "$@"
-	[ $? -eq ${EXIT_OK} ] || return ${EXIT_ERROR}
-
-	zone_port_settings_write "${zone}" "${port}" ${HOOK_PORT_SETTINGS}
-
-	log INFO "Port '${port}' has been added to zone '${zone}'"
-
-	exit ${EXIT_OK}
-}
-
-function hook_port_edit() {
-	assert [ $# -ge 2 ]
-
-	local zone="${1}"
-	local port="${2}"
-	shift 2
-
-	zone_port_settings_read "${zone}" "${port}" ${HOOK_PORT_SETTINGS}
-
-	__parse_cmdline_args "$@"
-	[ $? -eq ${EXIT_OK} ] || return ${EXIT_ERROR}
+	if zone_has_port "${zone}" "${port}"; then
+		zone_port_settings_read "${zone}" "${port}"
+	fi
 
-	zone_port_settings_write "${zone}" "${port}" ${HOOK_PORT_SETTINGS}
+	local arg
+	local val
+	while read arg; do
+		case "${arg}" in
+			--cost=*)
+				COST="$(cli_get_val "${arg}")"
+				;;
+			--priority=*)
+				PRIORITY="$(cli_get_val "${arg}")"
+				;;
+		esac
+	done <<< "$(args $@)"
 
-	log INFO "Port '${port}' (member of zone '${zone}') has been edited"
+	if ! zone_port_settings_write "${zone}" "${port}"; then
+		exit ${EXIT_ERROR}
+	fi
 
 	exit ${EXIT_OK}
 }
 
-function hook_port_destroy() {
+function hook_port_detach() {
 	assert [ $# -eq 2 ]
 
 	local zone="${1}"
@@ -302,12 +291,17 @@ function hook_port_destroy() {
 	# Shut down the port (if possible)
 	port_down "${port}"
 
-	log INFO "Port '${port}' has been removed from zone '${zone}'"
-	zone_port_settings_remove "${zone}" "${port}"
+	if ! zone_port_settings_remove "${zone}" "${port}"; then
+		exit ${EXIT_ERROR}
+	fi
 
 	exit ${EXIT_OK}
 }
 
+function hook_port_edit() {
+	hook_port_attach $@
+}
+
 function hook_port_up() {
 	assert [ $# -eq 2 ]
 
diff --git a/src/hooks/zones/pppoe b/src/hooks/zones/pppoe
index 540b172f..100c8a76 100644
--- a/src/hooks/zones/pppoe
+++ b/src/hooks/zones/pppoe
@@ -105,10 +105,18 @@ function hook_up() {
 	local zone=${1}
 	assert isset zone
 
+	# If this zone's port is not set, we will return
+	# with EXIT_OK so that this zone will remain active,
+	# but we cannot start pppd.
+	local port=$(__hook_get_port "${zone}")
+	if ! isset port || ! port_exists "${port}"; then
+		log WARNING "Could not bring up zone '${zone}' because no port is attached"
+		exit ${EXIT_OK}
+	fi
+
 	zone_settings_read "${zone}"
 
 	# Bring up the port.
-	local port=$(__hook_get_port "${zone}")
 	port_up "${port}"
 
 	# Start the ppp daemon.
@@ -306,7 +314,7 @@ function __hook_get_port() {
 	return ${EXIT_ERROR}
 }
 
-function hook_port_add() {
+function hook_port_attach() {
 	# Excepting at least two arguments here
 	assert [ $# -ge 2 ]
 
@@ -317,29 +325,34 @@ function hook_port_add() {
 	# PPPoE can only use one port
 	local ports_num="$(zone_get_ports_num "${zone}")"
 	if [ ${ports_num} -ge 1 ]; then
-		local port=$(__hook_get_port "${zone}")
+		local ports="$(zone_get_ports "${zone}")"
 		error "The pppoe zone hook only supports assigning one port"
-		error "  port '${port}' has already been assigned to zone '${zone}'"
+		error "  port '${ports}' has already been assigned to zone '${zone}'"
 		return ${EXIT_ERROR}
 	fi
 
-	zone_port_settings_write "${zone}" "${port}"
-	log INFO "Port '${port}' has been added to zone '${zone}'"
+	if ! zone_port_settings_write "${zone}" "${port}"; then
+		exit ${EXIT_ERROR}
+	fi
 
 	exit ${EXIT_OK}
 }
 
-function hook_port_remove() {
+function hook_port_detach() {
 	assert [ $# -eq 2 ]
 
 	local zone="${1}"
 	local port="${2}"
 
-	# Shut down the port (if possible)
-	port_down "${port}"
+	# Shut down the entire zone here, because it cannot
+	# run without a port any way and removing the port would
+	# create a hotplug event which will be processed after the
+	# port has already been detached...
+	zone_stop "${zone}"
 
-	log INFO "Port '${port}' has been removed from zone '${zone}'"
-	zone_port_settings_remove "${zone}" "${port}"
+	if ! zone_port_settings_remove "${zone}" "${port}"; then
+		exit ${EXIT_ERROR}
+	fi
 
 	exit ${EXIT_OK}
 }
diff --git a/src/network b/src/network
index 27f16c67..3d3b31ab 100644
--- a/src/network
+++ b/src/network
@@ -526,7 +526,10 @@ function cli_zone() {
 		esac
 
 		case "${action}" in
-			config|disable|down|edit|enable|port|status|up)
+			port)
+				cli_zone_port "${zone}" $@
+				;;
+			config|disable|down|edit|enable|status|up)
 				zone_${action} ${zone} $@
 				;;
 			*)
@@ -585,6 +588,50 @@ function cli_zone_destroy() {
 	exit ${EXIT_OK}
 }
 
+function cli_zone_port() {
+	if cli_help_requested $@; then
+		cli_show_man network-zone-port
+		exit ${EXIT_OK}
+	fi
+
+	local zone="${1}"
+	assert zone_exists "${zone}"
+
+	if port_exists "${2}"; then
+		local port="${2}"
+		local action="${3}"
+		shift 3
+
+		case "${action}" in
+			edit)
+				zone_port_edit "${zone}" "${port}" $@
+				;;
+			*)
+				error "Unrecognised argument: ${action}"
+				exit ${EXIT_ERROR}
+				;;
+		esac
+	else
+		local action="${2}"
+		shift 2
+
+		case "${action}" in
+			attach)
+				zone_port_attach "${zone}" $@
+				;;
+			detach)
+				zone_port_detach "${zone}" $@
+				;;
+			*)
+				error "Unrecognised argument: ${action}"
+				exit ${EXIT_ERROR}
+				;;
+		esac
+	fi
+
+	exit ${EXIT_OK}
+}
+
 function cli_list_hooks() {
 	local type=${1}
 	shift
-- 
2.39.2