diff mbox series

[V2,2/2] BUG: post-update command is no longer called from mongoose interface

Message ID 20240111155246.668425-3-stefano.babic@swupdate.org
State Accepted
Headers show
Series BUG: post-update command is no longer called | expand

Commit Message

Stefano Babic Jan. 11, 2024, 3:52 p.m. UTC
post-update is not called anymore. Reason are commit 8fccd13 and d116870 because
they force the source of an update independently from the real source.
The source for an update is set when a install request is sent and cached
into core. The progress interface must retrieve the source and be stateless
for it.

Signed-off-by: Stefano Babic <stefano.babic@swupdate.org>
Reported-by: Michael Glembotzki <Michael.Glembotzki@iris-sensing.com>
---
 core/notifier.c               |  5 ++---
 core/progress_thread.c        | 15 ++++++++-------
 core/stream_interface.c       |  9 +++++++--
 corelib/channel_curl.c        |  2 +-
 include/progress.h            |  2 +-
 include/util.h                |  3 ++-
 mongoose/mongoose_interface.c |  4 ++--
 7 files changed, 23 insertions(+), 17 deletions(-)

Comments

Storm, Christian Jan. 18, 2024, 10:24 a.m. UTC | #1
Hi Stefano,

I did test all my test scenarios and this works fine, thanks for this!


On Jan 11, 2024, at 16:52, Stefano Babic <stefano.babic@swupdate.org> wrote:

post-update is not called anymore. Reason are commit 8fccd13 and d116870 because
they force the source of an update independently from the real source.
The source for an update is set when a install request is sent and cached
into core. The progress interface must retrieve the source and be stateless
for it.

Exactly, this is what this patch does :)


