| Message ID | 20170928125652.2920-1-christian.storm@siemens.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] logging: pass through and make use of loglevel | expand |
On 28/09/2017 14:56, Christian Storm wrote: > Although there are distinct macros for logging messages of > different priorities (INFO, DEBUG, WARN, ...), the different > notifiers emitting the log messages cannot make use of this > distinction and log messages with the same uniform priority. > > Hence, pass through the loglevel to the notifiers emitting the > log messages so that they may log respecting the intended > priority. Consequently, adapt the notifiers to make use of this > information, e.g., console_notifier now prefixes log messages > with their priority information: > > ... > [TRACE] : SWUPDATE running : [network_initializer] : Valid image found: copying to FLASH > [INFO ] : SWUPDATE running : Installation in progress > ... > > Signed-off-by: Christian Storm <christian.storm@siemens.com> > --- > core/notifier.c | 34 ++++++++++++++++++++++++++++------ > core/swupdate.c | 2 +- > core/syslog.c | 15 ++++++++++++--- > corelib/downloader.c | 2 +- > corelib/lua_interface.c | 2 +- > corelib/network_thread.c | 4 +++- > corelib/stream_interface.c | 14 +++++++------- > handlers/boot_handler.c | 14 +++++++------- > handlers/ubivol_handler.c | 2 +- > include/util.h | 8 ++++---- > suricatta/server_hawkbit.c | 8 ++++---- > 11 files changed, 69 insertions(+), 36 deletions(-) > > diff --git a/core/notifier.c b/core/notifier.c > index 5c7dd7b..cb75996 100644 > --- a/core/notifier.c > +++ b/core/notifier.c > @@ -60,6 +60,7 @@ static struct notifylist clients; > struct notify_ipc_msg { > RECOVERY_STATUS status; > int error; > + int level; > char buf[NOTIFY_BUF_SIZE]; > }; > > @@ -96,7 +97,7 @@ int register_notifier(notifier client) > * IPC to the main process that will dispatch it > * to the notifiers. > */ > -void notify(RECOVERY_STATUS status, int error, const char *msg) > +void notify(RECOVERY_STATUS status, int error, int level, const char *msg) > { > struct notify_elem *elem; > struct notify_ipc_msg notifymsg; > @@ -105,6 +106,7 @@ void notify(RECOVERY_STATUS status, int error, const char *msg) > if (notifyfd > 0) { > notifymsg.status = status; > notifymsg.error = error; > + notifymsg.level = level; > if (msg) > strcpy(notifymsg.buf, msg); > else > @@ -115,14 +117,14 @@ void notify(RECOVERY_STATUS status, int error, const char *msg) > } > } else { /* Main process */ > STAILQ_FOREACH(elem, &clients, next) > - (elem->client)(status, error, msg); > + (elem->client)(status, error, level, msg); > } > } > > /* > * Default notifier, it prints to stdout > */ > -static void console_notifier (RECOVERY_STATUS status, int error, const char *msg) > +static void console_notifier (RECOVERY_STATUS status, int error, int level, const char *msg) > { > char current[80]; > switch(status) { > @@ -152,7 +154,26 @@ static void console_notifier (RECOVERY_STATUS status, int error, const char *msg > break; > } > > - fprintf(stdout, "[NOTIFY] : %s %s\n", current, msg ? msg : ""); > + switch (level) { > + case ERRORLEVEL: > + fprintf(stderr, "[ERROR]"); > + break; > + case WARNLEVEL: > + fprintf(stdout, "[WARN ]"); > + break; > + case INFOLEVEL: > + fprintf(stdout, "[INFO ]"); > + break; > + case DEBUGLEVEL: > + fprintf(stdout, "[DEBUG]"); > + break; > + case TRACELEVEL: > + fprintf(stdout, "[TRACE]"); > + break; > + } > + > + fprintf(level == ERRORLEVEL ? stderr : stdout, > + " : %s %s\n", current, msg ? msg : ""); > fflush(stdout); > } > > @@ -160,8 +181,9 @@ static void console_notifier (RECOVERY_STATUS status, int error, const char *msg > * Process notifier: this is called when a process has something to say > * and wants that the information is passed to the progress interface > */ > -static void process_notifier (RECOVERY_STATUS status, int event, const char *msg) > +static void process_notifier (RECOVERY_STATUS status, int event, int level, const char *msg) > { > + (void)level; > > /* Check just in case a process want to send an info outside */ > if (status != SUBPROCESS) > @@ -226,7 +248,7 @@ static void *notifier_thread (void __attribute__ ((__unused__)) *data) > len = recvfrom(serverfd, &msg, sizeof(msg), 0, NULL, NULL); > > if (len > 0) { > - notify(msg.status, msg.error, msg.buf); > + notify(msg.status, msg.error, msg.level, msg.buf); > } > > } while(1); > diff --git a/core/swupdate.c b/core/swupdate.c > index ef94bd9..a9c71cf 100644 > --- a/core/swupdate.c > +++ b/core/swupdate.c > @@ -862,7 +862,7 @@ int main(int argc, char **argv) > install_from_file(fname, opt_c); > cleanup_files(&swcfg); > > - notify(SUCCESS, 0, 0); > + notify(SUCCESS, 0, INFOLEVEL, NULL); > if (postupdate(&swcfg, NULL) != 0) { > ERROR("Post-update command execution failed."); > } > diff --git a/core/syslog.c b/core/syslog.c > index d031ed1..88b1218 100644 > --- a/core/syslog.c > +++ b/core/syslog.c > @@ -20,7 +20,7 @@ > #include <stdio.h> > #include "util.h" > > -static void syslog_notifier(RECOVERY_STATUS status, int error, const char *msg); > +static void syslog_notifier(RECOVERY_STATUS status, int error, int level, const char *msg); > > int syslog_init(void) > { > @@ -28,7 +28,7 @@ int syslog_init(void) > return register_notifier(syslog_notifier); > } > > -void syslog_notifier(RECOVERY_STATUS status, int error, const char *msg) > +void syslog_notifier(RECOVERY_STATUS status, int error, int level, const char *msg) > { > const char* statusMsg; > > @@ -45,7 +45,16 @@ void syslog_notifier(RECOVERY_STATUS status, int error, const char *msg) > default: statusMsg = "UNKNOWN"; break; > } > > - syslog(LOG_NOTICE, "%s%s %s\n", ((error != (int)RECOVERY_NO_ERROR) ? "FATAL_" : ""), statusMsg, msg); > + int logprio = LOG_INFO; > + switch (level) { > + case ERRORLEVEL: logprio = LOG_ERR; break; > + case WARNLEVEL: logprio = LOG_WARNING; break; > + case INFOLEVEL: logprio = LOG_INFO; break; > + case DEBUGLEVEL: > + case TRACELEVEL: logprio = LOG_DEBUG; break; > + } > + > + syslog(logprio, "%s%s %s\n", ((error != (int)RECOVERY_NO_ERROR) ? "FATAL_" : ""), statusMsg, msg); > > closelog(); > } > diff --git a/corelib/downloader.c b/corelib/downloader.c > index 58589f3..27bb4e8 100644 > --- a/corelib/downloader.c > +++ b/corelib/downloader.c > @@ -237,7 +237,7 @@ static RECOVERY_STATUS download_from_url(char *image_url, unsigned int retries, > } > > /* We are starting a download */ > - notify(DOWNLOAD, 0, 0); > + notify(DOWNLOAD, 0, INFOLEVEL, NULL); > > curl_global_init(CURL_GLOBAL_ALL); > curl_handle = curl_easy_init(); > diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c > index 5717807..5f48726 100644 > --- a/corelib/lua_interface.c > +++ b/corelib/lua_interface.c > @@ -304,7 +304,7 @@ static int l_notify (lua_State *L) { > const char *msg = luaL_checkstring (L, 3); > > if (strlen(msg)) > - notify((RECOVERY_STATUS)status, (int)error, msg); > + notify((RECOVERY_STATUS)status, (int)error, INFOLEVEL, msg); > > return 0; > } > diff --git a/corelib/network_thread.c b/corelib/network_thread.c > index 747ee1b..8c2b3b6 100644 > --- a/corelib/network_thread.c > +++ b/corelib/network_thread.c > @@ -56,6 +56,7 @@ > struct msg_elem { > RECOVERY_STATUS status; > int error; > + int level; > char *msg; > SIMPLEQ_ENTRY(msg_elem) next; > }; > @@ -76,7 +77,7 @@ static void clean_msg(char *msg, char drop) > } > } > > -static void network_notifier(RECOVERY_STATUS status, int error, const char *msg) > +static void network_notifier(RECOVERY_STATUS status, int error, int level, const char *msg) > { > int len = msg ? strlen(msg) : 0; > struct msg_elem *newmsg = (struct msg_elem *)calloc(1, sizeof(*newmsg) + len + 1); > @@ -97,6 +98,7 @@ static void network_notifier(RECOVERY_STATUS status, int error, const char *msg) > > newmsg->status = status; > newmsg->error = error; > + newmsg->level = level; > > if (msg) { > strncpy(newmsg->msg, msg, len); > diff --git a/corelib/stream_interface.c b/corelib/stream_interface.c > index fb56c96..404c116 100644 > --- a/corelib/stream_interface.c > +++ b/corelib/stream_interface.c > @@ -309,14 +309,14 @@ void *network_initializer(void *data) > /* handle installation requests (from either source) */ > while (1) { > > - printf ("Main loop Daemon\n"); > + TRACE("Main loop Daemon"); > > /* wait for someone to issue an install request */ > pthread_mutex_lock(&stream_mutex); > pthread_cond_wait(&stream_wkup, &stream_mutex); > inst.status = RUN; > pthread_mutex_unlock(&stream_mutex); > - notify(START, RECOVERY_NO_ERROR, "Software Update started !"); > + notify(START, RECOVERY_NO_ERROR, INFOLEVEL, "Software Update started !"); > > #ifdef CONFIG_MTD > mtd_cleanup(); > @@ -340,11 +340,11 @@ void *network_initializer(void *data) > */ > bootloader_env_set("recovery_status", "in_progress"); > > - notify(RUN, RECOVERY_NO_ERROR, "Installation in progress"); > + notify(RUN, RECOVERY_NO_ERROR, INFOLEVEL, "Installation in progress"); > ret = install_images(software, 0, 0); > if (ret != 0) { > bootloader_env_set("recovery_status", "failed"); > - notify(FAILURE, RECOVERY_ERROR, "Installation failed !"); > + notify(FAILURE, RECOVERY_ERROR, ERRORLEVEL, "Installation failed !"); > inst.last_install = FAILURE; > > } else { > @@ -353,12 +353,12 @@ void *network_initializer(void *data) > * that it is not required to start recovery again > */ > bootloader_env_unset("recovery_status"); > - notify(SUCCESS, RECOVERY_NO_ERROR, "SWUPDATE successful !"); > + notify(SUCCESS, RECOVERY_NO_ERROR, INFOLEVEL, "SWUPDATE successful !"); > inst.last_install = SUCCESS; > } > } else { > inst.last_install = FAILURE; > - notify(FAILURE, RECOVERY_ERROR, "Image invalid or corrupted. Not installing ..."); > + notify(FAILURE, RECOVERY_ERROR, ERRORLEVEL, "Image invalid or corrupted. Not installing ..."); > } > > swupdate_progress_end(inst.last_install); > @@ -367,7 +367,7 @@ void *network_initializer(void *data) > inst.status = IDLE; > pthread_mutex_unlock(&stream_mutex); > TRACE("Main thread sleep again !"); > - notify(IDLE, RECOVERY_NO_ERROR, "Waiting for requests..."); > + notify(IDLE, RECOVERY_NO_ERROR, INFOLEVEL, "Waiting for requests..."); > > /* release temp files we may have created */ > cleanup_files(software); > diff --git a/handlers/boot_handler.c b/handlers/boot_handler.c > index 41f87f7..86c55d0 100644 > --- a/handlers/boot_handler.c > +++ b/handlers/boot_handler.c > @@ -39,7 +39,6 @@ static int install_boot_environment(struct img_type *img, > { > int ret; > int fdout; > - char buf[64]; > > char filename[64]; > struct stat statbuf; > @@ -62,12 +61,13 @@ static int install_boot_environment(struct img_type *img, > } > > ret = bootloader_apply_list(filename); > - if (ret < 0) > - snprintf(buf, sizeof(buf), "Error setting bootloader environment"); > - else > - snprintf(buf, sizeof(buf), "Bootloader environment updated"); > - > - notify(RUN, RECOVERY_NO_ERROR, buf); > + if (ret < 0) { > + notify(RUN, RECOVERY_NO_ERROR, ERRORLEVEL, > + "Error setting bootloader environment"); > + } else { > + notify(RUN, RECOVERY_NO_ERROR, INFOLEVEL, > + "Bootloader environment updated"); > + } > > return ret; > } > diff --git a/handlers/ubivol_handler.c b/handlers/ubivol_handler.c > index bf58658..0e262b6 100644 > --- a/handlers/ubivol_handler.c > +++ b/handlers/ubivol_handler.c > @@ -100,7 +100,7 @@ static int update_volume(libubi_t libubi, struct img_type *img, > > snprintf(sbuf, sizeof(sbuf), "Installing image %s into volume %s(%s)", > img->fname, node, img->volname); > - notify(RUN, RECOVERY_NO_ERROR, sbuf); > + notify(RUN, RECOVERY_NO_ERROR, INFOLEVEL, sbuf); > > TRACE("Updating UBI : %s %lld\n", > img->fname, img->size); > diff --git a/include/util.h b/include/util.h > index cc0f465..24ec127 100644 > --- a/include/util.h > +++ b/include/util.h > @@ -70,7 +70,7 @@ struct installer { > char info[2048]; /* info */ > }; > > -typedef void (*notifier) (RECOVERY_STATUS status, int level, const char *msg); > +typedef void (*notifier) (RECOVERY_STATUS status, int error, int level, const char *msg); > > #define swupdate_notify(status, format, level, arg...) do { \ > if (loglevel >= level) { \ > @@ -86,11 +86,11 @@ typedef void (*notifier) (RECOVERY_STATUS status, int level, const char *msg); > else \ > snprintf(tmpbuf, sizeof(tmpbuf), \ > "ERROR : " format, ## arg); \ > - notify(FAILURE, 0, tmpbuf); \ > + notify(FAILURE, 0, level, tmpbuf); \ > } else {\ > snprintf(tmpbuf, sizeof(tmpbuf), \ > "[%s] : " format, __func__, ## arg); \ > - notify(RUN, RECOVERY_NO_ERROR, tmpbuf); \ > + notify(RUN, RECOVERY_NO_ERROR, level, tmpbuf); \ > } \ > } \ > } while(0) > @@ -155,7 +155,7 @@ off_t extract_next_file(int fd, int fdout, off_t start, int compressed, > int openfileoutput(const char *filename); > > int register_notifier(notifier client); > -void notify(RECOVERY_STATUS status, int level, const char *msg); > +void notify(RECOVERY_STATUS status, int error, int level, const char *msg); > void notify_init(void); > int syslog_init(void); > > diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c > index 243db4f..1b0e600 100644 > --- a/suricatta/server_hawkbit.c > +++ b/suricatta/server_hawkbit.c > @@ -301,9 +301,9 @@ static void check_action_changed(int action_id, const char *update_action) > if (ENOMEM_ASPRINTF == > asprintf(¬ifybuf, "{ \"id\" : \"%d\", \"update\" : \"%s\"}", > action_id, server_hawkbit.update_action)) { > - notify(SUBPROCESS, CHANGE, "Update type changed by server"); > + notify(SUBPROCESS, CHANGE, DEBUGLEVEL, "Update type changed by server"); > } else { > - notify(SUBPROCESS, CHANGE, notifybuf); > + notify(SUBPROCESS, CHANGE, DEBUGLEVEL, notifybuf); > free(notifybuf); > } > } > @@ -406,9 +406,9 @@ cleanup: > if (ENOMEM_ASPRINTF == > asprintf(¬ifybuf, "{ \"id\" : \"%d\", \"stopId\" : \"%d\"}", > action_id, stop_id)) { > - notify(SUBPROCESS, CANCELUPDATE, "Update cancelled"); > + notify(SUBPROCESS, CANCELUPDATE, INFOLEVEL, "Update cancelled"); > } else { > - notify(SUBPROCESS, CANCELUPDATE, notifybuf); > + notify(SUBPROCESS, CANCELUPDATE, INFOLEVEL, notifybuf); > free(notifybuf); > } > > It looks great. Acked-by: Stefano Babic <sbabic@denx.de> Best regards, Stefano Babic
On 28/09/2017 14:56, Christian Storm wrote: > Although there are distinct macros for logging messages of > different priorities (INFO, DEBUG, WARN, ...), the different > notifiers emitting the log messages cannot make use of this > distinction and log messages with the same uniform priority. > > Hence, pass through the loglevel to the notifiers emitting the > log messages so that they may log respecting the intended > priority. Consequently, adapt the notifiers to make use of this > information, e.g., console_notifier now prefixes log messages > with their priority information: > > ... > [TRACE] : SWUPDATE running : [network_initializer] : Valid image found: copying to FLASH > [INFO ] : SWUPDATE running : Installation in progress > ... > > Signed-off-by: Christian Storm <christian.storm@siemens.com> > --- > core/notifier.c | 34 ++++++++++++++++++++++++++++------ > core/swupdate.c | 2 +- > core/syslog.c | 15 ++++++++++++--- > corelib/downloader.c | 2 +- > corelib/lua_interface.c | 2 +- > corelib/network_thread.c | 4 +++- > corelib/stream_interface.c | 14 +++++++------- > handlers/boot_handler.c | 14 +++++++------- > handlers/ubivol_handler.c | 2 +- > include/util.h | 8 ++++---- > suricatta/server_hawkbit.c | 8 ++++---- > 11 files changed, 69 insertions(+), 36 deletions(-) > > diff --git a/core/notifier.c b/core/notifier.c > index 5c7dd7b..cb75996 100644 > --- a/core/notifier.c > +++ b/core/notifier.c > @@ -60,6 +60,7 @@ static struct notifylist clients; > struct notify_ipc_msg { > RECOVERY_STATUS status; > int error; > + int level; > char buf[NOTIFY_BUF_SIZE]; > }; > > @@ -96,7 +97,7 @@ int register_notifier(notifier client) > * IPC to the main process that will dispatch it > * to the notifiers. > */ > -void notify(RECOVERY_STATUS status, int error, const char *msg) > +void notify(RECOVERY_STATUS status, int error, int level, const char *msg) > { > struct notify_elem *elem; > struct notify_ipc_msg notifymsg; > @@ -105,6 +106,7 @@ void notify(RECOVERY_STATUS status, int error, const char *msg) > if (notifyfd > 0) { > notifymsg.status = status; > notifymsg.error = error; > + notifymsg.level = level; > if (msg) > strcpy(notifymsg.buf, msg); > else > @@ -115,14 +117,14 @@ void notify(RECOVERY_STATUS status, int error, const char *msg) > } > } else { /* Main process */ > STAILQ_FOREACH(elem, &clients, next) > - (elem->client)(status, error, msg); > + (elem->client)(status, error, level, msg); > } > } > > /* > * Default notifier, it prints to stdout > */ > -static void console_notifier (RECOVERY_STATUS status, int error, const char *msg) > +static void console_notifier (RECOVERY_STATUS status, int error, int level, const char *msg) > { > char current[80]; > switch(status) { > @@ -152,7 +154,26 @@ static void console_notifier (RECOVERY_STATUS status, int error, const char *msg > break; > } > > - fprintf(stdout, "[NOTIFY] : %s %s\n", current, msg ? msg : ""); > + switch (level) { > + case ERRORLEVEL: > + fprintf(stderr, "[ERROR]"); > + break; > + case WARNLEVEL: > + fprintf(stdout, "[WARN ]"); > + break; > + case INFOLEVEL: > + fprintf(stdout, "[INFO ]"); > + break; > + case DEBUGLEVEL: > + fprintf(stdout, "[DEBUG]"); > + break; > + case TRACELEVEL: > + fprintf(stdout, "[TRACE]"); > + break; > + } > + > + fprintf(level == ERRORLEVEL ? stderr : stdout, > + " : %s %s\n", current, msg ? msg : ""); > fflush(stdout); > } > > @@ -160,8 +181,9 @@ static void console_notifier (RECOVERY_STATUS status, int error, const char *msg > * Process notifier: this is called when a process has something to say > * and wants that the information is passed to the progress interface > */ > -static void process_notifier (RECOVERY_STATUS status, int event, const char *msg) > +static void process_notifier (RECOVERY_STATUS status, int event, int level, const char *msg) > { > + (void)level; > > /* Check just in case a process want to send an info outside */ > if (status != SUBPROCESS) > @@ -226,7 +248,7 @@ static void *notifier_thread (void __attribute__ ((__unused__)) *data) > len = recvfrom(serverfd, &msg, sizeof(msg), 0, NULL, NULL); > > if (len > 0) { > - notify(msg.status, msg.error, msg.buf); > + notify(msg.status, msg.error, msg.level, msg.buf); > } > > } while(1); > diff --git a/core/swupdate.c b/core/swupdate.c > index ef94bd9..a9c71cf 100644 > --- a/core/swupdate.c > +++ b/core/swupdate.c > @@ -862,7 +862,7 @@ int main(int argc, char **argv) > install_from_file(fname, opt_c); > cleanup_files(&swcfg); > > - notify(SUCCESS, 0, 0); > + notify(SUCCESS, 0, INFOLEVEL, NULL); > if (postupdate(&swcfg, NULL) != 0) { > ERROR("Post-update command execution failed."); > } > diff --git a/core/syslog.c b/core/syslog.c > index d031ed1..88b1218 100644 > --- a/core/syslog.c > +++ b/core/syslog.c > @@ -20,7 +20,7 @@ > #include <stdio.h> > #include "util.h" > > -static void syslog_notifier(RECOVERY_STATUS status, int error, const char *msg); > +static void syslog_notifier(RECOVERY_STATUS status, int error, int level, const char *msg); > > int syslog_init(void) > { > @@ -28,7 +28,7 @@ int syslog_init(void) > return register_notifier(syslog_notifier); > } > > -void syslog_notifier(RECOVERY_STATUS status, int error, const char *msg) > +void syslog_notifier(RECOVERY_STATUS status, int error, int level, const char *msg) > { > const char* statusMsg; > > @@ -45,7 +45,16 @@ void syslog_notifier(RECOVERY_STATUS status, int error, const char *msg) > default: statusMsg = "UNKNOWN"; break; > } > > - syslog(LOG_NOTICE, "%s%s %s\n", ((error != (int)RECOVERY_NO_ERROR) ? "FATAL_" : ""), statusMsg, msg); > + int logprio = LOG_INFO; > + switch (level) { > + case ERRORLEVEL: logprio = LOG_ERR; break; > + case WARNLEVEL: logprio = LOG_WARNING; break; > + case INFOLEVEL: logprio = LOG_INFO; break; > + case DEBUGLEVEL: > + case TRACELEVEL: logprio = LOG_DEBUG; break; > + } > + > + syslog(logprio, "%s%s %s\n", ((error != (int)RECOVERY_NO_ERROR) ? "FATAL_" : ""), statusMsg, msg); > > closelog(); > } > diff --git a/corelib/downloader.c b/corelib/downloader.c > index 58589f3..27bb4e8 100644 > --- a/corelib/downloader.c > +++ b/corelib/downloader.c > @@ -237,7 +237,7 @@ static RECOVERY_STATUS download_from_url(char *image_url, unsigned int retries, > } > > /* We are starting a download */ > - notify(DOWNLOAD, 0, 0); > + notify(DOWNLOAD, 0, INFOLEVEL, NULL); > > curl_global_init(CURL_GLOBAL_ALL); > curl_handle = curl_easy_init(); > diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c > index 5717807..5f48726 100644 > --- a/corelib/lua_interface.c > +++ b/corelib/lua_interface.c > @@ -304,7 +304,7 @@ static int l_notify (lua_State *L) { > const char *msg = luaL_checkstring (L, 3); > > if (strlen(msg)) > - notify((RECOVERY_STATUS)status, (int)error, msg); > + notify((RECOVERY_STATUS)status, (int)error, INFOLEVEL, msg); > > return 0; > } > diff --git a/corelib/network_thread.c b/corelib/network_thread.c > index 747ee1b..8c2b3b6 100644 > --- a/corelib/network_thread.c > +++ b/corelib/network_thread.c > @@ -56,6 +56,7 @@ > struct msg_elem { > RECOVERY_STATUS status; > int error; > + int level; > char *msg; > SIMPLEQ_ENTRY(msg_elem) next; > }; > @@ -76,7 +77,7 @@ static void clean_msg(char *msg, char drop) > } > } > > -static void network_notifier(RECOVERY_STATUS status, int error, const char *msg) > +static void network_notifier(RECOVERY_STATUS status, int error, int level, const char *msg) > { > int len = msg ? strlen(msg) : 0; > struct msg_elem *newmsg = (struct msg_elem *)calloc(1, sizeof(*newmsg) + len + 1); > @@ -97,6 +98,7 @@ static void network_notifier(RECOVERY_STATUS status, int error, const char *msg) > > newmsg->status = status; > newmsg->error = error; > + newmsg->level = level; > > if (msg) { > strncpy(newmsg->msg, msg, len); > diff --git a/corelib/stream_interface.c b/corelib/stream_interface.c > index fb56c96..404c116 100644 > --- a/corelib/stream_interface.c > +++ b/corelib/stream_interface.c > @@ -309,14 +309,14 @@ void *network_initializer(void *data) > /* handle installation requests (from either source) */ > while (1) { > > - printf ("Main loop Daemon\n"); > + TRACE("Main loop Daemon"); > > /* wait for someone to issue an install request */ > pthread_mutex_lock(&stream_mutex); > pthread_cond_wait(&stream_wkup, &stream_mutex); > inst.status = RUN; > pthread_mutex_unlock(&stream_mutex); > - notify(START, RECOVERY_NO_ERROR, "Software Update started !"); > + notify(START, RECOVERY_NO_ERROR, INFOLEVEL, "Software Update started !"); > > #ifdef CONFIG_MTD > mtd_cleanup(); > @@ -340,11 +340,11 @@ void *network_initializer(void *data) > */ > bootloader_env_set("recovery_status", "in_progress"); > > - notify(RUN, RECOVERY_NO_ERROR, "Installation in progress"); > + notify(RUN, RECOVERY_NO_ERROR, INFOLEVEL, "Installation in progress"); > ret = install_images(software, 0, 0); > if (ret != 0) { > bootloader_env_set("recovery_status", "failed"); > - notify(FAILURE, RECOVERY_ERROR, "Installation failed !"); > + notify(FAILURE, RECOVERY_ERROR, ERRORLEVEL, "Installation failed !"); > inst.last_install = FAILURE; > > } else { > @@ -353,12 +353,12 @@ void *network_initializer(void *data) > * that it is not required to start recovery again > */ > bootloader_env_unset("recovery_status"); > - notify(SUCCESS, RECOVERY_NO_ERROR, "SWUPDATE successful !"); > + notify(SUCCESS, RECOVERY_NO_ERROR, INFOLEVEL, "SWUPDATE successful !"); > inst.last_install = SUCCESS; > } > } else { > inst.last_install = FAILURE; > - notify(FAILURE, RECOVERY_ERROR, "Image invalid or corrupted. Not installing ..."); > + notify(FAILURE, RECOVERY_ERROR, ERRORLEVEL, "Image invalid or corrupted. Not installing ..."); > } > > swupdate_progress_end(inst.last_install); > @@ -367,7 +367,7 @@ void *network_initializer(void *data) > inst.status = IDLE; > pthread_mutex_unlock(&stream_mutex); > TRACE("Main thread sleep again !"); > - notify(IDLE, RECOVERY_NO_ERROR, "Waiting for requests..."); > + notify(IDLE, RECOVERY_NO_ERROR, INFOLEVEL, "Waiting for requests..."); > > /* release temp files we may have created */ > cleanup_files(software); > diff --git a/handlers/boot_handler.c b/handlers/boot_handler.c > index 41f87f7..86c55d0 100644 > --- a/handlers/boot_handler.c > +++ b/handlers/boot_handler.c > @@ -39,7 +39,6 @@ static int install_boot_environment(struct img_type *img, > { > int ret; > int fdout; > - char buf[64]; > > char filename[64]; > struct stat statbuf; > @@ -62,12 +61,13 @@ static int install_boot_environment(struct img_type *img, > } > > ret = bootloader_apply_list(filename); > - if (ret < 0) > - snprintf(buf, sizeof(buf), "Error setting bootloader environment"); > - else > - snprintf(buf, sizeof(buf), "Bootloader environment updated"); > - > - notify(RUN, RECOVERY_NO_ERROR, buf); > + if (ret < 0) { > + notify(RUN, RECOVERY_NO_ERROR, ERRORLEVEL, > + "Error setting bootloader environment"); > + } else { > + notify(RUN, RECOVERY_NO_ERROR, INFOLEVEL, > + "Bootloader environment updated"); > + } > > return ret; > } > diff --git a/handlers/ubivol_handler.c b/handlers/ubivol_handler.c > index bf58658..0e262b6 100644 > --- a/handlers/ubivol_handler.c > +++ b/handlers/ubivol_handler.c > @@ -100,7 +100,7 @@ static int update_volume(libubi_t libubi, struct img_type *img, > > snprintf(sbuf, sizeof(sbuf), "Installing image %s into volume %s(%s)", > img->fname, node, img->volname); > - notify(RUN, RECOVERY_NO_ERROR, sbuf); > + notify(RUN, RECOVERY_NO_ERROR, INFOLEVEL, sbuf); > > TRACE("Updating UBI : %s %lld\n", > img->fname, img->size); > diff --git a/include/util.h b/include/util.h > index cc0f465..24ec127 100644 > --- a/include/util.h > +++ b/include/util.h > @@ -70,7 +70,7 @@ struct installer { > char info[2048]; /* info */ > }; > > -typedef void (*notifier) (RECOVERY_STATUS status, int level, const char *msg); > +typedef void (*notifier) (RECOVERY_STATUS status, int error, int level, const char *msg); > > #define swupdate_notify(status, format, level, arg...) do { \ > if (loglevel >= level) { \ > @@ -86,11 +86,11 @@ typedef void (*notifier) (RECOVERY_STATUS status, int level, const char *msg); > else \ > snprintf(tmpbuf, sizeof(tmpbuf), \ > "ERROR : " format, ## arg); \ > - notify(FAILURE, 0, tmpbuf); \ > + notify(FAILURE, 0, level, tmpbuf); \ > } else {\ > snprintf(tmpbuf, sizeof(tmpbuf), \ > "[%s] : " format, __func__, ## arg); \ > - notify(RUN, RECOVERY_NO_ERROR, tmpbuf); \ > + notify(RUN, RECOVERY_NO_ERROR, level, tmpbuf); \ > } \ > } \ > } while(0) > @@ -155,7 +155,7 @@ off_t extract_next_file(int fd, int fdout, off_t start, int compressed, > int openfileoutput(const char *filename); > > int register_notifier(notifier client); > -void notify(RECOVERY_STATUS status, int level, const char *msg); > +void notify(RECOVERY_STATUS status, int error, int level, const char *msg); > void notify_init(void); > int syslog_init(void); > > diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c > index 243db4f..1b0e600 100644 > --- a/suricatta/server_hawkbit.c > +++ b/suricatta/server_hawkbit.c > @@ -301,9 +301,9 @@ static void check_action_changed(int action_id, const char *update_action) > if (ENOMEM_ASPRINTF == > asprintf(¬ifybuf, "{ \"id\" : \"%d\", \"update\" : \"%s\"}", > action_id, server_hawkbit.update_action)) { > - notify(SUBPROCESS, CHANGE, "Update type changed by server"); > + notify(SUBPROCESS, CHANGE, DEBUGLEVEL, "Update type changed by server"); > } else { > - notify(SUBPROCESS, CHANGE, notifybuf); > + notify(SUBPROCESS, CHANGE, DEBUGLEVEL, notifybuf); > free(notifybuf); > } > } > @@ -406,9 +406,9 @@ cleanup: > if (ENOMEM_ASPRINTF == > asprintf(¬ifybuf, "{ \"id\" : \"%d\", \"stopId\" : \"%d\"}", > action_id, stop_id)) { > - notify(SUBPROCESS, CANCELUPDATE, "Update cancelled"); > + notify(SUBPROCESS, CANCELUPDATE, INFOLEVEL, "Update cancelled"); > } else { > - notify(SUBPROCESS, CANCELUPDATE, notifybuf); > + notify(SUBPROCESS, CANCELUPDATE, INFOLEVEL, notifybuf); > free(notifybuf); > } > > Applied to -master, thanks ! Best regards, Stefano Babic
diff --git a/core/notifier.c b/core/notifier.c index 5c7dd7b..cb75996 100644 --- a/core/notifier.c +++ b/core/notifier.c @@ -60,6 +60,7 @@ static struct notifylist clients; struct notify_ipc_msg { RECOVERY_STATUS status; int error; + int level; char buf[NOTIFY_BUF_SIZE]; }; @@ -96,7 +97,7 @@ int register_notifier(notifier client) * IPC to the main process that will dispatch it * to the notifiers. */ -void notify(RECOVERY_STATUS status, int error, const char *msg) +void notify(RECOVERY_STATUS status, int error, int level, const char *msg) { struct notify_elem *elem; struct notify_ipc_msg notifymsg; @@ -105,6 +106,7 @@ void notify(RECOVERY_STATUS status, int error, const char *msg) if (notifyfd > 0) { notifymsg.status = status; notifymsg.error = error; + notifymsg.level = level; if (msg) strcpy(notifymsg.buf, msg); else @@ -115,14 +117,14 @@ void notify(RECOVERY_STATUS status, int error, const char *msg) } } else { /* Main process */ STAILQ_FOREACH(elem, &clients, next) - (elem->client)(status, error, msg); + (elem->client)(status, error, level, msg); } } /* * Default notifier, it prints to stdout */ -static void console_notifier (RECOVERY_STATUS status, int error, const char *msg) +static void console_notifier (RECOVERY_STATUS status, int error, int level, const char *msg) { char current[80]; switch(status) { @@ -152,7 +154,26 @@ static void console_notifier (RECOVERY_STATUS status, int error, const char *msg break; } - fprintf(stdout, "[NOTIFY] : %s %s\n", current, msg ? msg : ""); + switch (level) { + case ERRORLEVEL: + fprintf(stderr, "[ERROR]"); + break; + case WARNLEVEL: + fprintf(stdout, "[WARN ]"); + break; + case INFOLEVEL: + fprintf(stdout, "[INFO ]"); + break; + case DEBUGLEVEL: + fprintf(stdout, "[DEBUG]"); + break; + case TRACELEVEL: + fprintf(stdout, "[TRACE]"); + break; + } + + fprintf(level == ERRORLEVEL ? stderr : stdout, + " : %s %s\n", current, msg ? msg : ""); fflush(stdout); } @@ -160,8 +181,9 @@ static void console_notifier (RECOVERY_STATUS status, int error, const char *msg * Process notifier: this is called when a process has something to say * and wants that the information is passed to the progress interface */ -static void process_notifier (RECOVERY_STATUS status, int event, const char *msg) +static void process_notifier (RECOVERY_STATUS status, int event, int level, const char *msg) { + (void)level; /* Check just in case a process want to send an info outside */ if (status != SUBPROCESS) @@ -226,7 +248,7 @@ static void *notifier_thread (void __attribute__ ((__unused__)) *data) len = recvfrom(serverfd, &msg, sizeof(msg), 0, NULL, NULL); if (len > 0) { - notify(msg.status, msg.error, msg.buf); + notify(msg.status, msg.error, msg.level, msg.buf); } } while(1); diff --git a/core/swupdate.c b/core/swupdate.c index ef94bd9..a9c71cf 100644 --- a/core/swupdate.c +++ b/core/swupdate.c @@ -862,7 +862,7 @@ int main(int argc, char **argv) install_from_file(fname, opt_c); cleanup_files(&swcfg); - notify(SUCCESS, 0, 0); + notify(SUCCESS, 0, INFOLEVEL, NULL); if (postupdate(&swcfg, NULL) != 0) { ERROR("Post-update command execution failed."); } diff --git a/core/syslog.c b/core/syslog.c index d031ed1..88b1218 100644 --- a/core/syslog.c +++ b/core/syslog.c @@ -20,7 +20,7 @@ #include <stdio.h> #include "util.h" -static void syslog_notifier(RECOVERY_STATUS status, int error, const char *msg); +static void syslog_notifier(RECOVERY_STATUS status, int error, int level, const char *msg); int syslog_init(void) { @@ -28,7 +28,7 @@ int syslog_init(void) return register_notifier(syslog_notifier); } -void syslog_notifier(RECOVERY_STATUS status, int error, const char *msg) +void syslog_notifier(RECOVERY_STATUS status, int error, int level, const char *msg) { const char* statusMsg; @@ -45,7 +45,16 @@ void syslog_notifier(RECOVERY_STATUS status, int error, const char *msg) default: statusMsg = "UNKNOWN"; break; } - syslog(LOG_NOTICE, "%s%s %s\n", ((error != (int)RECOVERY_NO_ERROR) ? "FATAL_" : ""), statusMsg, msg); + int logprio = LOG_INFO; + switch (level) { + case ERRORLEVEL: logprio = LOG_ERR; break; + case WARNLEVEL: logprio = LOG_WARNING; break; + case INFOLEVEL: logprio = LOG_INFO; break; + case DEBUGLEVEL: + case TRACELEVEL: logprio = LOG_DEBUG; break; + } + + syslog(logprio, "%s%s %s\n", ((error != (int)RECOVERY_NO_ERROR) ? "FATAL_" : ""), statusMsg, msg); closelog(); } diff --git a/corelib/downloader.c b/corelib/downloader.c index 58589f3..27bb4e8 100644 --- a/corelib/downloader.c +++ b/corelib/downloader.c @@ -237,7 +237,7 @@ static RECOVERY_STATUS download_from_url(char *image_url, unsigned int retries, } /* We are starting a download */ - notify(DOWNLOAD, 0, 0); + notify(DOWNLOAD, 0, INFOLEVEL, NULL); curl_global_init(CURL_GLOBAL_ALL); curl_handle = curl_easy_init(); diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c index 5717807..5f48726 100644 --- a/corelib/lua_interface.c +++ b/corelib/lua_interface.c @@ -304,7 +304,7 @@ static int l_notify (lua_State *L) { const char *msg = luaL_checkstring (L, 3); if (strlen(msg)) - notify((RECOVERY_STATUS)status, (int)error, msg); + notify((RECOVERY_STATUS)status, (int)error, INFOLEVEL, msg); return 0; } diff --git a/corelib/network_thread.c b/corelib/network_thread.c index 747ee1b..8c2b3b6 100644 --- a/corelib/network_thread.c +++ b/corelib/network_thread.c @@ -56,6 +56,7 @@ struct msg_elem { RECOVERY_STATUS status; int error; + int level; char *msg; SIMPLEQ_ENTRY(msg_elem) next; }; @@ -76,7 +77,7 @@ static void clean_msg(char *msg, char drop) } } -static void network_notifier(RECOVERY_STATUS status, int error, const char *msg) +static void network_notifier(RECOVERY_STATUS status, int error, int level, const char *msg) { int len = msg ? strlen(msg) : 0; struct msg_elem *newmsg = (struct msg_elem *)calloc(1, sizeof(*newmsg) + len + 1); @@ -97,6 +98,7 @@ static void network_notifier(RECOVERY_STATUS status, int error, const char *msg) newmsg->status = status; newmsg->error = error; + newmsg->level = level; if (msg) { strncpy(newmsg->msg, msg, len); diff --git a/corelib/stream_interface.c b/corelib/stream_interface.c index fb56c96..404c116 100644 --- a/corelib/stream_interface.c +++ b/corelib/stream_interface.c @@ -309,14 +309,14 @@ void *network_initializer(void *data) /* handle installation requests (from either source) */ while (1) { - printf ("Main loop Daemon\n"); + TRACE("Main loop Daemon"); /* wait for someone to issue an install request */ pthread_mutex_lock(&stream_mutex); pthread_cond_wait(&stream_wkup, &stream_mutex); inst.status = RUN; pthread_mutex_unlock(&stream_mutex); - notify(START, RECOVERY_NO_ERROR, "Software Update started !"); + notify(START, RECOVERY_NO_ERROR, INFOLEVEL, "Software Update started !"); #ifdef CONFIG_MTD mtd_cleanup(); @@ -340,11 +340,11 @@ void *network_initializer(void *data) */ bootloader_env_set("recovery_status", "in_progress"); - notify(RUN, RECOVERY_NO_ERROR, "Installation in progress"); + notify(RUN, RECOVERY_NO_ERROR, INFOLEVEL, "Installation in progress"); ret = install_images(software, 0, 0); if (ret != 0) { bootloader_env_set("recovery_status", "failed"); - notify(FAILURE, RECOVERY_ERROR, "Installation failed !"); + notify(FAILURE, RECOVERY_ERROR, ERRORLEVEL, "Installation failed !"); inst.last_install = FAILURE; } else { @@ -353,12 +353,12 @@ void *network_initializer(void *data) * that it is not required to start recovery again */ bootloader_env_unset("recovery_status"); - notify(SUCCESS, RECOVERY_NO_ERROR, "SWUPDATE successful !"); + notify(SUCCESS, RECOVERY_NO_ERROR, INFOLEVEL, "SWUPDATE successful !"); inst.last_install = SUCCESS; } } else { inst.last_install = FAILURE; - notify(FAILURE, RECOVERY_ERROR, "Image invalid or corrupted. Not installing ..."); + notify(FAILURE, RECOVERY_ERROR, ERRORLEVEL, "Image invalid or corrupted. Not installing ..."); } swupdate_progress_end(inst.last_install); @@ -367,7 +367,7 @@ void *network_initializer(void *data) inst.status = IDLE; pthread_mutex_unlock(&stream_mutex); TRACE("Main thread sleep again !"); - notify(IDLE, RECOVERY_NO_ERROR, "Waiting for requests..."); + notify(IDLE, RECOVERY_NO_ERROR, INFOLEVEL, "Waiting for requests..."); /* release temp files we may have created */ cleanup_files(software); diff --git a/handlers/boot_handler.c b/handlers/boot_handler.c index 41f87f7..86c55d0 100644 --- a/handlers/boot_handler.c +++ b/handlers/boot_handler.c @@ -39,7 +39,6 @@ static int install_boot_environment(struct img_type *img, { int ret; int fdout; - char buf[64]; char filename[64]; struct stat statbuf; @@ -62,12 +61,13 @@ static int install_boot_environment(struct img_type *img, } ret = bootloader_apply_list(filename); - if (ret < 0) - snprintf(buf, sizeof(buf), "Error setting bootloader environment"); - else - snprintf(buf, sizeof(buf), "Bootloader environment updated"); - - notify(RUN, RECOVERY_NO_ERROR, buf); + if (ret < 0) { + notify(RUN, RECOVERY_NO_ERROR, ERRORLEVEL, + "Error setting bootloader environment"); + } else { + notify(RUN, RECOVERY_NO_ERROR, INFOLEVEL, + "Bootloader environment updated"); + } return ret; } diff --git a/handlers/ubivol_handler.c b/handlers/ubivol_handler.c index bf58658..0e262b6 100644 --- a/handlers/ubivol_handler.c +++ b/handlers/ubivol_handler.c @@ -100,7 +100,7 @@ static int update_volume(libubi_t libubi, struct img_type *img, snprintf(sbuf, sizeof(sbuf), "Installing image %s into volume %s(%s)", img->fname, node, img->volname); - notify(RUN, RECOVERY_NO_ERROR, sbuf); + notify(RUN, RECOVERY_NO_ERROR, INFOLEVEL, sbuf); TRACE("Updating UBI : %s %lld\n", img->fname, img->size); diff --git a/include/util.h b/include/util.h index cc0f465..24ec127 100644 --- a/include/util.h +++ b/include/util.h @@ -70,7 +70,7 @@ struct installer { char info[2048]; /* info */ }; -typedef void (*notifier) (RECOVERY_STATUS status, int level, const char *msg); +typedef void (*notifier) (RECOVERY_STATUS status, int error, int level, const char *msg); #define swupdate_notify(status, format, level, arg...) do { \ if (loglevel >= level) { \ @@ -86,11 +86,11 @@ typedef void (*notifier) (RECOVERY_STATUS status, int level, const char *msg); else \ snprintf(tmpbuf, sizeof(tmpbuf), \ "ERROR : " format, ## arg); \ - notify(FAILURE, 0, tmpbuf); \ + notify(FAILURE, 0, level, tmpbuf); \ } else {\ snprintf(tmpbuf, sizeof(tmpbuf), \ "[%s] : " format, __func__, ## arg); \ - notify(RUN, RECOVERY_NO_ERROR, tmpbuf); \ + notify(RUN, RECOVERY_NO_ERROR, level, tmpbuf); \ } \ } \ } while(0) @@ -155,7 +155,7 @@ off_t extract_next_file(int fd, int fdout, off_t start, int compressed, int openfileoutput(const char *filename); int register_notifier(notifier client); -void notify(RECOVERY_STATUS status, int level, const char *msg); +void notify(RECOVERY_STATUS status, int error, int level, const char *msg); void notify_init(void); int syslog_init(void); diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c index 243db4f..1b0e600 100644 --- a/suricatta/server_hawkbit.c +++ b/suricatta/server_hawkbit.c @@ -301,9 +301,9 @@ static void check_action_changed(int action_id, const char *update_action) if (ENOMEM_ASPRINTF == asprintf(¬ifybuf, "{ \"id\" : \"%d\", \"update\" : \"%s\"}", action_id, server_hawkbit.update_action)) { - notify(SUBPROCESS, CHANGE, "Update type changed by server"); + notify(SUBPROCESS, CHANGE, DEBUGLEVEL, "Update type changed by server"); } else { - notify(SUBPROCESS, CHANGE, notifybuf); + notify(SUBPROCESS, CHANGE, DEBUGLEVEL, notifybuf); free(notifybuf); } } @@ -406,9 +406,9 @@ cleanup: if (ENOMEM_ASPRINTF == asprintf(¬ifybuf, "{ \"id\" : \"%d\", \"stopId\" : \"%d\"}", action_id, stop_id)) { - notify(SUBPROCESS, CANCELUPDATE, "Update cancelled"); + notify(SUBPROCESS, CANCELUPDATE, INFOLEVEL, "Update cancelled"); } else { - notify(SUBPROCESS, CANCELUPDATE, notifybuf); + notify(SUBPROCESS, CANCELUPDATE, INFOLEVEL, notifybuf); free(notifybuf); }
Although there are distinct macros for logging messages of different priorities (INFO, DEBUG, WARN, ...), the different notifiers emitting the log messages cannot make use of this distinction and log messages with the same uniform priority. Hence, pass through the loglevel to the notifiers emitting the log messages so that they may log respecting the intended priority. Consequently, adapt the notifiers to make use of this information, e.g., console_notifier now prefixes log messages with their priority information: ... [TRACE] : SWUPDATE running : [network_initializer] : Valid image found: copying to FLASH [INFO ] : SWUPDATE running : Installation in progress ... Signed-off-by: Christian Storm <christian.storm@siemens.com> --- core/notifier.c | 34 ++++++++++++++++++++++++++++------ core/swupdate.c | 2 +- core/syslog.c | 15 ++++++++++++--- corelib/downloader.c | 2 +- corelib/lua_interface.c | 2 +- corelib/network_thread.c | 4 +++- corelib/stream_interface.c | 14 +++++++------- handlers/boot_handler.c | 14 +++++++------- handlers/ubivol_handler.c | 2 +- include/util.h | 8 ++++---- suricatta/server_hawkbit.c | 8 ++++---- 11 files changed, 69 insertions(+), 36 deletions(-)