luci-app-lxc: code cleanup

Number of suggested rewrites:
* use lua ubus bindings instead of os.execute
* combine multiple actions to use same handler 'lxc_action' and pass
actions as arguments to consolidate code
* read openwrt version from lua directly
* start using String.prototype.format for string formatting

Also, drop 'rename' and 'clone' until they are fully functional on all
platforms.

Thanks Jo-Philipp for suggestions on how to improve the code.

Signed-off-by: Petar Koretic <petar.koretic@sartura.hr>

2 files changed, 68 insertions, 158 deletions

diff --git a/utils/luci-app-lxc/files/controller/lxc.lua b/utils/luci-app-lxc/files/controller/lxc.lua
index 8172fb9c0..ce7b53e6e 100644
--- a/utils/luci-app-lxc/files/controller/lxc.lua
+++ b/utils/luci-app-lxc/files/controller/lxc.lua
@@ -16,6 +16,37 @@ Author: Petar Koretic <petar.koretic@sartura.hr>
 
 module("luci.controller.lxc", package.seeall)
 
+require "ubus"
+local conn = ubus.connect()
+if not conn then
+    error("Failed to connect to ubus")
+end
+
+
+function fork_exec(command)
+	local pid = nixio.fork()
+	if pid > 0 then
+		return
+	elseif pid == 0 then
+		-- change to root dir
+		nixio.chdir("/")
+
+		-- patch stdin, out, err to /dev/null
+		local null = nixio.open("/dev/null", "w+")
+		if null then
+			nixio.dup(null, nixio.stderr)
+			nixio.dup(null, nixio.stdout)
+			nixio.dup(null, nixio.stdin)
+			if null:fileno() > 2 then
+				null:close()
+			end
+		end
+
+		-- replace with target command
+		nixio.exec("/bin/sh", "-c", command)
+	end
+end
+
 function index()
 	page = node("admin", "services", "lxc")
 	page.target = cbi("lxc")
@@ -25,31 +56,7 @@ function index()
 	page = entry({"admin", "services", "lxc_create"}, call("lxc_create"), nil)
 	page.leaf = true
 
-	page = entry({"admin", "services", "lxc_stop"}, call("lxc_stop"), nil)
-	page.leaf = true
-
-	page = entry({"admin", "services", "lxc_start"}, call("lxc_start"), nil)
-	page.leaf = true
-
-	page = entry({"admin", "services", "lxc_reboot"}, call("lxc_reboot"), nil)
-	page.leaf = true
-
-	page = entry({"admin", "services", "lxc_delete"}, call("lxc_delete"), nil)
-	page.leaf = true
-
-	page = entry({"admin", "services", "lxc_list"}, call("lxc_list"), nil)
-	page.leaf = true
-
-	page = entry({"admin", "services", "lxc_rename"}, call("lxc_rename"), nil)
-	page.leaf = true
-
-	page = entry({"admin", "services", "lxc_clone"}, call("lxc_clone"), nil)
-	page.leaf = true
-
-	page = entry({"admin", "services", "lxc_freeze"}, call("lxc_freeze"), nil)
-	page.leaf = true
-
-	page = entry({"admin", "services", "lxc_unfreeze"}, call("lxc_unfreeze"), nil)
+	page = entry({"admin", "services", "lxc_action"}, call("lxc_action"), nil)
 	page.leaf = true
 
 	page = entry({"admin", "services", "lxc_configuration_get"}, call("lxc_configuration_get"), nil)
@@ -67,87 +74,23 @@ function lxc_create(lxc_name, lxc_template)
 
 	local url = uci:get("lxc", "lxc", "url")
 
-	local f = io.popen([[grep DISTRIB_TARGET /etc/openwrt_release | awk -F"[/'']" '{ print $2 }']])
-	if not f then
+	if not pcall(dofile, "/etc/openwrt_release") then
 		return luci.http.write("1")
 	end
 
