| Message ID | 20190417084342.17461-2-christian.storm@siemens.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | None | expand |
On 17/04/19 10:43, Christian Storm wrote: > Consolidate transaction marker handling that directly uses the > bootloader interface to use the state handling functions instead. > Then, persistent state in the bootloader environment is solely > managed via state's functions. > > Signed-off-by: Christian Storm <christian.storm@siemens.com> > --- > core/state.c | 17 +++++++++++------ > core/swupdate.c | 8 ++++---- > corelib/stream_interface.c | 10 +++++----- > include/state.h | 20 +++++++++++++++++++- > 4 files changed, 39 insertions(+), 16 deletions(-) > > diff --git a/core/state.c b/core/state.c > index a369ad2..45e461b 100644 > --- a/core/state.c > +++ b/core/state.c > @@ -26,16 +26,21 @@ > } \ > } while(0) > > -server_op_res_t save_state(char *key, update_state_t value) > +static server_op_res_t do_save_state(char *key, char* value) > { > - int ret; > - char value_str[2] = {value, '\0'}; > - > CHECK_STATE_VAR(key); > + return bootloader_env_set(key, value) == 0 ? SERVER_OK : SERVER_EERR; > +} > > - ret = bootloader_env_set(key, value_str); > +server_op_res_t save_state(char *key, update_state_t value) > +{ > + char value_str[2] = {value, '\0'}; > + return do_save_state(key, value_str); > +} > > - return ret == 0 ? SERVER_OK : SERVER_EERR; > +server_op_res_t save_state_string(char *key, update_state_t value) > +{ > + return do_save_state(key, get_state_string(value)); > } > > server_op_res_t read_state(char *key, update_state_t *value) > diff --git a/core/swupdate.c b/core/swupdate.c > index 55d3345..febf12a 100644 > --- a/core/swupdate.c > +++ b/core/swupdate.c > @@ -46,7 +46,7 @@ > #include "parselib.h" > #include "swupdate_settings.h" > #include "pctl.h" > -#include "bootloader.h" > +#include "state.h" > > #ifdef CONFIG_SYSTEMD > #include <systemd/sd-daemon.h> > @@ -370,7 +370,7 @@ static int install_from_file(char *fname, int check) > * Set "recovery_status" as begin of the transaction" > */ > if (swcfg.bootloader_transaction_marker) { > - bootloader_env_set(BOOTVAR_TRANSACTION, "in_progress"); > + save_state_string((char*)BOOTVAR_TRANSACTION, STATE_IN_PROGRESS); > } > > ret = install_images(&swcfg, fdsw, 1); > @@ -385,7 +385,7 @@ static int install_from_file(char *fname, int check) > } > > if (swcfg.bootloader_transaction_marker) { > - bootloader_env_unset(BOOTVAR_TRANSACTION); > + reset_state((char*)BOOTVAR_TRANSACTION); > } > fprintf(stdout, "Software updated successfully\n"); > fprintf(stdout, "Please reboot the device to start the new software\n"); > @@ -974,7 +974,7 @@ int main(int argc, char **argv) > switch (result) { > case EXIT_FAILURE: > if (swcfg.bootloader_transaction_marker) { > - bootloader_env_set(BOOTVAR_TRANSACTION, "failed"); > + save_state_string((char*)BOOTVAR_TRANSACTION, STATE_FAILED); > } > break; > case EXIT_SUCCESS: > diff --git a/corelib/stream_interface.c b/corelib/stream_interface.c > index d352900..b7971d6 100644 > --- a/corelib/stream_interface.c > +++ b/corelib/stream_interface.c > @@ -42,7 +42,7 @@ > #include "installer.h" > #include "progress.h" > #include "pctl.h" > -#include "bootloader.h" > +#include "state.h" > > #define BUFF_SIZE 4096 > #define PERCENT_LB_INDEX 4 > @@ -238,7 +238,7 @@ static int extract_files(int fd, struct swupdate_cfg *software) > */ > if (!installed_directly) { > if (software->bootloader_transaction_marker) { > - bootloader_env_set(BOOTVAR_TRANSACTION, "in_progress"); > + save_state_string((char*)BOOTVAR_TRANSACTION, STATE_IN_PROGRESS); > } > installed_directly = true; > } > @@ -396,14 +396,14 @@ void *network_initializer(void *data) > * initiated an update > */ > if (software->bootloader_transaction_marker) { > - bootloader_env_set(BOOTVAR_TRANSACTION, "in_progress"); > + save_state_string((char*)BOOTVAR_TRANSACTION, STATE_IN_PROGRESS); > } > > notify(RUN, RECOVERY_NO_ERROR, INFOLEVEL, "Installation in progress"); > ret = install_images(software, 0, 0); > if (ret != 0) { > if (software->bootloader_transaction_marker) { > - bootloader_env_set(BOOTVAR_TRANSACTION, "failed"); > + save_state_string((char*)BOOTVAR_TRANSACTION, STATE_FAILED); > } > notify(FAILURE, RECOVERY_ERROR, ERRORLEVEL, "Installation failed !"); > inst.last_install = FAILURE; > @@ -414,7 +414,7 @@ void *network_initializer(void *data) > * that it is not required to start recovery again > */ > if (software->bootloader_transaction_marker) { > - bootloader_env_unset(BOOTVAR_TRANSACTION); > + reset_state((char*)BOOTVAR_TRANSACTION); > } > notify(SUCCESS, RECOVERY_NO_ERROR, INFOLEVEL, "SWUPDATE successful !"); > inst.last_install = SUCCESS; > diff --git a/include/state.h b/include/state.h > index 1538286..497a87d 100644 > --- a/include/state.h > +++ b/include/state.h > @@ -22,6 +22,13 @@ > * server instance after, e.g., a successful reboot into the new firmware. The > * `{save,read,reset}_state()` functions are called by a server implementation > * to persistently manage the update state via, e.g., U-Boot's environment. > + * > + * Besides suricatta, this mechanism is also used by SWUpdate's core for > + * setting an update transaction marker, i.e., the bootloader environment > + * variable BOOTVAR_TRANSACTION (default: "recovery_status") is set to > + * "in_progress" prior to an update operation and either unset or set to > + * "failed" after the update operation, depending on whether an sw-description's > + * "bootloader_transaction_marker" property is true which is the default. > */ > > typedef enum { > @@ -32,14 +39,25 @@ typedef enum { > STATE_NOT_AVAILABLE = '4', > STATE_ERROR = '5', > STATE_WAIT = '6', > - STATE_LAST = STATE_WAIT > + STATE_IN_PROGRESS = '7', > + STATE_LAST = STATE_IN_PROGRESS > } update_state_t; > > static inline bool is_valid_state(update_state_t state) { > return (state >= STATE_OK && state <= STATE_LAST); > } > > +static inline char* get_state_string(update_state_t state) { > + switch (state) { > + case STATE_IN_PROGRESS: return (char*)"in_progress"; > + case STATE_FAILED: return (char*)"failed"; > + default: break; > + } > + return (char*)state; > +} > + > server_op_res_t save_state(char *key, update_state_t value); > +server_op_res_t save_state_string(char *key, update_state_t value); > server_op_res_t read_state(char *key, update_state_t *value); > server_op_res_t reset_state(char *key); > update_state_t get_state(void); > Acked-by: Stefano Babic <sbabic@denx.de> Best regards, Stefano Babic
diff --git a/core/state.c b/core/state.c index a369ad2..45e461b 100644 --- a/core/state.c +++ b/core/state.c @@ -26,16 +26,21 @@ } \ } while(0) -server_op_res_t save_state(char *key, update_state_t value) +static server_op_res_t do_save_state(char *key, char* value) { - int ret; - char value_str[2] = {value, '\0'}; - CHECK_STATE_VAR(key); + return bootloader_env_set(key, value) == 0 ? SERVER_OK : SERVER_EERR; +} - ret = bootloader_env_set(key, value_str); +server_op_res_t save_state(char *key, update_state_t value) +{ + char value_str[2] = {value, '\0'}; + return do_save_state(key, value_str); +} - return ret == 0 ? SERVER_OK : SERVER_EERR; +server_op_res_t save_state_string(char *key, update_state_t value) +{ + return do_save_state(key, get_state_string(value)); } server_op_res_t read_state(char *key, update_state_t *value) diff --git a/core/swupdate.c b/core/swupdate.c index 55d3345..febf12a 100644 --- a/core/swupdate.c +++ b/core/swupdate.c @@ -46,7 +46,7 @@ #include "parselib.h" #include "swupdate_settings.h" #include "pctl.h" -#include "bootloader.h" +#include "state.h" #ifdef CONFIG_SYSTEMD #include <systemd/sd-daemon.h> @@ -370,7 +370,7 @@ static int install_from_file(char *fname, int check) * Set "recovery_status" as begin of the transaction" */ if (swcfg.bootloader_transaction_marker) { - bootloader_env_set(BOOTVAR_TRANSACTION, "in_progress"); + save_state_string((char*)BOOTVAR_TRANSACTION, STATE_IN_PROGRESS); } ret = install_images(&swcfg, fdsw, 1); @@ -385,7 +385,7 @@ static int install_from_file(char *fname, int check) } if (swcfg.bootloader_transaction_marker) { - bootloader_env_unset(BOOTVAR_TRANSACTION); + reset_state((char*)BOOTVAR_TRANSACTION); } fprintf(stdout, "Software updated successfully\n"); fprintf(stdout, "Please reboot the device to start the new software\n"); @@ -974,7 +974,7 @@ int main(int argc, char **argv) switch (result) { case EXIT_FAILURE: if (swcfg.bootloader_transaction_marker) { - bootloader_env_set(BOOTVAR_TRANSACTION, "failed"); + save_state_string((char*)BOOTVAR_TRANSACTION, STATE_FAILED); } break; case EXIT_SUCCESS: diff --git a/corelib/stream_interface.c b/corelib/stream_interface.c index d352900..b7971d6 100644 --- a/corelib/stream_interface.c +++ b/corelib/stream_interface.c @@ -42,7 +42,7 @@ #include "installer.h" #include "progress.h" #include "pctl.h" -#include "bootloader.h" +#include "state.h" #define BUFF_SIZE 4096 #define PERCENT_LB_INDEX 4 @@ -238,7 +238,7 @@ static int extract_files(int fd, struct swupdate_cfg *software) */ if (!installed_directly) { if (software->bootloader_transaction_marker) { - bootloader_env_set(BOOTVAR_TRANSACTION, "in_progress"); + save_state_string((char*)BOOTVAR_TRANSACTION, STATE_IN_PROGRESS); } installed_directly = true; } @@ -396,14 +396,14 @@ void *network_initializer(void *data) * initiated an update */ if (software->bootloader_transaction_marker) { - bootloader_env_set(BOOTVAR_TRANSACTION, "in_progress"); + save_state_string((char*)BOOTVAR_TRANSACTION, STATE_IN_PROGRESS); } notify(RUN, RECOVERY_NO_ERROR, INFOLEVEL, "Installation in progress"); ret = install_images(software, 0, 0); if (ret != 0) { if (software->bootloader_transaction_marker) { - bootloader_env_set(BOOTVAR_TRANSACTION, "failed"); + save_state_string((char*)BOOTVAR_TRANSACTION, STATE_FAILED); } notify(FAILURE, RECOVERY_ERROR, ERRORLEVEL, "Installation failed !"); inst.last_install = FAILURE; @@ -414,7 +414,7 @@ void *network_initializer(void *data) * that it is not required to start recovery again */ if (software->bootloader_transaction_marker) { - bootloader_env_unset(BOOTVAR_TRANSACTION); + reset_state((char*)BOOTVAR_TRANSACTION); } notify(SUCCESS, RECOVERY_NO_ERROR, INFOLEVEL, "SWUPDATE successful !"); inst.last_install = SUCCESS; diff --git a/include/state.h b/include/state.h index 1538286..497a87d 100644 --- a/include/state.h +++ b/include/state.h @@ -22,6 +22,13 @@ * server instance after, e.g., a successful reboot into the new firmware. The * `{save,read,reset}_state()` functions are called by a server implementation * to persistently manage the update state via, e.g., U-Boot's environment. + * + * Besides suricatta, this mechanism is also used by SWUpdate's core for + * setting an update transaction marker, i.e., the bootloader environment + * variable BOOTVAR_TRANSACTION (default: "recovery_status") is set to + * "in_progress" prior to an update operation and either unset or set to + * "failed" after the update operation, depending on whether an sw-description's + * "bootloader_transaction_marker" property is true which is the default. */ typedef enum { @@ -32,14 +39,25 @@ typedef enum { STATE_NOT_AVAILABLE = '4', STATE_ERROR = '5', STATE_WAIT = '6', - STATE_LAST = STATE_WAIT + STATE_IN_PROGRESS = '7', + STATE_LAST = STATE_IN_PROGRESS } update_state_t; static inline bool is_valid_state(update_state_t state) { return (state >= STATE_OK && state <= STATE_LAST); } +static inline char* get_state_string(update_state_t state) { + switch (state) { + case STATE_IN_PROGRESS: return (char*)"in_progress"; + case STATE_FAILED: return (char*)"failed"; + default: break; + } + return (char*)state; +} + server_op_res_t save_state(char *key, update_state_t value); +server_op_res_t save_state_string(char *key, update_state_t value); server_op_res_t read_state(char *key, update_state_t *value); server_op_res_t reset_state(char *key); update_state_t get_state(void);
Consolidate transaction marker handling that directly uses the bootloader interface to use the state handling functions instead. Then, persistent state in the bootloader environment is solely managed via state's functions. Signed-off-by: Christian Storm <christian.storm@siemens.com> --- core/state.c | 17 +++++++++++------ core/swupdate.c | 8 ++++---- corelib/stream_interface.c | 10 +++++----- include/state.h | 20 +++++++++++++++++++- 4 files changed, 39 insertions(+), 16 deletions(-)