diff mbox series

[3/6] IPC: rework install IPC for future extensions

Message ID 20201111120327.607444-4-sbabic@denx.de
State Changes Requested
Headers show
Series Rework and extend IPC for install | expand

Commit Message

Stefano Babic Nov. 11, 2020, 12:03 p.m. UTC
The IPC was originally very simple and used just to send a SWU to
SWUpdate. Later, additional features were added to communicate with
internal SWUpdate's processes, like suricatta or the installer. The same
message for install was (mis)used, letting the interface quite confused.
Adding new commands is then difficult, because changes in the messages
mean changes in more subsystems.This patch addresses all these topics.

The message used to communicate with processes is renamed to "procmsg"
from "instmsg", but without changing its fields. A new "instmsg" is
created from scratch and this contains an "install request" structuire,
that the originator must fill with setup for the installation. Each
interface is updated to the new interface. The API ro external processes
is changed, but it will contain a field to track the API version and to
allow to have compatibility layers in future to avoid further breakages.
The "install request" is thought to add more information as in the past,
and to allow to change an installation per instance, while most
parameters are now set just at the startup via configuration file or
command line parameters.

Signed-off-by: Stefano Babic <sbabic@denx.de>
---
 core/network_thread.c          | 20 ++++--------
 core/stream_interface.c        |  8 ++---
 corelib/channel_curl.c         |  7 +++--
 corelib/downloader.c           |  1 +
 include/installer_priv.h       |  6 ++--
 include/network_ipc.h          | 40 ++++++++++++++----------
 ipc/network_ipc.c              | 57 ++++++++++++++++------------------
 mongoose/mongoose_interface.c  |  3 +-
 suricatta/server_hawkbit.c     | 14 ++++-----
 suricatta/suricatta.c          |  6 ++--
 tools/swupdate-hawkbitcfg.c    | 20 ++++++------
 tools/swupdate-sendtohawkbit.c | 12 +++----
 12 files changed, 97 insertions(+), 97 deletions(-)
diff mbox series

Patch