-	local target = f:read("*all")
+	local target = _G.DISTRIB_TARGET:match('([^/]+)')
 
-	local res = os.execute("lxc-create -t download -n " .. lxc_name .. " -- --server=" .. url .. " --no-validate --dist openwrt --release bb --arch " .. target)
+	local res = os.execute("lxc-create -t download -n " .. lxc_name .. " -- --server=" .. url .. " --no-validate --dist " .. lxc_template .. " --release bb --arch " .. target)
 
 	luci.http.write(tostring(res))
 end
 
-function lxc_start(lxc_name)
-	luci.http.prepare_content("text/plain")
-
-	local res = os.execute("ubus call lxc start '{\"name\" : \"" .. lxc_name .. "\"}' ")
-
-	luci.http.write(tostring(res))
-end
-
-function lxc_stop(lxc_name)
-	luci.http.prepare_content("text/plain")
-
-	local res = os.execute("ubus call lxc stop '{\"name\" : \"" .. lxc_name .. "\"}' ")
-
-	luci.http.write(tostring(res))
-end
-
-function lxc_delete(lxc_name)
-	luci.http.prepare_content("text/plain")
-
-	os.execute("ubus call lxc stop '{\"name\" : \"" .. lxc_name .. "\"}' ")
-	local res = os.execute("ubus call lxc destroy '{\"name\" : \"" .. lxc_name .. "\"}' ")
-
-	luci.http.write(tostring(res))
-end
-
-function lxc_reboot(lxc_name)
-	luci.http.prepare_content("text/plain")
-
-	local res = os.execute("ubus call lxc reboot '{\"name\" : \"" .. lxc_name .. "\"}' ")
-
-	luci.http.write(tostring(res))
-end
-
-function lxc_rename(lxc_name_cur, lxc_name_new)
-	luci.http.prepare_content("text/plain")
-
-	local res = os.execute("ubus call lxc rename '{\"name\" : \"" .. lxc_name_cur .. "\", \"newname\" : \"" .. lxc_name_new .. "\"}' ")
-
-	luci.http.write(tostring(res))
-end
-
-function lxc_freeze(lxc_name)
-	luci.http.prepare_content("text/plain")
-
-	local res = os.execute("ubus call lxc freeze '{\"name\" : \"" .. lxc_name .. "\"}' ")
-
-	luci.http.write(tostring(res))
-end
-
-function lxc_unfreeze(lxc_name)
-	luci.http.prepare_content("text/plain")
-
-	local res = os.execute("ubus call lxc unfreeze '{\"name\" : \"" .. lxc_name .. "\"}' ")
-
-	luci.http.write(tostring(res))
-end
-
-function lxc_list()
+function lxc_action(lxc_action, lxc_name)
 	luci.http.prepare_content("application/json")
 
-	local cmd = io.popen("ubus call lxc list")
-	if not cmd then
-		return luci.http.write("{}")
-	end
-
-	local res = cmd:read("*all")
-	cmd:close()
+	local data, ec = conn:call("lxc", lxc_action, lxc_name and { name = lxc_name} or {} )
 
-	luci.http.write(res)
+	luci.http.write_json(ec and {} or data)
 end
 
 function lxc_configuration_get(lxc_name)
diff --git a/utils/luci-app-lxc/files/view/lxc.htm b/utils/luci-app-lxc/files/view/lxc.htm
index 1d38721da..1376968e3 100644
--- a/utils/luci-app-lxc/files/view/lxc.htm
+++ b/utils/luci-app-lxc/files/view/lxc.htm
@@ -98,7 +98,7 @@ Author: Petar Koretic <petar.koretic@sartura.hr>
 
 		loading(loader_add)
 
