diff mbox series

[V2,07/10] IPC: let decide selections at runtime

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

Commit Message

Stefano Babic Nov. 13, 2020, 8:31 a.m. UTC
It is useful to change the selection type at runtime if more as one
section inside sw-description can be installed. This activates a
selection sent via IPC to be used for the next update. The default
configuration is cached and will be active at next update if the request
does not contain any valid selection.

Signed-off-by: Stefano Babic <sbabic@denx.de>
---
 core/stream_interface.c | 19 ++++++++++++++++++-
 core/swupdate.c         |  8 +++++++-
 include/swupdate.h      |  2 ++
 3 files changed, 27 insertions(+), 2 deletions(-)

Comments

Stefan Herbrechtsmeier Nov. 14, 2020, 10:52 a.m. UTC | #1
Hi Stefano,

Am 13.11.20 um 09:31 schrieb Stefano Babic:
> It is useful to change the selection type at runtime if more as one
> section inside sw-description can be installed.

How does your solution works together with an a/b partition? Can I 
change the <software> only?

> This activates a
> selection sent via IPC to be used for the next update. The default
> configuration is cached and will be active at next update if the request
> does not contain any valid selection.

What happens if an other process send an update in between or the 
process dies?

Regards
   Stefan
Stefano Babic Nov. 14, 2020, 11:11 a.m. UTC | #2
Hi Stefan,

On 14.11.20 11:52, Stefan Herbrechtsmeier wrote:
> Hi Stefano,
> 
> Am 13.11.20 um 09:31 schrieb Stefano Babic:
>> It is useful to change the selection type at runtime if more as one
>> section inside sw-description can be installed.
> 
> How does your solution works together with an a/b partition? Can I 
> change the <software> only?

It should not change and it is independent from A/B. Which are your 
concerns ?

> 
>> This activates a
>> selection sent via IPC to be used for the next update. The default
>> configuration is cached and will be active at next update if the request
>> does not contain any valid selection.
> 
> What happens if an other process send an update in between or the 
> process dies?

This is already ruled inside SWUpdate. In fact, you can have already 
multiple install requests if sent by different interface. If you send at 
the same time an update through the Webserver, the Backend or an USB, 
the first one is accepted and the other ones are rejected because 
SWUpdate is busy. The IPC sends (and this is not changed by this series) 
a NACK to the issuer.

Regards,
Stefano
Stefan Herbrechtsmeier Nov. 14, 2020, 11:41 a.m. UTC | #3
Hi Stefano,

Am 14.11.20 um 12:11 schrieb Stefano Babic:
> On 14.11.20 11:52, Stefan Herbrechtsmeier wrote:
>> Am 13.11.20 um 09:31 schrieb Stefano Babic:
>>> It is useful to change the selection type at runtime if more as one
>>> section inside sw-description can be installed.
>>
>> How does your solution works together with an a/b partition? Can I 
>> change the <software> only?
> 
> It should not change and it is independent from A/B. Which are your 
> concerns ?

What is the use case of your changes?

Is it possible to change the <software> part of the selection and keep 
the <mode> to update an inactive A/B with a specific <software>?
>>> This activates a
>>> selection sent via IPC to be used for the next update. The default
>>> configuration is cached and will be active at next update if the request
>>> does not contain any valid selection.
>>
>> What happens if an other process send an update in between or the 
>> process dies?
> 
> This is already ruled inside SWUpdate. In fact, you can have already 
> multiple install requests if sent by different interface. If you send at 
> the same time an update through the Webserver, the Backend or an USB, 
> the first one is accepted and the other ones are rejected because 
> SWUpdate is busy. The IPC sends (and this is not changed by this series) 
> a NACK to the issuer.

What happens if the Webserver change the selection, an other Backend 
send an update and after the finished update the Webserver send an 
update. Is the changed selection only used for the Webserver update?

I think the selection belongs to the update command and shouldn't 
influence other updates. The IPC should be stateless like a REST api.

Regards
   Stefan
Stefano Babic Nov. 14, 2020, 2:34 p.m. UTC | #4
Hi Stefan,

On 14.11.20 12:41, Stefan Herbrechtsmeier wrote:
> Hi Stefano,
> 
> Am 14.11.20 um 12:11 schrieb Stefano Babic:
>> On 14.11.20 11:52, Stefan Herbrechtsmeier wrote:
>>> Am 13.11.20 um 09:31 schrieb Stefano Babic:
>>>> It is useful to change the selection type at runtime if more as one
>>>> section inside sw-description can be installed.
>>>
>>> How does your solution works together with an a/b partition? Can I
>>> change the <software> only?
>>
>> It should not change and it is independent from A/B. Which are your
>> concerns ?
> 
> What is the use case of your changes?

I try to simplify. Think about I have a more complex device that can be
extended with additional modules that can be plugged in optionally. The
update of the system is not affected and runs with A/B concept, but the
additional modules should be individually updated if available. An
application running a local GUI control the behavior and let the end
user decide which modules should be updated. The whole release is still
delivered in a single SWU.