diff --git a/core/network_thread.c b/core/network_thread.c
index 2c383b7..a3c10a0 100644
--- a/core/network_thread.c
+++ b/core/network_thread.c
@@ -263,7 +263,7 @@  void *network_thread (void *data)
 			switch (msg.type) {
 			case POST_UPDATE:
 				if (postupdate(get_swupdate_cfg(),
-							   msg.data.instmsg.len > 0 ? msg.data.instmsg.buf : NULL) == 0) {
+							   msg.data.procmsg.len > 0 ? msg.data.procmsg.buf : NULL) == 0) {
 					msg.type = ACK;
 					sprintf(msg.data.msg, "Post-update actions successfully executed.");
 				} else {
@@ -279,14 +279,14 @@  void *network_thread (void *data)
 				 *  the payload
 				 */
 
-				pipe = pctl_getfd_from_type(msg.data.instmsg.source);
+				pipe = pctl_getfd_from_type(msg.data.procmsg.source);
 				if (pipe < 0) {
 					ERROR("Cannot find channel for requested process");
 					msg.type = NACK;
 					break;
 				}
 				TRACE("Received Message for %s",
-					pctl_getname_from_type(msg.data.instmsg.source));
+					pctl_getname_from_type(msg.data.procmsg.source));
 				if (fcntl(pipe, F_GETFL) < 0 && errno == EBADF) {
 					ERROR("Pipe not available or closed: %d", pipe);
 					msg.type = NACK;
@@ -316,10 +316,10 @@  void *network_thread (void *data)
 				FD_ZERO(&pipefds);
 				FD_SET(pipe, &pipefds);
 				tv.tv_usec = 0;
-				if (!msg.data.instmsg.timeout)
+				if (!msg.data.procmsg.timeout)
 					tv.tv_sec = DEFAULT_INTERNAL_TIMEOUT;
 				else
-					tv.tv_sec = msg.data.instmsg.timeout;
+					tv.tv_sec = msg.data.procmsg.timeout;
 				ret = select(pipe + 1, &pipefds, NULL, NULL, &tv);
 
 				/*
@@ -349,15 +349,7 @@  void *network_thread (void *data)
 				TRACE("Incoming network request: processing...");
 				if (instp->status == IDLE) {
 					instp->fd = ctrlconnfd;
-					instp->source = msg.data.instmsg.source;
-
-					/*
-					 * Communicate if a dry run is asked and set it
-					 */
-					if (msg.type == REQ_INSTALL_DRYRUN)
-						instp->dry_run = 1;
-					else
-						instp->dry_run = 0;
+					instp->req = msg.data.instmsg.req;
 
 					/*
 					 * Prepare answer
diff --git a/core/stream_interface.c b/core/stream_interface.c
index 633116d..e7298d9 100644
--- a/core/stream_interface.c
+++ b/core/stream_interface.c
@@ -500,7 +500,7 @@  void *network_initializer(void *data)
 		/*
 		 * Check if the dry run flag is overwritten
 		 */
-		if (inst.dry_run)
+		if (inst.req.dry_run)
 			software->globals.dry_run = 1;
 		else
 			software->globals.dry_run = 0;
@@ -606,9 +606,9 @@  void *network_initializer(void *data)
  */
 int get_install_info(sourcetype *source, char *buf, size_t len)
 {
-	len = min(len, inst.len);
-	memcpy(buf, inst.data, len);
-	*source = inst.source;
+	len = min(len - 1, strlen(inst.req.info));
+	strncpy(buf, inst.req.info, len);
+	*source = inst.req.source;
 
 	return len;
 }
diff --git a/corelib/channel_curl.c b/corelib/channel_curl.c
index 5c80770..bf39063 100644
--- a/corelib/channel_curl.c
+++ b/corelib/channel_curl.c
@@ -1040,8 +1040,11 @@  channel_op_res_t channel_get_file(channel_t *this, void *data)
 	struct swupdate_request req;
 	swupdate_prepare_req(&req);
 	req.dry_run = channel_data->dry_run;
-	req.len = channel_data->info == NULL ? 0 : strlen(channel_data->info);
-	req.info = channel_data->info;
+	req.source = channel_data->source;
+	if (channel_data->info) {
+		strncpy(req.info, channel_data->info,
+			  sizeof(req.info) - 1 );
+	}
 	for (int retries = 3; retries >= 0; retries--) {
 		file_handle = ipc_inst_start_ext(channel_data->source,
 			&req, sizeof(struct swupdate_request));
diff --git a/corelib/downloader.c b/corelib/downloader.c
index 462d379..cda5cb1 100644
--- a/corelib/downloader.c
+++ b/corelib/downloader.c
@@ -49,6 +49,7 @@  static RECOVERY_STATUS download_from_url(channel_data_t* channel_data)
 	TRACE("Image download started : %s", channel_data->url);
 
 	RECOVERY_STATUS result = SUCCESS;
+	channel_data->source = SOURCE_DOWNLOADER;
 	channel_op_res_t chanresult = channel->get_file(channel, channel_data);
 	if (chanresult != CHANNEL_OK) {
 		result = FAILURE;
diff --git a/include/installer_priv.h b/include/installer_priv.h
index 9322a68..1a7013a 100644
--- a/include/installer_priv.h
+++ b/include/installer_priv.h
@@ -9,6 +9,7 @@ 
 #define _INSTALLER_PRIV_H
 
 #include "swupdate_status.h"
+#include "network_ipc.h"
 
 struct installer {
 	int	fd;			/* install image file handle */
@@ -16,10 +17,7 @@  struct installer {
 	RECOVERY_STATUS	last_install;	/* result from last installation */
 	int	last_error;		/* error code if installation failed */
 	char	errormsg[64];		/* error message if installation failed */
-	sourcetype source; 		/* Who triggered the update */
-	int	dry_run;		/* set it if no changes in hardware must be done */
-	unsigned int len;    		/* Len of data valid in info, data is optional */
-	char	data[2048];   		/* This is a placeholder for installation request */
+	struct swupdate_request req;
 };
 
 #endif
diff --git a/include/network_ipc.h b/include/network_ipc.h
index 18da134..238709f 100644
--- a/include/network_ipc.h
+++ b/include/network_ipc.h
@@ -49,6 +49,21 @@  enum {
 	CMD_ENABLE	/* Enable or disable suricatta mode */
 };
 
+#define SWUPDATE_API_VERSION 	0x1
+/*
+ * Install structure to be filled before calling
+ * ipc and async functions
+ */
+struct swupdate_request {
+	unsigned int apiversion;
+	sourcetype source;
+	bool dry_run;
+	size_t len;
+	char info[512];
+	char software_set[256];
+	char running_mode[256];
+};
+
 typedef union {
 	char msg[128];
 	struct { 
@@ -57,6 +72,14 @@  typedef union {
 		int error;
 		char desc[2048];
 	} status;
+	struct {
+		struct swupdate_request req;
+		unsigned int len;    /* Len of data valid in buf */
+		char	buf[2048];   /*
+				      * Buffer that each source can fill
+				      * with additional information
+				      */
+	} instmsg;
 	struct {
 		sourcetype source; /* Who triggered the update */
 		int	cmd;	   /* Optional encoded command */
@@ -66,7 +89,7 @@  typedef union {
 				      * Buffer that each source can fill
 				      * with additional information
 				      */
-	} instmsg;
+	} procmsg;
 	struct {
 		char key_ascii[65]; /* Key size in ASCII (256 bit, 32 bytes bin) + termination */
 		char ivt_ascii[33]; /* Key size in ASCII (16 bytes bin) + termination */
@@ -79,21 +102,6 @@  typedef struct {
 	msgdata data;
 } ipc_message;
 
-#define SWUPDATE_API_VERSION 	0x1
-/*
- * Install structure to be filled before calling
- * ipc and async functions
- */
-struct swupdate_request {
-	unsigned int apiversion;
-	int type;
-	bool dry_run;
-	size_t len;
-	const char *info;
-	char *software_set;
-	char *running_mode;
-};
-
 char *get_ctrl_socket(void);
 int ipc_inst_start(void);
 int ipc_inst_start_ext(sourcetype source, void *priv, ssize_t size);
diff --git a/ipc/network_ipc.c b/ipc/network_ipc.c
index d817ffc..71bbb50 100644
--- a/ipc/network_ipc.c
+++ b/ipc/network_ipc.c
@@ -72,20 +72,20 @@  int ipc_postupdate(ipc_message *msg) {
 
 	ssize_t ret;
 	char* tmpbuf = NULL;
-	if (msg->data.instmsg.len > 0) {
-		if ((tmpbuf = strndupa(msg->data.instmsg.buf,
-				msg->data.instmsg.len > sizeof(msg->data.instmsg.buf)
-				    ? sizeof(msg->data.instmsg.buf)
-				    : msg->data.instmsg.len)) == NULL) {
+	if (msg->data.procmsg.len > 0) {
+		if ((tmpbuf = strndupa(msg->data.procmsg.buf,
+				msg->data.procmsg.len > sizeof(msg->data.procmsg.buf)
+				    ? sizeof(msg->data.procmsg.buf)
+				    : msg->data.procmsg.len)) == NULL) {
 			close(connfd);
 			return -1;
 		}
 	}
 	memset(msg, 0, sizeof(*msg));
 	if (tmpbuf != NULL) {
-		msg->data.instmsg.buf[sizeof(msg->data.instmsg.buf) - 1] = '\0';
-		strncpy(msg->data.instmsg.buf, tmpbuf, sizeof(msg->data.instmsg.buf) - 1);
-		msg->data.instmsg.len = strnlen(tmpbuf, sizeof(msg->data.instmsg.buf) - 1);
+		msg->data.procmsg.buf[sizeof(msg->data.procmsg.buf) - 1] = '\0';
+		strncpy(msg->data.procmsg.buf, tmpbuf, sizeof(msg->data.procmsg.buf) - 1);
+		msg->data.procmsg.len = strnlen(tmpbuf, sizeof(msg->data.procmsg.buf) - 1);
 	}
 	msg->magic = IPC_MAGIC;
 	msg->type = POST_UPDATE;
@@ -181,18 +181,28 @@  int ipc_inst_start_ext(sourcetype source, void *priv, ssize_t size)
 	int connfd;
 	ipc_message msg;
 	ssize_t ret;
-	bool dry_run = false;
-	struct swupdate_request *req = NULL;
-	size_t len = 0;
-	const char *buf = NULL;
+	struct swupdate_request *req;
+	struct swupdate_request localreq;
+
 
 	if (priv) {
+		/*
+		 * To be expanded: in future if more API will
+		 * be supported, a conversion will be take place
+		 * to send to the installer a single identifiable
+		 * request
+		 */
 		if (size != sizeof(struct swupdate_request))
 			return -EINVAL;
 		req = (struct swupdate_request *)priv;
-		dry_run = req->dry_run;
-		len = req->len;
-		buf = req->info;
+	} else {
+		/*
+		 * ensure that a valid install request
+		 * reaches the installer, add an empty
+		 * one with default values
+		 */
+		swupdate_prepare_req(&localreq);
+		req = &localreq;
 	}
 	connfd = prepare_ipc();
 	if (connfd < 0)
@@ -204,22 +214,9 @@  int ipc_inst_start_ext(sourcetype source, void *priv, ssize_t size)
 	 * Command is request to install
 	 */
 	msg.magic = IPC_MAGIC;
-	msg.type = (!dry_run) ? REQ_INSTALL : REQ_INSTALL_DRYRUN;
-
-	/*
-	 * Pass data from interface originating
-	 * the update, if any
-	 */
-	msg.data.instmsg.source = source;
-	if (len > sizeof(msg.data.instmsg.buf))
-		len = sizeof(msg.data.instmsg.buf);
-	if (!source) {
-		msg.data.instmsg.len = 0;
-	} else {
-		msg.data.instmsg.len = len;
-		memcpy(msg.data.instmsg.buf, buf, len);
-	}
+	msg.type = REQ_INSTALL;
 
+	msg.data.instmsg.req = *req;
 	ret = write(connfd, &msg, sizeof(msg));
 	if (ret != sizeof(msg)) {
 		close(connfd);
diff --git a/mongoose/mongoose_interface.c b/mongoose/mongoose_interface.c
index 27340bd..c2ba2c0 100644
--- a/mongoose/mongoose_interface.c
+++ b/mongoose/mongoose_interface.c
@@ -291,7 +291,8 @@  static void upload_handler(struct mg_connection *nc, int ev, void *p)
 		struct swupdate_request req;
 		swupdate_prepare_req(&req);
 		req.len = strlen(mp->file_name);
-		req.info = mp->file_name;
+		strncpy(req.info, mp->file_name, sizeof(req.info) - 1);
+		req.source = SOURCE_WEBSERVER;
 		fus->fd = ipc_inst_start_ext(SOURCE_WEBSERVER, &req, sizeof(req));
 		if (fus->fd < 0) {
 			mg_http_send_error(nc, 500, "Failed to queue command");
diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c
index db82ac8..5373db5 100644
--- a/suricatta/server_hawkbit.c
+++ b/suricatta/server_hawkbit.c
@@ -1843,8 +1843,8 @@  static server_op_res_t server_activation_ipc(ipc_message *msg)
 	update_state_t update_state = STATE_NOT_AVAILABLE;
 	struct json_object *json_root;
 
-	json_root = server_tokenize_msg(msg->data.instmsg.buf,
-					sizeof(msg->data.instmsg.buf));
+	json_root = server_tokenize_msg(msg->data.procmsg.buf,
+					sizeof(msg->data.procmsg.buf));
 	if (!json_root)
 		return SERVER_EERR;
 
@@ -1942,7 +1942,7 @@  static server_op_res_t server_activation_ipc(ipc_message *msg)
 		}
 	}
 
-	msg->data.instmsg.len = 0;
+	msg->data.procmsg.len = 0;
 
 cleanup:
 	free(details);
@@ -1956,8 +1956,8 @@  static server_op_res_t server_configuration_ipc(ipc_message *msg)
 	unsigned int polling;
 	json_object *json_data;
 
-	json_root = server_tokenize_msg(msg->data.instmsg.buf,
-					sizeof(msg->data.instmsg.buf));
+	json_root = server_tokenize_msg(msg->data.procmsg.buf,
+					sizeof(msg->data.procmsg.buf));
 	if (!json_root)
 		return SERVER_EERR;
 
@@ -1979,7 +1979,7 @@  server_op_res_t server_ipc(ipc_message *msg)
 {
 	server_op_res_t result = SERVER_OK;
 
-	switch (msg->data.instmsg.cmd) {
+	switch (msg->data.procmsg.cmd) {
 	case CMD_ACTIVATION:
 		result = server_activation_ipc(msg);
 		break;
@@ -1996,7 +1996,7 @@  server_op_res_t server_ipc(ipc_message *msg)
 	} else
 		msg->type = ACK;
 
-	msg->data.instmsg.len = 0;
+	msg->data.procmsg.len = 0;
 
 	return SERVER_OK;
 }
diff --git a/suricatta/suricatta.c b/suricatta/suricatta.c
index e5f8cc4..9e07efb 100644
--- a/suricatta/suricatta.c
+++ b/suricatta/suricatta.c
@@ -46,8 +46,8 @@  static server_op_res_t suricatta_enable(ipc_message *msg)
 	struct json_object *json_root;
 	json_object *json_data;
 
-	json_root = server_tokenize_msg(msg->data.instmsg.buf,
-					sizeof(msg->data.instmsg.buf));
+	json_root = server_tokenize_msg(msg->data.procmsg.buf,
+					sizeof(msg->data.procmsg.buf));
 	if (!json_root) {
 		msg->type = NACK;
 		ERROR("Wrong JSON message, see documentation");
@@ -92,7 +92,7 @@  static server_op_res_t suricatta_ipc(int fd, time_t *seconds)
 	if (ret != sizeof(msg))
 		return SERVER_EERR;
 
-	switch (msg.data.instmsg.cmd) {
+	switch (msg.data.procmsg.cmd) {
 	case CMD_ENABLE:
 		result = suricatta_enable(&msg);
 		/*
diff --git a/tools/swupdate-hawkbitcfg.c b/tools/swupdate-hawkbitcfg.c
index 3ccc24c..2e5a332 100644
--- a/tools/swupdate-hawkbitcfg.c
+++ b/tools/swupdate-hawkbitcfg.c
@@ -47,16 +47,16 @@  static void send_msg(ipc_message *msg)
 {
 	int rc;
 
-	fprintf(stdout, "Sending: '%s'", msg->data.instmsg.buf);
+	fprintf(stdout, "Sending: '%s'", msg->data.procmsg.buf);
 	rc = ipc_send_cmd(msg);
 
 	fprintf(stdout, " returned %d\n", rc);
 	if (rc == 0) {
 		fprintf(stdout, "Server returns %s\n",
 				(msg->type == ACK) ? "ACK" : "NACK");
-		if (msg->data.instmsg.len > 0) {
+		if (msg->data.procmsg.len > 0) {
 			fprintf(stdout, "Returned message: %s\n",
-					msg->data.instmsg.buf);
+					msg->data.procmsg.buf);
 		}
 	}
 }
@@ -80,11 +80,11 @@  int main(int argc, char *argv[]) {
 	}
 
 	memset(&msg, 0, sizeof(msg));
-	msg.data.instmsg.source = SOURCE_SURICATTA;
+	msg.data.procmsg.source = SOURCE_SURICATTA;
 	msg.type = SWUPDATE_SUBPROCESS;
 
-	size = sizeof(msg.data.instmsg.buf);
-	buf = msg.data.instmsg.buf;
+	size = sizeof(msg.data.procmsg.buf);
+	buf = msg.data.procmsg.buf;
 
 	/* Process options with getopt */
 	while ((c = getopt_long(argc, argv, "p:edh",
@@ -92,12 +92,12 @@  int main(int argc, char *argv[]) {
 		switch (c) {
 		case 'p':
 			opt_p = 1;
-			msg.data.instmsg.cmd = CMD_CONFIG;
+			msg.data.procmsg.cmd = CMD_CONFIG;
 			polling_time = strtoul(optarg, NULL, 10);
 			break;
 		case 'e':
 		case 'd':
-			msg.data.instmsg.cmd = CMD_ENABLE;
+			msg.data.procmsg.cmd = CMD_ENABLE;
 			opt_e = 1;
 			enable = (c == 'e');
 			break;
@@ -121,12 +121,12 @@  int main(int argc, char *argv[]) {
 	 */
 	if (opt_p) {
 		snprintf(buf, size, "{ \"polling\" : \"%lu\"}", polling_time);
-		msg.data.instmsg.len = strnlen(buf, size);
+		msg.data.procmsg.len = strnlen(buf, size);
 		send_msg(&msg);
 	}
 	if (opt_e) {
 		snprintf(buf, size, "{ \"enable\" : %s}", enable ? "true" : "false");
-		msg.data.instmsg.len = strnlen(buf, size);
+		msg.data.procmsg.len = strnlen(buf, size);
 		send_msg(&msg);
 	}
 
diff --git a/tools/swupdate-sendtohawkbit.c b/tools/swupdate-sendtohawkbit.c
index 0c65ce4..3701fb9 100644
--- a/tools/swupdate-sendtohawkbit.c
+++ b/tools/swupdate-sendtohawkbit.c
@@ -43,12 +43,12 @@  int main(int argc, char *argv[]) {
 	}
 
 	memset(&msg, 0, sizeof(msg));
-	msg.data.instmsg.source = SOURCE_SURICATTA;
-	msg.data.instmsg.cmd = CMD_ACTIVATION;
+	msg.data.procmsg.source = SOURCE_SURICATTA;
+	msg.data.procmsg.cmd = CMD_ACTIVATION;
 	msg.type = SWUPDATE_SUBPROCESS;
 
-	size = sizeof(msg.data.instmsg.buf);
-	buf = msg.data.instmsg.buf;
+	size = sizeof(msg.data.procmsg.buf);
+	buf = msg.data.procmsg.buf;
 
 	/*
 	 * Build a json string with the command line parameters
@@ -91,8 +91,8 @@  int main(int argc, char *argv[]) {
 	else
 		written = snprintf(buf, size, "}");
 
-	fprintf(stdout, "Sending: '%s'", msg.data.instmsg.buf);
-	msg.data.instmsg.len = strnlen(msg.data.instmsg.buf, sizeof(msg.data.instmsg.buf));
+	fprintf(stdout, "Sending: '%s'", msg.data.procmsg.buf);
+	msg.data.procmsg.len = strnlen(msg.data.procmsg.buf, sizeof(msg.data.procmsg.buf));
 
 	rc = ipc_send_cmd(&msg);