-		new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_create/' + lxc_name + "/" + lxc_template , null,
+		new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_create/' + '%h/%h'.format(lxc_name, lxc_template) , null,
 		function(x)
 		{
 			bt_create.disabled = false
@@ -118,7 +118,7 @@ Author: Petar Koretic <petar.koretic@sartura.hr>
 		var actions = ''
 		actions += '<input type="button" onclick="action_handler(this)" data-action="start" value="<%:Start%>" class="cbi-button cbi-button-apply" />'
 		actions+= '<input type="button" onclick="action_handler(this)" data-action="stop" value="<%:Stop%>" class="cbi-button cbi-button-reset" />'
-		actions+= '<input type="button" onclick="action_handler(this)" data-action="delete" value="<%:Delete%>" class="cbi-button cbi-button-remove" />'
+		actions+= '<input type="button" onclick="action_handler(this)" data-action="destroy" value="<%:Delete%>" class="cbi-button cbi-button-remove" />'
 		actions+= ' <select class="cbi-input-select cbi-button" onchange="action_more_handler(this)">\
 						<option selected disabled>more</option>\
 						<option>configure</option>\
@@ -130,7 +130,7 @@ Author: Petar Koretic <petar.koretic@sartura.hr>
 
 		var row = t_lxc_list.insertRow(-1)
 		var cell = row.insertCell(-1)
-		cell.innerHTML = "<strong>" + lxc_name + "</strong>"
+		cell.innerHTML = '%q%h%q'.format("<strong>", lxc_name, "</strong>")
 		cell.width = "30%"
 		cell.setAttribute("data-id", lxc_name)
 
@@ -158,14 +158,14 @@ Author: Petar Koretic <petar.koretic@sartura.hr>
 		{
 			loading(loader)
 
-			new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_stop/' + lxc_name, null,
-			function(x)
+			new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_action/' + '%h/%h'.format(action, lxc_name), null,
+			function(x, ec)
 			{
 				loading(loader, 0)
 				bt_action.disabled = false
 
-				if (!x || x.responseText != "0")
-					return info_message(output_list,"Invalid response from system!")
+				if (!x || ec)
+					return info_message(output_list,"Action failed!")
 
 				set_status(status_img, "red")
 
@@ -176,36 +176,36 @@ Author: Petar Koretic <petar.koretic@sartura.hr>
 		{
 			loading(loader)
 
-			new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_start/' + lxc_name, null,
-			function(x)
+			new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_action/' + '%h/%h'.format(action, lxc_name), null,
+			function(x, data)
 			{
 				loading(loader, 0)
 				bt_action.disabled = false
 
 				//FIXME: uncomment after fixing 'lxc-start'
-				if (!x /*|| x.responseText != "0"*/)
-					return info_message(output_list,"Invalid response from system!")
+				if (!x /*|| ec */)
+					return info_message(output_list,"Action failed!")
 
 				//FIXME: uncomment after fixing 'lxc-start'
 				//set_status(status_img, "green")
 			});
 		}
 
-		else if (action == "delete")
+		else if (action == "destroy")
 		{
 			if (!confirm("This will completely remove LXC container from the disk. Are you sure? (container will be stopped if running)"))
 				return
 
 			loading(loader)
 
-			new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_delete/' + lxc_name, null,
-			function(x)
+			new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_action/' + '%h/%h'.format(action, lxc_name), null,
+			function(x, ec)
 			{
 				loading(loader, 0)
 				bt_action.disabled = false
 
-				if (!x || x.responseText != "0")
-					return info_message(output_list,"Invalid response from system!")
+				if (!x || ec)
+					return info_message(output_list,"Action failed!")
 
 				var row = self.parentNode.parentNode
 				row.parentNode.removeChild(row)
@@ -214,26 +214,6 @@ Author: Petar Koretic <petar.koretic@sartura.hr>
 		}
 	}
 
-	function lxc_rename_handler(self)
-	{
-		var td = self.parentNode
-		var lxc_name_holder = td.querySelector('[data-id]')
-		var lxc_name_cur = lxc_name_holder.getAttribute('data-id')
-		var lxc_name_new = lxc_name_holder.value
-
-		if (t_lxc_list.querySelector("[data-id='" + lxc_name_new + "']") != null)
-			return info_message(output_list, "Container with new name already exists!", 4000)
-
-		new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_rename/' + lxc_name_cur + '/' + lxc_name_new, null,
-		function(x)
-		{
-			if (!x || x.responseText != "0")
-				return info_message(output_list,"Invalid response from system!")
-
-			info_message(output_list,"LXC renamed")
-		})
-	}
-
 	function lxc_configure_handler(self)
 	{
 		var td = self.parentNode
@@ -245,7 +225,7 @@ Author: Petar Koretic <petar.koretic@sartura.hr>
 		function(x)
 		{
 			if (!x || x.responseText != "0")
-				return info_message(output_list,"Invalid response from system!")
+				return info_message(output_list,"Action failed!")
 
 			info_message(output_list,"LXC configuration updated")
 			var row = td.parentNode
@@ -299,19 +279,6 @@ Author: Petar Koretic <petar.koretic@sartura.hr>
 
 			break
 
-			case "rename":
-				var tr = document.createElement('tr')
-				var row = self.parentNode.parentNode
-				var next_row = row.nextSibling
-				if (next_row && next_row.getAttribute('data-action') !== null)
-					row.parentNode.removeChild(next_row)
-
-				tr.innerHTML="<td colspan='" + row.cells.length + "'>" + lxc_rename_template(lxc_name) + "</td>"
-				tr.setAttribute('data-action','')
-				row.parentNode.insertBefore(tr, row.nextSibling)
-
-				break
-
 			case "freeze":
 				var tr = self.parentNode.parentNode
 				var img = tr.querySelector('img')
@@ -319,12 +286,12 @@ Author: Petar Koretic <petar.koretic@sartura.hr>
 					return info_message(output_list,"Container is not running!")
 
 				loading(loader)
-				new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_freeze/' + lxc_name, null,
-				function(x)
+				new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_action/' + '%h/%h'.format(option, lxc_name), null,
+				function(x, ec)
 				{
 					loading(loader, 0)
-					if (!x || x.responseText != "0")
-						return info_message(output_list,"Invalid response from system!")
+					if (!x || ec)
+						return info_message(output_list,"Action failed!")
 
 					set_status(img, "purple")
 				})
@@ -339,12 +306,12 @@ Author: Petar Koretic <petar.koretic@sartura.hr>
 					return info_message(output_list,"Container is not frozen!")
 
 				loading(loader)
-				new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_unfreeze/' + lxc_name, null,
-				function(x)
+				new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_action/' + '%h/%h'.format(option, lxc_name), null,
+				function(x, ec)
 				{
 					loading(loader, 0)
-					if (!x || x.responseText != "0")
-						return info_message(output_list,"Invalid response from system!")
+					if (!x || ec)
+						return info_message(output_list,"Action failed!")
 
 					set_status(img, "green")
 				})
@@ -361,12 +328,12 @@ Author: Petar Koretic <petar.koretic@sartura.hr>
 					return
 
 				loading(loader)
-				new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_reboot/' + lxc_name, null,
-				function(x)
+				new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_action/' + '%h/%h'.format(option, lxc_name), null,
+				function(x, ec)
 				{
 					loading(loader, 0)
-					if (!x || x.responseText != "0")
-						return info_message(output_list,"Invalid response from system!")
+					if (!x || ec)
+						return info_message(output_list,"Action failed!")
 
 					info_message(output_list,"LXC rebooted")
 				})
@@ -392,7 +359,7 @@ Author: Petar Koretic <petar.koretic@sartura.hr>
 
 	function lxc_list_update()
 	{
-		XHR.poll(4, '<%=luci.dispatcher.build_url("admin", "services")%>/lxc_list', null,
+		XHR.poll(4, '<%=luci.dispatcher.build_url("admin", "services")%>/lxc_action/list', null,
 		function(x, data)
 		{
 			if (!x) return;