> 
> Is it possible to change the <software> part of the selection and keep
> the <mode> to update an inactive A/B with a specific <software>?

I am afraid I do not really get the issue here. The couple
<software>,<mode> identifies the part of the sw-description related to
the update. I do not see any relevant change here regarding this.

>>>> This activates a
>>>> selection sent via IPC to be used for the next update. The default
>>>> configuration is cached and will be active at next update if the
>>>> request
>>>> does not contain any valid selection.
>>>
>>> What happens if an other process send an update in between or the
>>> process dies?
>>
>> This is already ruled inside SWUpdate. In fact, you can have already
>> multiple install requests if sent by different interface. If you send
>> at the same time an update through the Webserver, the Backend or an
>> USB, the first one is accepted and the other ones are rejected because
>> SWUpdate is busy. The IPC sends (and this is not changed by this
>> series) a NACK to the issuer.
> 
> What happens if the Webserver change the selection,

How ? It cannot...

The Webserver stills sends an update without any selection and then the
chosen selection is the default defined at the startup via -e parameter.

Not only: the selection (see patches) *belong* to an update to still
remain atomic. After an update is completed, the default selection takes
place again. So you scenario becomes:

- a *local* process sends an update with a different selection

- <update is running>
- new update request from an another interface (Webserver, Backend,
etc): rejected while former update is running
- update completed (success or not, it does not matter). At this point
the selection for this update is dropped.
- Webserver sends an update: the "default" selection is taken, it runs
as usual.		
- same thing if update is with a backend, the selection cannot be
changed from the startup: the one set via "-e" is still valid.


> an other Backend
> send an update and after the finished update the Webserver send an
> update. Is the changed selection only used for the Webserver update?

It is not used by any OTA update. It is just available for custom
applications and its life is just for the single update request.

> 
> I think the selection belongs to the update command

But this is how I have implemented: selection is part of the update
request and dies after update is completed.

> and shouldn't
> influence other updates.

I fully agree, but it is exactly in this way.....

> The IPC should be stateless like a REST api.

Regards,
Stefano
Stefan Herbrechtsmeier Nov. 14, 2020, 3:45 p.m. UTC | #5
Hi Stefano,

Am 14.11.20 um 15:34 schrieb Stefano Babic:
> On 14.11.20 12:41, Stefan Herbrechtsmeier wrote:
>> Am 14.11.20 um 12:11 schrieb Stefano Babic:
>>> On 14.11.20 11:52, Stefan Herbrechtsmeier wrote:
>>>> Am 13.11.20 um 09:31 schrieb Stefano Babic:
>>>>> It is useful to change the selection type at runtime if more as one
>>>>> section inside sw-description can be installed.
>>>>
>>>> How does your solution works together with an a/b partition? Can I
>>>> change the <software> only?
>>>
>>> It should not change and it is independent from A/B. Which are your
>>> concerns ?
>>
>> What is the use case of your changes?
> 
> I try to simplify. Think about I have a more complex device that can be
> extended with additional modules that can be plugged in optionally. The
> update of the system is not affected and runs with A/B concept, but the
> additional modules should be individually updated if available. An
> application running a local GUI control the behavior and let the end
> user decide which modules should be updated. The whole release is still
> delivered in a single SWU.

This sounds more like you want to mask parts of a swu.

>> Is it possible to change the <software> part of the selection and keep
>> the <mode> to update an inactive A/B with a specific <software>?
> 
> I am afraid I do not really get the issue here. The couple
> <software>,<mode> identifies the part of the sw-description related to
> the update. I do not see any relevant change here regarding this.

In the examples only the <mode> was used for the A/B. If the IPC only 
changes the <software> part you don't need a blacklist because you could 
always have a <mode> under the <software> to select the correct image / 
partition.

>>>>> This activates a
>>>>> selection sent via IPC to be used for the next update. The default
>>>>> configuration is cached and will be active at next update if the
>>>>> request
>>>>> does not contain any valid selection.
>>>>
>>>> What happens if an other process send an update in between or the
>>>> process dies?
>>>
>>> This is already ruled inside SWUpdate. In fact, you can have already
>>> multiple install requests if sent by different interface. If you send
>>> at the same time an update through the Webserver, the Backend or an
>>> USB, the first one is accepted and the other ones are rejected because
>>> SWUpdate is busy. The IPC sends (and this is not changed by this
>>> series) a NACK to the issuer.
>>
>> What happens if the Webserver change the selection,
> 
> How ? It cannot...
> 
> The Webserver stills sends an update without any selection and then the
> chosen selection is the default defined at the startup via -e parameter.
> 
> Not only: the selection (see patches) *belong* to an update to still
> remain atomic. After an update is completed, the default selection takes
> place again. So you scenario becomes:
> 
> - a *local* process sends an update with a different selection
> 
> - <update is running>
> - new update request from an another interface (Webserver, Backend,
> etc): rejected while former update is running
> - update completed (success or not, it does not matter). At this point
> the selection for this update is dropped.
> - Webserver sends an update: the "default" selection is taken, it runs
> as usual.		
> - same thing if update is with a backend, the selection cannot be
> changed from the startup: the one set via "-e" is still valid.
> 
> 
>> an other Backend
>> send an update and after the finished update the Webserver send an
>> update. Is the changed selection only used for the Webserver update?
> 
> It is not used by any OTA update. It is just available for custom
> applications and its life is just for the single update request.
> 
>>
>> I think the selection belongs to the update command
> 
> But this is how I have implemented: selection is part of the update
> request and dies after update is completed.
> 
>> and shouldn't
>> influence other updates.
> 
> I fully agree, but it is exactly in this way.....
> 
>> The IPC should be stateless like a REST api.