Signed-off-by: Stefano Babic <stefano.babic@swupdate.org>
Reported-by: Michael Glembotzki <Michael.Glembotzki@iris-sensing.com>
---
core/notifier.c               |  5 ++---
core/progress_thread.c        | 15 ++++++++-------
core/stream_interface.c       |  9 +++++++--
corelib/channel_curl.c        |  2 +-
include/progress.h            |  2 +-
include/util.h                |  3 ++-
mongoose/mongoose_interface.c |  4 ++--
7 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/core/notifier.c b/core/notifier.c
index 15220605..aec7a139 100644
--- a/core/notifier.c
+++ b/core/notifier.c
@@ -352,7 +352,6 @@ static void progress_notifier (RECOVERY_STATUS status, int event, int level, con
{
int dwl_percent = 0;
unsigned long long dwl_bytes = 0;
- sourcetype source = SOURCE_UNKNOWN;
(void)level;

/* Check just in case a process want to send an info outside */
@@ -360,8 +359,8 @@ static void progress_notifier (RECOVERY_STATUS status, int event, int level, con
      return;

if (event == RECOVERY_DWL &&
-    (sscanf(msg, "%d-%llu-%d", &dwl_percent, &dwl_bytes, (int *)&source) == 3)) {
-       swupdate_download_update(dwl_percent, dwl_bytes, source);
+    (sscanf(msg, "%d-%llu", &dwl_percent, &dwl_bytes) == 2)) {
+       swupdate_download_update(dwl_percent, dwl_bytes);
      return;
}

diff --git a/core/progress_thread.c b/core/progress_thread.c
index 161d2f61..269c85d3 100644
--- a/core/progress_thread.c
+++ b/core/progress_thread.c
@@ -85,6 +85,7 @@ static void send_progress_msg(void)
buf = &pprog->msg;
count = sizeof(pprog->msg);
errno = 0;
+ pprog->msg.source = get_install_source();
while (count > 0) {
int attempt = 0;
do {
@@ -107,7 +108,7 @@ static void send_progress_msg(void)
}
}

-static void _swupdate_download_update(unsigned int perc, unsigned long long totalbytes, sourcetype source)
+static void _swupdate_download_update(unsigned int perc, unsigned long long totalbytes)
{
/*
* TODO: totalbytes should be forwarded correctly
@@ -117,7 +118,6 @@ static void _swupdate_download_update(unsigned int perc, unsigned long long tota
pthread_mutex_lock(&pprog->lock);
if (perc != pprog->msg.dwl_percent) {
pprog->msg.status = DOWNLOAD;
- pprog->msg.source = source;
pprog->msg.dwl_percent = perc;
pprog->msg.dwl_bytes = totalbytes;
send_progress_msg();
@@ -133,9 +133,10 @@ void swupdate_progress_init(unsigned int nsteps) {
pprog->msg.nsteps = nsteps;
pprog->msg.cur_step = 0;
pprog->msg.status = START;
- pprog->msg.cur_percent = 0;
- pprog->msg.infolen = get_install_info(&pprog->msg.source, pprog->msg.info,
+ pprog->msg.cur_percent = 0;

This is wrongly indented.


+ pprog->msg.infolen = get_install_info(pprog->msg.info,
sizeof(pprog->msg.info));
+ pprog->msg.source = get_install_source();
send_progress_msg();
/* Info is just an event, reset it after sending */
pprog->msg.infolen = 0;
@@ -161,7 +162,7 @@ void swupdate_progress_update(unsigned int perc)
pthread_mutex_unlock(&pprog->lock);
}

-void swupdate_download_update(unsigned int perc, unsigned long long totalbytes, sourcetype source)
+void swupdate_download_update(unsigned int perc, unsigned long long totalbytes)
{
char info[PRINFOSIZE];    /* info */

@@ -175,13 +176,13 @@ void swupdate_download_update(unsigned int perc, unsigned long long totalbytes,
* and decode them in the notifier, in this case
* the progress_notifier
*/
- snprintf(info, sizeof(info) - 1, "%d-%llu-%d", perc, totalbytes, source);
+ snprintf(info, sizeof(info) - 1, "%d-%llu", perc, totalbytes);
notify(PROGRESS, RECOVERY_DWL, TRACELEVEL, info);
return;
}

/* Called by main process, emit a progress message */
- _swupdate_download_update(perc, totalbytes, source);
+ _swupdate_download_update(perc, totalbytes);
}

void swupdate_progress_inc_step(const char *image, const char *handler_name)
diff --git a/core/stream_interface.c b/core/stream_interface.c
index 0b783291..b9bce6d7 100644
--- a/core/stream_interface.c
+++ b/core/stream_interface.c
@@ -710,6 +710,7 @@ void *network_initializer(void *data)

pthread_mutex_lock(&stream_mutex);
inst.status = IDLE;
+ inst.req.source = SOURCE_UNKNOWN;
pthread_mutex_unlock(&stream_mutex);
TRACE("Main thread sleep again !");
notify(IDLE, RECOVERY_NO_ERROR, INFOLEVEL, "Waiting for requests...");
@@ -779,15 +780,19 @@ void get_install_running_mode(char *buf, size_t len)
 * The data is not locked because it is retrieve
 * at different times
 */
-int get_install_info(sourcetype *source, char *buf, size_t len)
+int get_install_info(char *buf, size_t len)
{
len = min(len - 1, strlen(inst.req.info));
strncpy(buf, inst.req.info, len);
- *source = inst.req.source;

return len;
}

+sourcetype get_install_source(void)
+{
+ return inst.req.source;
+}
+
void set_version_range(const char *minversion,
const char *maxversion, const char *current)
{
diff --git a/corelib/channel_curl.c b/corelib/channel_curl.c
index 33dbbd7f..35f7f37f 100644
--- a/corelib/channel_curl.c
+++ b/corelib/channel_curl.c
@@ -463,7 +463,7 @@ static int channel_callback_xferinfo(void *p, curl_off_t dltotal, curl_off_t dln
DEBUG("Downloaded %d%% (%zu of %zu kB).", percent,
(size_t)dlnow / 1024,
(size_t)dltotal / 1024);
- swupdate_download_update(percent, dltotal, data->source);
+ swupdate_download_update(percent, dltotal);

return 0;
}
diff --git a/include/progress.h b/include/progress.h
index de680f47..64fa23d7 100644
--- a/include/progress.h
+++ b/include/progress.h
@@ -24,6 +24,6 @@ void swupdate_progress_end(RECOVERY_STATUS status);
void swupdate_progress_done(const char *info);
void swupdate_progress_info(RECOVERY_STATUS status, int cause, const char *msg);

-void swupdate_download_update(unsigned int perc, unsigned long long totalbytes, sourcetype source);
+void swupdate_download_update(unsigned int perc, unsigned long long totalbytes);

void *progress_bar_thread (void *data);
diff --git a/include/util.h b/include/util.h
index 062840fc..490014e7 100644
--- a/include/util.h
+++ b/include/util.h
@@ -245,7 +245,8 @@ unsigned char *get_aes_ivt(void);
int set_aes_key(const char *key, const char *ivt);

/* Getting global information */
-int get_install_info(sourcetype *source, char *buf, size_t len);
+int get_install_info(char *buf, size_t len);
+sourcetype  get_install_source(void);
void get_install_swset(char *buf, size_t len);
void get_install_running_mode(char *buf, size_t len);
char *get_root_device(void);
diff --git a/mongoose/mongoose_interface.c b/mongoose/mongoose_interface.c
index 4b61acb1..39dca711 100644
--- a/mongoose/mongoose_interface.c
+++ b/mongoose/mongoose_interface.c
@@ -584,7 +584,7 @@ static void upload_handler(struct mg_connection *nc, int ev, void *ev_data,
break;
}

- swupdate_download_update(0, mp->len, SOURCE_WEBSERVER);
+ swupdate_download_update(0, mp->len);

if (swupdate_file_setnonblock(fus->fd, true)) {
WARN("IPC cannot be set in non-blocking, fallback to block mode");
@@ -637,7 +637,7 @@ static void upload_handler(struct mg_connection *nc, int ev, void *ev_data,
percent = (uint8_t)(100.0 * ((double)fus->len / (double)mp->len));
if (percent != fus->percent) {
fus->percent = percent;
- swupdate_download_update(fus->percent, mp->len, SOURCE_WEBSERVER);
+ swupdate_download_update(fus->percent, mp->len);
}

fus->last_io_time = mg_millis();
--
2.34.1



Kind regards,
  Christian

--
Dr. Christian Storm
Siemens AG, Technology, T CED OES-DE
Otto-Hahn-Ring 6, 81739 Munich, Germany
diff mbox series

Patch

diff --git a/core/notifier.c b/core/notifier.c
index 15220605..aec7a139 100644
--- a/core/notifier.c
+++ b/core/notifier.c
@@ -352,7 +352,6 @@  static void progress_notifier (RECOVERY_STATUS status, int event, int level, con
 {
 	int dwl_percent = 0;
 	unsigned long long dwl_bytes = 0;
-	sourcetype source = SOURCE_UNKNOWN;
 	(void)level;
 
 	/* Check just in case a process want to send an info outside */
@@ -360,8 +359,8 @@  static void progress_notifier (RECOVERY_STATUS status, int event, int level, con
 	       return;
 
 	if (event == RECOVERY_DWL &&
-	    (sscanf(msg, "%d-%llu-%d", &dwl_percent, &dwl_bytes, (int *)&source) == 3)) {
-	       swupdate_download_update(dwl_percent, dwl_bytes, source);
+	    (sscanf(msg, "%d-%llu", &dwl_percent, &dwl_bytes) == 2)) {
+	       swupdate_download_update(dwl_percent, dwl_bytes);
 	       return;
 	}
 
diff --git a/core/progress_thread.c b/core/progress_thread.c
index 161d2f61..269c85d3 100644
--- a/core/progress_thread.c
+++ b/core/progress_thread.c
@@ -85,6 +85,7 @@  static void send_progress_msg(void)
 		buf = &pprog->msg;
 		count = sizeof(pprog->msg);
 		errno = 0;
+		pprog->msg.source = get_install_source();
 		while (count > 0) {
 			int attempt = 0;
 			do {
@@ -107,7 +108,7 @@  static void send_progress_msg(void)
 	}
 }
 
-static void _swupdate_download_update(unsigned int perc, unsigned long long totalbytes, sourcetype source)
+static void _swupdate_download_update(unsigned int perc, unsigned long long totalbytes)
 {
 	/*
 	 * TODO: totalbytes should be forwarded correctly
@@ -117,7 +118,6 @@  static void _swupdate_download_update(unsigned int perc, unsigned long long tota
 	pthread_mutex_lock(&pprog->lock);
 	if (perc != pprog->msg.dwl_percent) {
 		pprog->msg.status = DOWNLOAD;
-		pprog->msg.source = source;
 		pprog->msg.dwl_percent = perc;
 		pprog->msg.dwl_bytes = totalbytes;
 		send_progress_msg();
@@ -133,9 +133,10 @@  void swupdate_progress_init(unsigned int nsteps) {
 	pprog->msg.nsteps = nsteps;
 	pprog->msg.cur_step = 0;
 	pprog->msg.status = START;
-	pprog->msg.cur_percent = 0;
-	pprog->msg.infolen = get_install_info(&pprog->msg.source, pprog->msg.info,
+		pprog->msg.cur_percent = 0;
+	pprog->msg.infolen = get_install_info(pprog->msg.info,
 						sizeof(pprog->msg.info));
+	pprog->msg.source = get_install_source();
 	send_progress_msg();
 	/* Info is just an event, reset it after sending */
 	pprog->msg.infolen = 0;
@@ -161,7 +162,7 @@  void swupdate_progress_update(unsigned int perc)
 	pthread_mutex_unlock(&pprog->lock);
 }
 
-void swupdate_download_update(unsigned int perc, unsigned long long totalbytes, sourcetype source)
+void swupdate_download_update(unsigned int perc, unsigned long long totalbytes)
 {
 	char	info[PRINFOSIZE];   		/* info */
 
@@ -175,13 +176,13 @@  void swupdate_download_update(unsigned int perc, unsigned long long totalbytes,
 		 * and decode them in the notifier, in this case
 		 * the progress_notifier
 		 */
-		snprintf(info, sizeof(info) - 1, "%d-%llu-%d", perc, totalbytes, source);
+		snprintf(info, sizeof(info) - 1, "%d-%llu", perc, totalbytes);
 		notify(PROGRESS, RECOVERY_DWL, TRACELEVEL, info);
 		return;
 	}
 
 	/* Called by main process, emit a progress message */
-	_swupdate_download_update(perc, totalbytes, source);
+	_swupdate_download_update(perc, totalbytes);
 }
 
 void swupdate_progress_inc_step(const char *image, const char *handler_name)
diff --git a/core/stream_interface.c b/core/stream_interface.c
index 0b783291..b9bce6d7 100644
--- a/core/stream_interface.c
+++ b/core/stream_interface.c
@@ -710,6 +710,7 @@  void *network_initializer(void *data)
 
 		pthread_mutex_lock(&stream_mutex);
 		inst.status = IDLE;
+		inst.req.source = SOURCE_UNKNOWN;
 		pthread_mutex_unlock(&stream_mutex);
 		TRACE("Main thread sleep again !");
 		notify(IDLE, RECOVERY_NO_ERROR, INFOLEVEL, "Waiting for requests...");
@@ -779,15 +780,19 @@  void get_install_running_mode(char *buf, size_t len)
  * The data is not locked because it is retrieve
  * at different times
  */
-int get_install_info(sourcetype *source, char *buf, size_t len)
+int get_install_info(char *buf, size_t len)
 {
 	len = min(len - 1, strlen(inst.req.info));
 	strncpy(buf, inst.req.info, len);
-	*source = inst.req.source;
 
 	return len;
 }
 
+sourcetype get_install_source(void)
+{
+	return inst.req.source;
+}
+
 void set_version_range(const char *minversion,
 		const char *maxversion, const char *current)
 {
diff --git a/corelib/channel_curl.c b/corelib/channel_curl.c
index 33dbbd7f..35f7f37f 100644
--- a/corelib/channel_curl.c
+++ b/corelib/channel_curl.c
@@ -463,7 +463,7 @@  static int channel_callback_xferinfo(void *p, curl_off_t dltotal, curl_off_t dln
 	DEBUG("Downloaded %d%% (%zu of %zu kB).", percent,
 		(size_t)dlnow / 1024,
 		(size_t)dltotal / 1024);
-	swupdate_download_update(percent, dltotal, data->source);
+	swupdate_download_update(percent, dltotal);
 
 	return 0;
 }
diff --git a/include/progress.h b/include/progress.h
index de680f47..64fa23d7 100644
--- a/include/progress.h
+++ b/include/progress.h
@@ -24,6 +24,6 @@  void swupdate_progress_end(RECOVERY_STATUS status);
 void swupdate_progress_done(const char *info);
 void swupdate_progress_info(RECOVERY_STATUS status, int cause, const char *msg);
 
-void swupdate_download_update(unsigned int perc, unsigned long long totalbytes, sourcetype source);
+void swupdate_download_update(unsigned int perc, unsigned long long totalbytes);
 
 void *progress_bar_thread (void *data);
diff --git a/include/util.h b/include/util.h
index 062840fc..490014e7 100644
--- a/include/util.h
+++ b/include/util.h
@@ -245,7 +245,8 @@  unsigned char *get_aes_ivt(void);
 int set_aes_key(const char *key, const char *ivt);
 
 /* Getting global information */
-int get_install_info(sourcetype *source, char *buf, size_t len);
+int get_install_info(char *buf, size_t len);
+sourcetype  get_install_source(void);
 void get_install_swset(char *buf, size_t len);
 void get_install_running_mode(char *buf, size_t len);
 char *get_root_device(void);
diff --git a/mongoose/mongoose_interface.c b/mongoose/mongoose_interface.c
index 4b61acb1..39dca711 100644
--- a/mongoose/mongoose_interface.c
+++ b/mongoose/mongoose_interface.c
@@ -584,7 +584,7 @@  static void upload_handler(struct mg_connection *nc, int ev, void *ev_data,
 				break;
 			}
 
-			swupdate_download_update(0, mp->len, SOURCE_WEBSERVER);
+			swupdate_download_update(0, mp->len);
 
 			if (swupdate_file_setnonblock(fus->fd, true)) {
 				WARN("IPC cannot be set in non-blocking, fallback to block mode");
@@ -637,7 +637,7 @@  static void upload_handler(struct mg_connection *nc, int ev, void *ev_data,
 			percent = (uint8_t)(100.0 * ((double)fus->len / (double)mp->len));
 			if (percent != fus->percent) {
 				fus->percent = percent;
-				swupdate_download_update(fus->percent, mp->len, SOURCE_WEBSERVER);
+				swupdate_download_update(fus->percent, mp->len);
 			}
 
 			fus->last_io_time = mg_millis();