| Message ID | 20250716085414.86242-1-stefano.babic@swupdate.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add reboot to the list of parms for update type | expand |
Hi, thanks for this feature addition! > On 16. Jul 2025, at 10:54, Stefano Babic <stefano.babic@swupdate.org> wrote: > > Let that an update type can set if a reboot will be required or not, > without having to set it explicitely in sw-description. This simplifies > the setup for the user, because it is statically defined if an update of > a certain type should be followed by a reboot or not. > > For compatibility, setting "reboot" in sw-description is still allowed, > and this has a gigher priority than the value for the type. > > Signed-off-by: Stefano Babic <stefano.babic@swupdate.org> > Suggested-by: Christian Storm <christian.storm@siemens.com> > Suggested-by: Ian Dannapel <Ian.Dannapel@iris-sensing.com> > --- > core/stream_interface.c | 16 ++++++++++++++-- > core/swupdate.c | 18 ++++++++++++++++++ > doc/source/sw-description.rst | 5 +++++ > include/swupdate.h | 12 +++++++++++- > include/util.h | 9 +++++++++ > parser/parser.c | 8 +++++--- > 6 files changed, 62 insertions(+), 6 deletions(-) > > diff --git a/core/stream_interface.c b/core/stream_interface.c > index fe7a952d..d8e8850e 100644 > --- a/core/stream_interface.c > +++ b/core/stream_interface.c > @@ -58,6 +58,16 @@ enum { > > static pthread_t network_thread_id; > > +static bool check_reboot_required(struct swupdate_cfg *sw) { > + if (sw->reboot_required != REBOOT_UNSET) > + return sw->reboot_required == REBOOT_REQUIRED; > + struct swupdate_type_cfg *typecfg = sw->update_type; > + if (typecfg->reboot_required != REBOOT_UNSET) > + return typecfg->reboot_required == REBOOT_REQUIRED; > + > + return REBOOT_REQUIRED; Hm, this is boolean "by chance", why not return literally `true`? The other return cases are "true" boolean values... > +} > + > /* > * NOTE: these sync vars are _not_ static, they are _shared_ between the > * installer, the display thread and the network thread > @@ -562,6 +572,7 @@ void *network_initializer(void *data) > struct swupdate_cfg *software = data; > struct swupdate_request *req; > struct swupdate_parms parms; > + bool do_reboot = true; > > /* No installation in progress */ > memset(&inst, 0, sizeof(inst)); > @@ -711,7 +722,8 @@ void *network_initializer(void *data) > * if this update is signalled to be without reboot (On the Fly), > * send a notification via progress for processes responsible for booting > */ > - if (!software->reboot_required) { > + do_reboot = check_reboot_required(software); > + if (!do_reboot) { If check_reboot_required() returns bool, i.e., true on reboot required and false otherwise, then the `!` is wrong. > swupdate_progress_info(RUN, CAUSE_REBOOT_MODE , "{ \"reboot-mode\" : \"no-reboot\"}"); > } > > @@ -778,7 +790,7 @@ void *network_initializer(void *data) > */ > if (req->source == SOURCE_SURICATTA && > /*simple check without JSON */ strstr(req->info, "hawkbit") && > - !software->reboot_required) { > + !do_reboot) { Same here. > ipc_message msg; > size_t size = sizeof(msg.data.procmsg.buf); > char *buf = msg.data.procmsg.buf; > diff --git a/core/swupdate.c b/core/swupdate.c > index c844cb06..60f428b2 100644 > --- a/core/swupdate.c > +++ b/core/swupdate.c > @@ -308,6 +308,8 @@ static int parse_cert_purpose(const char *text) > > static void read_updatetype_settings(void *elem, struct swupdate_type_cfg *typecfg) > { > + char tmp[SWUPDATE_GENERAL_STRING_SIZE] = ""; > + > GET_FIELD_STRING(LIBCFG_PARSER, elem, > "postupdatecmd", typecfg->postupdatecmd); > GET_FIELD_STRING(LIBCFG_PARSER, elem, > @@ -326,6 +328,22 @@ static void read_updatetype_settings(void *elem, struct swupdate_type_cfg *typec > "no-reinstalling", typecfg->current_version); > if (strlen(typecfg->current_version)) > typecfg->no_reinstalling = true; > + > + /* > + * As default, reboot is initiated > + */ > + GET_FIELD_STRING(LIBCFG_PARSER, elem, > + "reboot", tmp); Superfluous line break? > + typecfg->reboot_required = REBOOT_UNSET; > + if (tmp[0] != '\0') { > + if (!is_enabled_disabled(tmp)) { > + WARN("Possible mismatch for reboot attribute, it is neither enabled nor disabled"); > + } else if (is_enabled(tmp)) { > + typecfg->reboot_required = REBOOT_REQUIRED; > + } else { > + typecfg->reboot_required = REBOOT_DISABLED; > + } > + } > } > > static int read_globals_settings(void *elem, void *data) > diff --git a/doc/source/sw-description.rst b/doc/source/sw-description.rst > index 3c568848..494b2402 100644 > --- a/doc/source/sw-description.rst > +++ b/doc/source/sw-description.rst > @@ -1277,6 +1277,7 @@ because the specific typoe was not found. In swupdate.cfg there will be somethin > preupdatecmd = "/usr/bin/echo Hello"; > max-version = "8.99"; > no-reinstalling = "5.5"; > + reboot = "enabled"; Hm, can't this be boolean, i.e., reboot = true; ? I mean, this is a binary switch and the binary switch `reboot` in sw-description is of type boolean, so for symmetry reasons.. > }, > { > name = "Application"; > @@ -1285,6 +1286,7 @@ because the specific typoe was not found. In swupdate.cfg there will be somethin > preupdatecmd = "/usr/bin/echo Hello"; > max-version = "5.99"; > no-reinstalling = "4.5"; > + reboot = "disabled"; > } > ) > > @@ -1295,6 +1297,9 @@ Values in the "globals" are still identified with a type, that SWUpdate calls "d > A flag into swupdate.cfg (update-type-required, globals section) will force that "update-type" is > mandatory, and SWU without the type set will be simply rejected. > > +The flag reboot identifies if the update will be followed by a rebbot Typo: rebbot > or it is markeed as "no rebootable". Typo: markeed > +This can still be overwritten by the global attribute "reboot" in sw-description. > + > Embedded Script > --------------- > > diff --git a/include/swupdate.h b/include/swupdate.h > index b7f796e0..324585c6 100644 > --- a/include/swupdate.h > +++ b/include/swupdate.h > @@ -27,6 +27,15 @@ typedef enum { > INSTALL_FROM_STREAM > } swupdate_file_t; > > +/* > + * Type of reboot after an update > + */ > +typedef enum { > + REBOOT_UNSET, > + REBOOT_REQUIRED, > + REBOOT_DISABLED > +} swupdate_reboot_t; > + > struct extproc { > char name[SWUPDATE_GENERAL_STRING_SIZE]; > char exec[SWUPDATE_GENERAL_STRING_SIZE]; > @@ -51,6 +60,7 @@ struct swupdate_type_cfg { > bool no_downgrading; > bool no_reinstalling; > bool check_max_version; > + swupdate_reboot_t reboot_required; > LIST_ENTRY(swupdate_type_cfg) next; > }; > LIST_HEAD(swupdate_type_list, swupdate_type_cfg); > @@ -84,7 +94,7 @@ struct swupdate_cfg { > int cert_purpose; > bool no_transaction_marker; > bool no_state_marker; > - bool reboot_required; > + swupdate_reboot_t reboot_required; > struct hw_type hw; > struct hwlist hardware; > struct swver installed_sw_list; > diff --git a/include/util.h b/include/util.h > index 00097246..bc0ae63b 100644 > --- a/include/util.h > +++ b/include/util.h > @@ -331,3 +331,12 @@ char *swupdate_time_iso8601(struct timeval *tv); > /* eMMC functions */ > int emmc_write_bootpart(int fd, int bootpart); > int emmc_get_active_bootpart(int fd); > + > +/* used to compare if an attribute is set to enabled or disabled */ > + > +static inline bool is_enabled_disabled(const char *s) { > + return (!(strcmp(s, "enabled") && strcmp(s, "disabled"))); > +} > +static inline bool is_enabled(const char *s) { > + return (!strcmp(s, "enabled")); > +} > diff --git a/parser/parser.c b/parser/parser.c > index 063451a0..b260558b 100644 > --- a/parser/parser.c > +++ b/parser/parser.c > @@ -220,12 +220,14 @@ static bool get_common_fields(parsertype p, void *cfg, struct swupdate_cfg *swcf > /* > * As default, reboot is initiated > */ > - swcfg->reboot_required = true; > + swcfg->reboot_required = REBOOT_UNSET; > if((setting = find_node(p, cfg, "reboot", swcfg)) != NULL) { > - GET_FIELD_BOOL(p, setting, NULL, &swcfg->reboot_required); > + bool must_reboot = true; > + GET_FIELD_BOOL(p, setting, NULL, &must_reboot); > + swcfg->reboot_required = must_reboot ? REBOOT_REQUIRED : REBOOT_DISABLED; > } > > - TRACE("reboot_required %d", swcfg->reboot_required); > + TRACE("reboot_required %d", swcfg->reboot_required != REBOOT_DISABLED); > > /* > * Check if SWU should be cached > -- > 2.43.0 > Kind regards, Christian
HI Christian, On 7/16/25 14:13, 'Storm, Christian' via swupdate wrote: > Hi, > > thanks for this feature addition! > >> On 16. Jul 2025, at 10:54, Stefano Babic <stefano.babic@swupdate.org> wrote: >> >> Let that an update type can set if a reboot will be required or not, >> without having to set it explicitely in sw-description. This simplifies >> the setup for the user, because it is statically defined if an update of >> a certain type should be followed by a reboot or not. >> >> For compatibility, setting "reboot" in sw-description is still allowed, >> and this has a gigher priority than the value for the type. >> >> Signed-off-by: Stefano Babic <stefano.babic@swupdate.org> >> Suggested-by: Christian Storm <christian.storm@siemens.com> >> Suggested-by: Ian Dannapel <Ian.Dannapel@iris-sensing.com> >> --- >> core/stream_interface.c | 16 ++++++++++++++-- >> core/swupdate.c | 18 ++++++++++++++++++ >> doc/source/sw-description.rst | 5 +++++ >> include/swupdate.h | 12 +++++++++++- >> include/util.h | 9 +++++++++ >> parser/parser.c | 8 +++++--- >> 6 files changed, 62 insertions(+), 6 deletions(-) >> >> diff --git a/core/stream_interface.c b/core/stream_interface.c >> index fe7a952d..d8e8850e 100644 >> --- a/core/stream_interface.c >> +++ b/core/stream_interface.c >> @@ -58,6 +58,16 @@ enum { >> >> static pthread_t network_thread_id; >> >> +static bool check_reboot_required(struct swupdate_cfg *sw) { >> + if (sw->reboot_required != REBOOT_UNSET) >> + return sw->reboot_required == REBOOT_REQUIRED; >> + struct swupdate_type_cfg *typecfg = sw->update_type; >> + if (typecfg->reboot_required != REBOOT_UNSET) >> + return typecfg->reboot_required == REBOOT_REQUIRED; >> + >> + return REBOOT_REQUIRED; > > Hm, this is boolean "by chance", why not return literally `true`? The other return cases are "true" boolean values... Right, I fix it. > > >> +} >> + >> /* >> * NOTE: these sync vars are _not_ static, they are _shared_ between the >> * installer, the display thread and the network thread >> @@ -562,6 +572,7 @@ void *network_initializer(void *data) >> struct swupdate_cfg *software = data; >> struct swupdate_request *req; >> struct swupdate_parms parms; >> + bool do_reboot = true; >> >> /* No installation in progress */ >> memset(&inst, 0, sizeof(inst)); >> @@ -711,7 +722,8 @@ void *network_initializer(void *data) >> * if this update is signalled to be without reboot (On the Fly), >> * send a notification via progress for processes responsible for booting >> */ >> - if (!software->reboot_required) { >> + do_reboot = check_reboot_required(software); >> + if (!do_reboot) { > > If check_reboot_required() returns bool, i.e., true on reboot required and false otherwise, then the `!` is wrong. Why ? I have not changed the logic here. !do_reboot means we do not want to reboot, and an INFO message must be sent. > > >> swupdate_progress_info(RUN, CAUSE_REBOOT_MODE , "{ \"reboot-mode\" : \"no-reboot\"}"); >> } >> >> @@ -778,7 +790,7 @@ void *network_initializer(void *data) >> */ >> if (req->source == SOURCE_SURICATTA && >> /*simple check without JSON */ strstr(req->info, "hawkbit") && >> - !software->reboot_required) { >> + !do_reboot) { > > Same here. I do not think so. > > >> ipc_message msg; >> size_t size = sizeof(msg.data.procmsg.buf); >> char *buf = msg.data.procmsg.buf; >> diff --git a/core/swupdate.c b/core/swupdate.c >> index c844cb06..60f428b2 100644 >> --- a/core/swupdate.c >> +++ b/core/swupdate.c >> @@ -308,6 +308,8 @@ static int parse_cert_purpose(const char *text) >> >> static void read_updatetype_settings(void *elem, struct swupdate_type_cfg *typecfg) >> { >> + char tmp[SWUPDATE_GENERAL_STRING_SIZE] = ""; >> + >> GET_FIELD_STRING(LIBCFG_PARSER, elem, >> "postupdatecmd", typecfg->postupdatecmd); >> GET_FIELD_STRING(LIBCFG_PARSER, elem, >> @@ -326,6 +328,22 @@ static void read_updatetype_settings(void *elem, struct swupdate_type_cfg *typec >> "no-reinstalling", typecfg->current_version); >> if (strlen(typecfg->current_version)) >> typecfg->no_reinstalling = true; >> + >> + /* >> + * As default, reboot is initiated >> + */ >> + GET_FIELD_STRING(LIBCFG_PARSER, elem, >> + "reboot", tmp); > > Superfluous line break? > I do not see this, do you see on the mail ? https://patchwork.ozlabs.org/project/swupdate/patch/20250716085414.86242-1-stefano.babic@swupdate.org/ >> + typecfg->reboot_required = REBOOT_UNSET; >> + if (tmp[0] != '\0') { >> + if (!is_enabled_disabled(tmp)) { >> + WARN("Possible mismatch for reboot attribute, it is neither enabled nor disabled"); >> + } else if (is_enabled(tmp)) { >> + typecfg->reboot_required = REBOOT_REQUIRED; >> + } else { >> + typecfg->reboot_required = REBOOT_DISABLED; >> + } >> + } >> } >> >> static int read_globals_settings(void *elem, void *data) >> diff --git a/doc/source/sw-description.rst b/doc/source/sw-description.rst >> index 3c568848..494b2402 100644 >> --- a/doc/source/sw-description.rst >> +++ b/doc/source/sw-description.rst >> @@ -1277,6 +1277,7 @@ because the specific typoe was not found. In swupdate.cfg there will be somethin >> preupdatecmd = "/usr/bin/echo Hello"; >> max-version = "8.99"; >> no-reinstalling = "5.5"; >> + reboot = "enabled"; > > Hm, can't this be boolean, i.e., reboot = true; ? I mean, this is a binary switch and the binary switch `reboot` in sw-description is of type boolean, so for symmetry reasons.. I have already three states, not set / enabled / disabled. And I cannot exclude that in future someone want to have something like "postponed2 or whatever. The fact that in sw-description is still boolean is for compatibility reason. If I switch here to enabled / disabled, that makes sense, it breaks compatibility. An option could be to accept both syntax in sw-description, and reboot = false; or reboot = "disabled"; are both valid. > > >> }, >> { >> name = "Application"; >> @@ -1285,6 +1286,7 @@ because the specific typoe was not found. In swupdate.cfg there will be somethin >> preupdatecmd = "/usr/bin/echo Hello"; >> max-version = "5.99"; >> no-reinstalling = "4.5"; >> + reboot = "disabled"; >> } >> ) >> >> @@ -1295,6 +1297,9 @@ Values in the "globals" are still identified with a type, that SWUpdate calls "d >> A flag into swupdate.cfg (update-type-required, globals section) will force that "update-type" is >> mandatory, and SWU without the type set will be simply rejected. >> >> +The flag reboot identifies if the update will be followed by a rebbot > > Typo: rebbot > Thanks. >> or it is markeed as "no rebootable". > > Typo: markeed > >> +This can still be overwritten by the global attribute "reboot" in sw-description. >> + >> Embedded Script >> --------------- >> >> diff --git a/include/swupdate.h b/include/swupdate.h >> index b7f796e0..324585c6 100644 >> --- a/include/swupdate.h >> +++ b/include/swupdate.h >> @@ -27,6 +27,15 @@ typedef enum { >> INSTALL_FROM_STREAM >> } swupdate_file_t; >> >> +/* >> + * Type of reboot after an update >> + */ >> +typedef enum { >> + REBOOT_UNSET, >> + REBOOT_REQUIRED, >> + REBOOT_DISABLED >> +} swupdate_reboot_t; >> + >> struct extproc { >> char name[SWUPDATE_GENERAL_STRING_SIZE]; >> char exec[SWUPDATE_GENERAL_STRING_SIZE]; >> @@ -51,6 +60,7 @@ struct swupdate_type_cfg { >> bool no_downgrading; >> bool no_reinstalling; >> bool check_max_version; >> + swupdate_reboot_t reboot_required; >> LIST_ENTRY(swupdate_type_cfg) next; >> }; >> LIST_HEAD(swupdate_type_list, swupdate_type_cfg); >> @@ -84,7 +94,7 @@ struct swupdate_cfg { >> int cert_purpose; >> bool no_transaction_marker; >> bool no_state_marker; >> - bool reboot_required; >> + swupdate_reboot_t reboot_required; >> struct hw_type hw; >> struct hwlist hardware; >> struct swver installed_sw_list; >> diff --git a/include/util.h b/include/util.h >> index 00097246..bc0ae63b 100644 >> --- a/include/util.h >> +++ b/include/util.h >> @@ -331,3 +331,12 @@ char *swupdate_time_iso8601(struct timeval *tv); >> /* eMMC functions */ >> int emmc_write_bootpart(int fd, int bootpart); >> int emmc_get_active_bootpart(int fd); >> + >> +/* used to compare if an attribute is set to enabled or disabled */ >> + >> +static inline bool is_enabled_disabled(const char *s) { >> + return (!(strcmp(s, "enabled") && strcmp(s, "disabled"))); >> +} >> +static inline bool is_enabled(const char *s) { >> + return (!strcmp(s, "enabled")); >> +} >> diff --git a/parser/parser.c b/parser/parser.c >> index 063451a0..b260558b 100644 >> --- a/parser/parser.c >> +++ b/parser/parser.c >> @@ -220,12 +220,14 @@ static bool get_common_fields(parsertype p, void *cfg, struct swupdate_cfg *swcf >> /* >> * As default, reboot is initiated >> */ >> - swcfg->reboot_required = true; >> + swcfg->reboot_required = REBOOT_UNSET; >> if((setting = find_node(p, cfg, "reboot", swcfg)) != NULL) { >> - GET_FIELD_BOOL(p, setting, NULL, &swcfg->reboot_required); >> + bool must_reboot = true; >> + GET_FIELD_BOOL(p, setting, NULL, &must_reboot); >> + swcfg->reboot_required = must_reboot ? REBOOT_REQUIRED : REBOOT_DISABLED; >> } >> >> - TRACE("reboot_required %d", swcfg->reboot_required); >> + TRACE("reboot_required %d", swcfg->reboot_required != REBOOT_DISABLED); >> >> /* >> * Check if SWU should be cached >> -- >> 2.43.0 >> > > > > Kind regards, > Christian > Best regards, Stefano
Hi Stefano, >>> /* >>> * NOTE: these sync vars are _not_ static, they are _shared_ between the >>> * installer, the display thread and the network thread >>> @@ -562,6 +572,7 @@ void *network_initializer(void *data) >>> struct swupdate_cfg *software = data; >>> struct swupdate_request *req; >>> struct swupdate_parms parms; >>> + bool do_reboot = true; >>> >>> /* No installation in progress */ >>> memset(&inst, 0, sizeof(inst)); >>> @@ -711,7 +722,8 @@ void *network_initializer(void *data) >>> * if this update is signalled to be without reboot (On the Fly), >>> * send a notification via progress for processes responsible for booting >>> */ >>> - if (!software->reboot_required) { >>> + do_reboot = check_reboot_required(software); >>> + if (!do_reboot) { >> If check_reboot_required() returns bool, i.e., true on reboot required and false otherwise, then the `!` is wrong. > > Why ? > > I have not changed the logic here. !do_reboot means we do not want to reboot, and an INFO message must be sent. Yes, my bad! I inverted the logics. >>> swupdate_progress_info(RUN, CAUSE_REBOOT_MODE , "{ \"reboot-mode\" : \"no-reboot\"}"); >>> } >>> >>> @@ -778,7 +790,7 @@ void *network_initializer(void *data) >>> */ >>> if (req->source == SOURCE_SURICATTA && >>> /*simple check without JSON */ strstr(req->info, "hawkbit") && >>> - !software->reboot_required) { >>> + !do_reboot) { >> Same here. > > I do not think so. ... and of course you're right :) >>> ipc_message msg; >>> size_t size = sizeof(msg.data.procmsg.buf); >>> char *buf = msg.data.procmsg.buf; >>> diff --git a/core/swupdate.c b/core/swupdate.c >>> index c844cb06..60f428b2 100644 >>> --- a/core/swupdate.c >>> +++ b/core/swupdate.c >>> @@ -308,6 +308,8 @@ static int parse_cert_purpose(const char *text) >>> >>> static void read_updatetype_settings(void *elem, struct swupdate_type_cfg *typecfg) >>> { >>> + char tmp[SWUPDATE_GENERAL_STRING_SIZE] = ""; >>> + >>> GET_FIELD_STRING(LIBCFG_PARSER, elem, >>> "postupdatecmd", typecfg->postupdatecmd); >>> GET_FIELD_STRING(LIBCFG_PARSER, elem, >>> @@ -326,6 +328,22 @@ static void read_updatetype_settings(void *elem, struct swupdate_type_cfg *typec >>> "no-reinstalling", typecfg->current_version); >>> if (strlen(typecfg->current_version)) >>> typecfg->no_reinstalling = true; >>> + >>> + /* >>> + * As default, reboot is initiated >>> + */ >>> + GET_FIELD_STRING(LIBCFG_PARSER, elem, >>> + "reboot", tmp); >> Superfluous line break? > > I do not see this, do you see on the mail ? > > https://patchwork.ozlabs.org/project/swupdate/patch/20250716085414.86242-1-stefano.babic@swupdate.org/ Yes, I see it there and as well in the Patchwork-attached patch. Strange... Shouldn't it read GET_FIELD_STRING(LIBCFG_PARSER, elem, "reboot", tmp); i.e., on one line? >>> + typecfg->reboot_required = REBOOT_UNSET; >>> + if (tmp[0] != '\0') { >>> + if (!is_enabled_disabled(tmp)) { >>> + WARN("Possible mismatch for reboot attribute, it is neither enabled nor disabled"); >>> + } else if (is_enabled(tmp)) { >>> + typecfg->reboot_required = REBOOT_REQUIRED; >>> + } else { >>> + typecfg->reboot_required = REBOOT_DISABLED; >>> + } >>> + } >>> } >>> >>> static int read_globals_settings(void *elem, void *data) >>> diff --git a/doc/source/sw-description.rst b/doc/source/sw-description.rst >>> index 3c568848..494b2402 100644 >>> --- a/doc/source/sw-description.rst >>> +++ b/doc/source/sw-description.rst >>> @@ -1277,6 +1277,7 @@ because the specific typoe was not found. In swupdate.cfg there will be somethin >>> preupdatecmd = "/usr/bin/echo Hello"; >>> max-version = "8.99"; >>> no-reinstalling = "5.5"; >>> + reboot = "enabled"; >> Hm, can't this be boolean, i.e., reboot = true; ? I mean, this is a binary switch and the binary switch `reboot` in sw-description is of type boolean, so for symmetry reasons.. > > I have already three states, not set / enabled / disabled. And I cannot exclude that in future someone want to have something like "postponed2 or whatever. Yes, currently it's unused but the infrastructure for future proof is laid out. > The fact that in sw-description is still boolean is for compatibility reason. If I switch here to enabled / disabled, that makes sense, it breaks compatibility. An option could be to accept both syntax in sw-description, and > > reboot = false; > > or > > reboot = "disabled"; > > are both valid. That would be an option as well, yes, thanks. To be honest, I don't care too much what type the setting is of but rather that it's symmetric. Kind regards, Christian
diff --git a/core/stream_interface.c b/core/stream_interface.c index fe7a952d..d8e8850e 100644 --- a/core/stream_interface.c +++ b/core/stream_interface.c @@ -58,6 +58,16 @@ enum { static pthread_t network_thread_id; +static bool check_reboot_required(struct swupdate_cfg *sw) { + if (sw->reboot_required != REBOOT_UNSET) + return sw->reboot_required == REBOOT_REQUIRED; + struct swupdate_type_cfg *typecfg = sw->update_type; + if (typecfg->reboot_required != REBOOT_UNSET) + return typecfg->reboot_required == REBOOT_REQUIRED; + + return REBOOT_REQUIRED; +} + /* * NOTE: these sync vars are _not_ static, they are _shared_ between the * installer, the display thread and the network thread @@ -562,6 +572,7 @@ void *network_initializer(void *data) struct swupdate_cfg *software = data; struct swupdate_request *req; struct swupdate_parms parms; + bool do_reboot = true; /* No installation in progress */ memset(&inst, 0, sizeof(inst)); @@ -711,7 +722,8 @@ void *network_initializer(void *data) * if this update is signalled to be without reboot (On the Fly), * send a notification via progress for processes responsible for booting */ - if (!software->reboot_required) { + do_reboot = check_reboot_required(software); + if (!do_reboot) { swupdate_progress_info(RUN, CAUSE_REBOOT_MODE , "{ \"reboot-mode\" : \"no-reboot\"}"); } @@ -778,7 +790,7 @@ void *network_initializer(void *data) */ if (req->source == SOURCE_SURICATTA && /*simple check without JSON */ strstr(req->info, "hawkbit") && - !software->reboot_required) { + !do_reboot) { ipc_message msg; size_t size = sizeof(msg.data.procmsg.buf); char *buf = msg.data.procmsg.buf; diff --git a/core/swupdate.c b/core/swupdate.c index c844cb06..60f428b2 100644 --- a/core/swupdate.c +++ b/core/swupdate.c @@ -308,6 +308,8 @@ static int parse_cert_purpose(const char *text) static void read_updatetype_settings(void *elem, struct swupdate_type_cfg *typecfg) { + char tmp[SWUPDATE_GENERAL_STRING_SIZE] = ""; + GET_FIELD_STRING(LIBCFG_PARSER, elem, "postupdatecmd", typecfg->postupdatecmd); GET_FIELD_STRING(LIBCFG_PARSER, elem, @@ -326,6 +328,22 @@ static void read_updatetype_settings(void *elem, struct swupdate_type_cfg *typec "no-reinstalling", typecfg->current_version); if (strlen(typecfg->current_version)) typecfg->no_reinstalling = true; + + /* + * As default, reboot is initiated + */ + GET_FIELD_STRING(LIBCFG_PARSER, elem, + "reboot", tmp); + typecfg->reboot_required = REBOOT_UNSET; + if (tmp[0] != '\0') { + if (!is_enabled_disabled(tmp)) { + WARN("Possible mismatch for reboot attribute, it is neither enabled nor disabled"); + } else if (is_enabled(tmp)) { + typecfg->reboot_required = REBOOT_REQUIRED; + } else { + typecfg->reboot_required = REBOOT_DISABLED; + } + } } static int read_globals_settings(void *elem, void *data) diff --git a/doc/source/sw-description.rst b/doc/source/sw-description.rst index 3c568848..494b2402 100644 --- a/doc/source/sw-description.rst +++ b/doc/source/sw-description.rst @@ -1277,6 +1277,7 @@ because the specific typoe was not found. In swupdate.cfg there will be somethin preupdatecmd = "/usr/bin/echo Hello"; max-version = "8.99"; no-reinstalling = "5.5"; + reboot = "enabled"; }, { name = "Application"; @@ -1285,6 +1286,7 @@ because the specific typoe was not found. In swupdate.cfg there will be somethin preupdatecmd = "/usr/bin/echo Hello"; max-version = "5.99"; no-reinstalling = "4.5"; + reboot = "disabled"; } ) @@ -1295,6 +1297,9 @@ Values in the "globals" are still identified with a type, that SWUpdate calls "d A flag into swupdate.cfg (update-type-required, globals section) will force that "update-type" is mandatory, and SWU without the type set will be simply rejected. +The flag reboot identifies if the update will be followed by a rebbot or it is markeed as "no rebootable". +This can still be overwritten by the global attribute "reboot" in sw-description. + Embedded Script --------------- diff --git a/include/swupdate.h b/include/swupdate.h index b7f796e0..324585c6 100644 --- a/include/swupdate.h +++ b/include/swupdate.h @@ -27,6 +27,15 @@ typedef enum { INSTALL_FROM_STREAM } swupdate_file_t; +/* + * Type of reboot after an update + */ +typedef enum { + REBOOT_UNSET, + REBOOT_REQUIRED, + REBOOT_DISABLED +} swupdate_reboot_t; + struct extproc { char name[SWUPDATE_GENERAL_STRING_SIZE]; char exec[SWUPDATE_GENERAL_STRING_SIZE]; @@ -51,6 +60,7 @@ struct swupdate_type_cfg { bool no_downgrading; bool no_reinstalling; bool check_max_version; + swupdate_reboot_t reboot_required; LIST_ENTRY(swupdate_type_cfg) next; }; LIST_HEAD(swupdate_type_list, swupdate_type_cfg); @@ -84,7 +94,7 @@ struct swupdate_cfg { int cert_purpose; bool no_transaction_marker; bool no_state_marker; - bool reboot_required; + swupdate_reboot_t reboot_required; struct hw_type hw; struct hwlist hardware; struct swver installed_sw_list; diff --git a/include/util.h b/include/util.h index 00097246..bc0ae63b 100644 --- a/include/util.h +++ b/include/util.h @@ -331,3 +331,12 @@ char *swupdate_time_iso8601(struct timeval *tv); /* eMMC functions */ int emmc_write_bootpart(int fd, int bootpart); int emmc_get_active_bootpart(int fd); + +/* used to compare if an attribute is set to enabled or disabled */ + +static inline bool is_enabled_disabled(const char *s) { + return (!(strcmp(s, "enabled") && strcmp(s, "disabled"))); +} +static inline bool is_enabled(const char *s) { + return (!strcmp(s, "enabled")); +} diff --git a/parser/parser.c b/parser/parser.c index 063451a0..b260558b 100644 --- a/parser/parser.c +++ b/parser/parser.c @@ -220,12 +220,14 @@ static bool get_common_fields(parsertype p, void *cfg, struct swupdate_cfg *swcf /* * As default, reboot is initiated */ - swcfg->reboot_required = true; + swcfg->reboot_required = REBOOT_UNSET; if((setting = find_node(p, cfg, "reboot", swcfg)) != NULL) { - GET_FIELD_BOOL(p, setting, NULL, &swcfg->reboot_required); + bool must_reboot = true; + GET_FIELD_BOOL(p, setting, NULL, &must_reboot); + swcfg->reboot_required = must_reboot ? REBOOT_REQUIRED : REBOOT_DISABLED; } - TRACE("reboot_required %d", swcfg->reboot_required); + TRACE("reboot_required %d", swcfg->reboot_required != REBOOT_DISABLED); /* * Check if SWU should be cached
Let that an update type can set if a reboot will be required or not, without having to set it explicitely in sw-description. This simplifies the setup for the user, because it is statically defined if an update of a certain type should be followed by a reboot or not. For compatibility, setting "reboot" in sw-description is still allowed, and this has a gigher priority than the value for the type. Signed-off-by: Stefano Babic <stefano.babic@swupdate.org> Suggested-by: Christian Storm <christian.storm@siemens.com> Suggested-by: Ian Dannapel <Ian.Dannapel@iris-sensing.com> --- core/stream_interface.c | 16 ++++++++++++++-- core/swupdate.c | 18 ++++++++++++++++++ doc/source/sw-description.rst | 5 +++++ include/swupdate.h | 12 +++++++++++- include/util.h | 9 +++++++++ parser/parser.c | 8 +++++--- 6 files changed, 62 insertions(+), 6 deletions(-)