Sorry I get confused because you change the global config.

Regards
   Stefan
diff mbox series

Patch

diff --git a/core/stream_interface.c b/core/stream_interface.c
index e7298d9..89ae52e 100644
--- a/core/stream_interface.c
+++ b/core/stream_interface.c
@@ -476,6 +476,7 @@  void *network_initializer(void *data)
 {
 	int ret;
 	struct swupdate_cfg *software = data;
+	struct swupdate_request *req;
 
 	/* No installation in progress */
 	memset(&inst, 0, sizeof(inst));
@@ -497,14 +498,30 @@  void *network_initializer(void *data)
 		pthread_mutex_unlock(&stream_mutex);
 		notify(START, RECOVERY_NO_ERROR, INFOLEVEL, "Software Update started !");
 
+		req = &inst.req;
+
 		/*
 		 * Check if the dry run flag is overwritten
 		 */
-		if (inst.req.dry_run)
+		if (req->dry_run)
 			software->globals.dry_run = 1;
 		else
 			software->globals.dry_run = 0;
 
+		/*
+		 * Find the selection to be installed
+		 */
+		if ((strnlen(req->software_set, sizeof(req->software_set)) > 0) &&
+				(strnlen(req->running_mode, sizeof(req->running_mode)) > 0)) {
+			strlcpy(software->software_set, req->software_set, sizeof(software->software_set) - 1);
+			strlcpy(software->running_mode, req->running_mode, sizeof(software->running_mode) - 1);
+		} else {
+			strlcpy(software->software_set, software->globals.default_software_set,
+				sizeof(software->software_set) - 1);
+			strlcpy(software->running_mode, software->globals.default_running_mode,
+				sizeof(software->running_mode) - 1);
+		}
+
 		/*
 		 * Check if the stream should be saved
 		 */
diff --git a/core/swupdate.c b/core/swupdate.c
index 8bbc89e..49b0461 100644
--- a/core/swupdate.c
+++ b/core/swupdate.c
@@ -414,8 +414,14 @@  static int parse_image_selector(const char *selector, struct swupdate_cfg *sw)
 
 	*pos = '\0';
 
+	/*
+	 * the runtime copy in swcfg can be overloaded by IPC,
+	 * so maintain a copy to restore it after an update
+	 */
+	strlcpy(sw->globals.default_software_set, selector, sizeof(sw->globals.default_software_set));
 	strlcpy(sw->software_set, selector, sizeof(sw->software_set));
 	/* pos + 1 will either be NULL or valid text */
+	strlcpy(sw->globals.default_running_mode, pos + 1, sizeof(sw->globals.default_running_mode));
 	strlcpy(sw->running_mode, pos + 1, sizeof(sw->running_mode));
 
 	if (strlen(sw->software_set) == 0 || strlen(sw->running_mode) == 0)
@@ -986,7 +992,7 @@  int main(int argc, char **argv)
 			exit(EXIT_FAILURE);
 		}
 		fprintf(stderr, "software set: %s mode: %s\n",
-			swcfg.software_set, swcfg.running_mode);
+			swcfg.globals.default_software_set, swcfg.globals.default_running_mode);
 	}
 
 	/* Read sw-versions */
diff --git a/include/swupdate.h b/include/swupdate.h
index 29b756d..4ae0b43 100644
--- a/include/swupdate.h
+++ b/include/swupdate.h
@@ -135,6 +135,8 @@  struct swupdate_global_cfg {
 	char aeskeyfname[SWUPDATE_GENERAL_STRING_SIZE];
 	char postupdatecmd[SWUPDATE_GENERAL_STRING_SIZE];
 	char preupdatecmd[SWUPDATE_GENERAL_STRING_SIZE];
+	char default_software_set[SWUPDATE_GENERAL_STRING_SIZE];
+	char default_running_mode[SWUPDATE_GENERAL_STRING_SIZE];
 	char minimum_version[SWUPDATE_GENERAL_STRING_SIZE];
 	char current_version[SWUPDATE_GENERAL_STRING_SIZE];
 	int cert_purpose;