[PATCH v1] Add bootchooser command option for active boot target

[PATCH v1] Add bootchooser command option for active boot target

[Markus Burri]

Add bootchooser command option to extract and save the active target.
The information about active target are stored into two variables
(bootchooser.active_target and bootchooser.active_boot)
This information may be used into scripts to do actions related to the active
target before starting it.

Signed-off-by: Markus Burri <markus.burri@mt.com>

Gbp-Pq: Name 0058-Add-command-for-next-boot-target.patch
---
 Documentation/user/bootchooser.rst |  6 ++++++
 commands/bootchooser.c             | 12 +++++++++++-
 common/bootchooser.c               | 21 +++++++++++++++++++++
 include/bootchooser.h              |  1 +
 4 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/Documentation/user/bootchooser.rst b/Documentation/user/bootchooser.rst
index 8456e11..68ec1b6 100644
--- a/Documentation/user/bootchooser.rst
+++ b/Documentation/user/bootchooser.rst
@@ -168,6 +168,12 @@ options not specific to any boot target.
   set of ``global.bootchooser.<targetname>.<variablename>`` variables must exist.
 ``global.bootchooser.last_chosen``
   *bootchooser* sets this to the boot target that was chosen on last boot (index).
+``global.bootchooser.active_target``
+  Set to the active boot target that will boot next.
+  Call *bootchooser -n* to update the value.
+``global.bootchooser.active_boot``
+  Set to the boot device of the active target that will boot next. 
+  Call *bootchooser -n* to update the value.
 
 .. _bootchooser,setup_example:
 
diff --git a/commands/bootchooser.c b/commands/bootchooser.c
index ac763a6..20bd2d6 100644
--- a/commands/bootchooser.c
+++ b/commands/bootchooser.c
@@ -59,8 +59,9 @@ static int do_bootchooser(int argc, char *argv[])
 	int info = 0;
 	bool done_something = false;
 	bool last_boot_successful = false;
+	bool active_boot_target = false;
 
-	while ((opt = getopt(argc, argv, "a:p:is")) > 0) {
+	while ((opt = getopt(argc, argv, "a:p:ins")) > 0) {
 		switch (opt) {
 		case 'a':
 			if (!strcmp(optarg, "default"))
@@ -77,6 +78,9 @@ static int do_bootchooser(int argc, char *argv[])
 		case 'i':
 			info = 1;
 			break;
+		case 'n':
+			active_boot_target = true;
+			break;
 		case 's':
 			last_boot_successful = true;
 			break;
@@ -120,6 +124,11 @@ static int do_bootchooser(int argc, char *argv[])
 		done_something = true;
 	}
 
+	if (active_boot_target) {
+		bootchooser_active_target(bootchooser);
+		done_something = true;
+	}
+
 	if (!done_something) {
 		printf("Nothing to do\n");
 		ret = COMMAND_ERROR_USAGE;
@@ -137,6 +146,7 @@ BAREBOX_CMD_HELP_TEXT("Options:")
 BAREBOX_CMD_HELP_OPT ("-a <n|default> [TARGETS]",  "set remaining attempts of given targets to 'n' or the default attempts")
 BAREBOX_CMD_HELP_OPT ("-p <n|default> [TARGETS]",  "set priority of given targets to 'n' or the default priority")
 BAREBOX_CMD_HELP_OPT ("-i",  "Show information about the bootchooser")
+BAREBOX_CMD_HELP_OPT ("-n",  "Save active boot target to bootchooser.active_target and active boot device to bootchooser.active_boot")
 BAREBOX_CMD_HELP_OPT ("-s",  "Mark the last boot successful")
 BAREBOX_CMD_HELP_END
 
diff --git a/common/bootchooser.c b/common/bootchooser.c
index c08db03..1ec29e6 100644
--- a/common/bootchooser.c
+++ b/common/bootchooser.c
@@ -619,6 +619,27 @@ void bootchooser_info(struct bootchooser *bc)
 	       bc->last_chosen->name : "unknown");
 }
 
+/**
+ * bootchooser_active_target - Save information about active boot target
+ * @bc: The bootchooser
+ */
+void bootchooser_active_target(struct bootchooser *bc)
+{
+	struct bootchooser_target *target;
+
+	list_for_each_entry(target, &bc->targets, list) {
+		if (bootchooser_target_ok(target, NULL)) {
+			if (bc->verbose) {
+				pr_info("bootchooser.active_target=%s\n", target->name);
+				pr_info("bootchooser.active_boot=%s\n", target->boot);
+			}
+			nvvar_add("bootchooser.active_target", target->name);
+			nvvar_add("bootchooser.active_boot", target->boot);
+			return;
+		}
+	}
+}
+
 /**
  * bootchooser_get_target - get the target that shall be booted next
  * @bc:		The bootchooser
diff --git a/include/bootchooser.h b/include/bootchooser.h
index 7822c01..9825593 100644
--- a/include/bootchooser.h
+++ b/include/bootchooser.h
@@ -12,6 +12,7 @@ int bootchooser_save(struct bootchooser *bootchooser);
 int bootchooser_put(struct bootchooser *bootchooser);
 
 void bootchooser_info(struct bootchooser *bootchooser);
+void bootchooser_active_target(struct bootchooser *bc);
 
 int bootchooser_boot(struct bootchooser *bc);
 
-- 
2.39.2

Re: [PATCH v1] Add bootchooser command option for active boot target

[Ahmad Fatoum]

Hello Markus,

On 14.04.23 11:40, Markus Burri wrote:
> Add bootchooser command option to extract and save the active target.
> The information about active target are stored into two variables
> (bootchooser.active_target and bootchooser.active_boot)
> This information may be used into scripts to do actions related to the active
> target before starting it.

Can you share an example script that depends on your new bootchooser -n feature
to aid review?

> 
> Signed-off-by: Markus Burri <markus.burri@mt.com>
> 
> Gbp-Pq: Name 0058-Add-command-for-next-boot-target.patch
> ---
>  Documentation/user/bootchooser.rst |  6 ++++++
>  commands/bootchooser.c             | 12 +++++++++++-
>  common/bootchooser.c               | 21 +++++++++++++++++++++
>  include/bootchooser.h              |  1 +
>  4 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/user/bootchooser.rst b/Documentation/user/bootchooser.rst
> index 8456e11..68ec1b6 100644
> --- a/Documentation/user/bootchooser.rst
> +++ b/Documentation/user/bootchooser.rst
> @@ -168,6 +168,12 @@ options not specific to any boot target.
>    set of ``global.bootchooser.<targetname>.<variablename>`` variables must exist.
>  ``global.bootchooser.last_chosen``
>    *bootchooser* sets this to the boot target that was chosen on last boot (index).
> +``global.bootchooser.active_target``
> +  Set to the active boot target that will boot next.
> +  Call *bootchooser -n* to update the value.
> +``global.bootchooser.active_boot``
> +  Set to the boot device of the active target that will boot next. 
> +  Call *bootchooser -n* to update the value.
>  
>  .. _bootchooser,setup_example:
>  
> diff --git a/commands/bootchooser.c b/commands/bootchooser.c
> index ac763a6..20bd2d6 100644
> --- a/commands/bootchooser.c
> +++ b/commands/bootchooser.c
> @@ -59,8 +59,9 @@ static int do_bootchooser(int argc, char *argv[])
>  	int info = 0;
>  	bool done_something = false;
>  	bool last_boot_successful = false;
> +	bool active_boot_target = false;
>  
> -	while ((opt = getopt(argc, argv, "a:p:is")) > 0) {
> +	while ((opt = getopt(argc, argv, "a:p:ins")) > 0) {
>  		switch (opt) {
>  		case 'a':
>  			if (!strcmp(optarg, "default"))
> @@ -77,6 +78,9 @@ static int do_bootchooser(int argc, char *argv[])
>  		case 'i':
>  			info = 1;
>  			break;
> +		case 'n':
> +			active_boot_target = true;
> +			break;
>  		case 's':
>  			last_boot_successful = true;
>  			break;
> @@ -120,6 +124,11 @@ static int do_bootchooser(int argc, char *argv[])
>  		done_something = true;
>  	}
>  
> +	if (active_boot_target) {
> +		bootchooser_active_target(bootchooser);
> +		done_something = true;
> +	}
> +
>  	if (!done_something) {
>  		printf("Nothing to do\n");
>  		ret = COMMAND_ERROR_USAGE;
> @@ -137,6 +146,7 @@ BAREBOX_CMD_HELP_TEXT("Options:")
>  BAREBOX_CMD_HELP_OPT ("-a <n|default> [TARGETS]",  "set remaining attempts of given targets to 'n' or the default attempts")
>  BAREBOX_CMD_HELP_OPT ("-p <n|default> [TARGETS]",  "set priority of given targets to 'n' or the default priority")
>  BAREBOX_CMD_HELP_OPT ("-i",  "Show information about the bootchooser")
> +BAREBOX_CMD_HELP_OPT ("-n",  "Save active boot target to bootchooser.active_target and active boot device to bootchooser.active_boot")
>  BAREBOX_CMD_HELP_OPT ("-s",  "Mark the last boot successful")
>  BAREBOX_CMD_HELP_END
>  
> diff --git a/common/bootchooser.c b/common/bootchooser.c
> index c08db03..1ec29e6 100644
> --- a/common/bootchooser.c
> +++ b/common/bootchooser.c
> @@ -619,6 +619,27 @@ void bootchooser_info(struct bootchooser *bc)
>  	       bc->last_chosen->name : "unknown");
>  }
>  
> +/**
> + * bootchooser_active_target - Save information about active boot target
> + * @bc: The bootchooser
> + */
> +void bootchooser_active_target(struct bootchooser *bc)
> +{
> +	struct bootchooser_target *target;
> +
> +	list_for_each_entry(target, &bc->targets, list) {
> +		if (bootchooser_target_ok(target, NULL)) {
> +			if (bc->verbose) {
> +				pr_info("bootchooser.active_target=%s\n", target->name);
> +				pr_info("bootchooser.active_boot=%s\n", target->boot);
> +			}
> +			nvvar_add("bootchooser.active_target", target->name);
> +			nvvar_add("bootchooser.active_boot", target->boot);
> +			return;
> +		}
> +	}
> +}
> +
>  /**
>   * bootchooser_get_target - get the target that shall be booted next
>   * @bc:		The bootchooser
> diff --git a/include/bootchooser.h b/include/bootchooser.h
> index 7822c01..9825593 100644
> --- a/include/bootchooser.h
> +++ b/include/bootchooser.h
> @@ -12,6 +12,7 @@ int bootchooser_save(struct bootchooser *bootchooser);
>  int bootchooser_put(struct bootchooser *bootchooser);
>  
>  void bootchooser_info(struct bootchooser *bootchooser);
> +void bootchooser_active_target(struct bootchooser *bc);
>  
>  int bootchooser_boot(struct bootchooser *bc);
>  

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: EXTERNAL - [PATCH v1] Add bootchooser command option for active boot target

[Ahmad Fatoum]

Hello Markus,

(Adding the list back to Cc)

On 14.04.23 12:49, Burri Markus LabTec wrote:
> Hello Ahmad
> 
> Here is example script:
> 
> #!/bin/sh
> bootchooser -n
> #Execute all hook scripts found on active target
> HOOKDIR="/mnt/${nv.bootchooser.active_boot}/boot/barebox-hooks"
> cd ${HOOKDIR}
> for HOOK in *; do
>     echo "run barebox-hook: \'${HOOK}\'"
>     . ${HOOKDIR}/${HOOK}
> done
> cd /

I assume this is an init script? This script will execute
the hooks of the old rootfs once before booting the new rootfs.

This doesn't sound like a good idea.

Fundamentally, the barebox environment is not power-fail safe,
that's why bootchooser is using barebox-state. So writing nv
variables without user interaction is not ok.

Did you know that you can have scripts as bootchooser targets?
Instead of e.g. bootchooser.system0.boot=mmc0.1, you can add
/env/boot/system0 and then do whatever you want there before 
calling boot mmc0. Would that suffice?

Cheers,
Ahmad

 
> -----Original Message-----
> From: Ahmad Fatoum <a.fatoum@pengutronix.de> 
> Sent: Freitag, 14. April 2023 11:49
> To: Burri Markus LabTec <Markus.Burri@mt.com>; barebox@lists.infradead.org
> Subject: Re: EXTERNAL - [PATCH v1] Add bootchooser command option for active boot target
> 
> Hello Markus,
> 
> On 14.04.23 11:40, Markus Burri wrote:
>> Add bootchooser command option to extract and save the active target.
>> The information about active target are stored into two variables 
>> (bootchooser.active_target and bootchooser.active_boot) This 
>> information may be used into scripts to do actions related to the 
>> active target before starting it.
> 
> Can you share an example script that depends on your new bootchooser -n feature to aid review?
> 
>>
>> Signed-off-by: Markus Burri <markus.burri@mt.com>
>>
>> Gbp-Pq: Name 0058-Add-command-for-next-boot-target.patch
>> ---
>>  Documentation/user/bootchooser.rst |  6 ++++++
>>  commands/bootchooser.c             | 12 +++++++++++-
>>  common/bootchooser.c               | 21 +++++++++++++++++++++
>>  include/bootchooser.h              |  1 +
>>  4 files changed, 39 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/user/bootchooser.rst 
>> b/Documentation/user/bootchooser.rst
>> index 8456e11..68ec1b6 100644
>> --- a/Documentation/user/bootchooser.rst
>> +++ b/Documentation/user/bootchooser.rst
>> @@ -168,6 +168,12 @@ options not specific to any boot target.
>>    set of ``global.bootchooser.<targetname>.<variablename>`` variables must exist.
>>  ``global.bootchooser.last_chosen``
>>    *bootchooser* sets this to the boot target that was chosen on last boot (index).
>> +``global.bootchooser.active_target``
>> +  Set to the active boot target that will boot next.
>> +  Call *bootchooser -n* to update the value.
>> +``global.bootchooser.active_boot``
>> +  Set to the boot device of the active target that will boot next. 
>> +  Call *bootchooser -n* to update the value.
>>  
>>  .. _bootchooser,setup_example:
>>  
>> diff --git a/commands/bootchooser.c b/commands/bootchooser.c index 
>> ac763a6..20bd2d6 100644
>> --- a/commands/bootchooser.c
>> +++ b/commands/bootchooser.c
>> @@ -59,8 +59,9 @@ static int do_bootchooser(int argc, char *argv[])
>>  	int info = 0;
>>  	bool done_something = false;
>>  	bool last_boot_successful = false;
>> +	bool active_boot_target = false;
>>  
>> -	while ((opt = getopt(argc, argv, "a:p:is")) > 0) {
>> +	while ((opt = getopt(argc, argv, "a:p:ins")) > 0) {
>>  		switch (opt) {
>>  		case 'a':
>>  			if (!strcmp(optarg, "default"))
>> @@ -77,6 +78,9 @@ static int do_bootchooser(int argc, char *argv[])
>>  		case 'i':
>>  			info = 1;
>>  			break;
>> +		case 'n':
>> +			active_boot_target = true;
>> +			break;
>>  		case 's':
>>  			last_boot_successful = true;
>>  			break;
>> @@ -120,6 +124,11 @@ static int do_bootchooser(int argc, char *argv[])
>>  		done_something = true;
>>  	}
>>  
>> +	if (active_boot_target) {
>> +		bootchooser_active_target(bootchooser);
>> +		done_something = true;
>> +	}
>> +
>>  	if (!done_something) {
>>  		printf("Nothing to do\n");
>>  		ret = COMMAND_ERROR_USAGE;
>> @@ -137,6 +146,7 @@ BAREBOX_CMD_HELP_TEXT("Options:")  
>> BAREBOX_CMD_HELP_OPT ("-a <n|default> [TARGETS]",  "set remaining 
>> attempts of given targets to 'n' or the default attempts")  
>> BAREBOX_CMD_HELP_OPT ("-p <n|default> [TARGETS]",  "set priority of 
>> given targets to 'n' or the default priority")  BAREBOX_CMD_HELP_OPT 
>> ("-i",  "Show information about the bootchooser")
>> +BAREBOX_CMD_HELP_OPT ("-n",  "Save active boot target to 
>> +bootchooser.active_target and active boot device to 
>> +bootchooser.active_boot")
>>  BAREBOX_CMD_HELP_OPT ("-s",  "Mark the last boot successful")  
>> BAREBOX_CMD_HELP_END
>>  
>> diff --git a/common/bootchooser.c b/common/bootchooser.c index 
>> c08db03..1ec29e6 100644
>> --- a/common/bootchooser.c
>> +++ b/common/bootchooser.c
>> @@ -619,6 +619,27 @@ void bootchooser_info(struct bootchooser *bc)
>>  	       bc->last_chosen->name : "unknown");  }
>>  
>> +/**
>> + * bootchooser_active_target - Save information about active boot 
>> +target
>> + * @bc: The bootchooser
>> + */
>> +void bootchooser_active_target(struct bootchooser *bc) {
>> +	struct bootchooser_target *target;
>> +
>> +	list_for_each_entry(target, &bc->targets, list) {
>> +		if (bootchooser_target_ok(target, NULL)) {
>> +			if (bc->verbose) {
>> +				pr_info("bootchooser.active_target=%s\n", target->name);
>> +				pr_info("bootchooser.active_boot=%s\n", target->boot);
>> +			}
>> +			nvvar_add("bootchooser.active_target", target->name);
>> +			nvvar_add("bootchooser.active_boot", target->boot);
>> +			return;
>> +		}
>> +	}
>> +}
>> +
>>  /**
>>   * bootchooser_get_target - get the target that shall be booted next
>>   * @bc:		The bootchooser
>> diff --git a/include/bootchooser.h b/include/bootchooser.h index 
>> 7822c01..9825593 100644
>> --- a/include/bootchooser.h
>> +++ b/include/bootchooser.h
>> @@ -12,6 +12,7 @@ int bootchooser_save(struct bootchooser 
>> *bootchooser);  int bootchooser_put(struct bootchooser *bootchooser);
>>  
>>  void bootchooser_info(struct bootchooser *bootchooser);
>> +void bootchooser_active_target(struct bootchooser *bc);
>>  
>>  int bootchooser_boot(struct bootchooser *bc);
>>  
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

RE: EXTERNAL - [PATCH v1] Add bootchooser command option for active boot target

[Burri Markus LabTec]

Hello Ahmad

The Idea is to run some scripts stored in the rootfs before booting the system. In our case e.g we enable led according to customer needs. This is stored in to rootfs where the customer is able to update without update barebox
The new command option will store this information in a variable where I can use this information in the init script.
It sound that a script as bootchooser target will fit my requirement. I didn't know that. I will try
Thx

Best regards Markus
 

-----Original Message-----
From: Ahmad Fatoum <a.fatoum@pengutronix.de> 
Sent: Freitag, 14. April 2023 14:27
To: Burri Markus LabTec <Markus.Burri@mt.com>
Cc: barebox@lists.infradead.org
Subject: Re: EXTERNAL - [PATCH v1] Add bootchooser command option for active boot target

Hello Markus,

(Adding the list back to Cc)

On 14.04.23 12:49, Burri Markus LabTec wrote:
> Hello Ahmad
> 
> Here is example script:
> 
> #!/bin/sh
> bootchooser -n
> #Execute all hook scripts found on active target 
> HOOKDIR="/mnt/${nv.bootchooser.active_boot}/boot/barebox-hooks"
> cd ${HOOKDIR}
> for HOOK in *; do
>     echo "run barebox-hook: \'${HOOK}\'"
>     . ${HOOKDIR}/${HOOK}
> done
> cd /

I assume this is an init script? This script will execute the hooks of the old rootfs once before booting the new rootfs.

This doesn't sound like a good idea.

Fundamentally, the barebox environment is not power-fail safe, that's why bootchooser is using barebox-state. So writing nv variables without user interaction is not ok.

Did you know that you can have scripts as bootchooser targets?
Instead of e.g. bootchooser.system0.boot=mmc0.1, you can add
/env/boot/system0 and then do whatever you want there before calling boot mmc0. Would that suffice?

Cheers,
Ahmad

 
> -----Original Message-----
> From: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Sent: Freitag, 14. April 2023 11:49
> To: Burri Markus LabTec <Markus.Burri@mt.com>; 
> barebox@lists.infradead.org
> Subject: Re: EXTERNAL - [PATCH v1] Add bootchooser command option for 
> active boot target
> 
> Hello Markus,
> 
> On 14.04.23 11:40, Markus Burri wrote:
>> Add bootchooser command option to extract and save the active target.
>> The information about active target are stored into two variables 
>> (bootchooser.active_target and bootchooser.active_boot) This 
>> information may be used into scripts to do actions related to the 
>> active target before starting it.
> 
> Can you share an example script that depends on your new bootchooser -n feature to aid review?
> 
>>
>> Signed-off-by: Markus Burri <markus.burri@mt.com>
>>
>> Gbp-Pq: Name 0058-Add-command-for-next-boot-target.patch
>> ---
>>  Documentation/user/bootchooser.rst |  6 ++++++
>>  commands/bootchooser.c             | 12 +++++++++++-
>>  common/bootchooser.c               | 21 +++++++++++++++++++++
>>  include/bootchooser.h              |  1 +
>>  4 files changed, 39 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/user/bootchooser.rst
>> b/Documentation/user/bootchooser.rst
>> index 8456e11..68ec1b6 100644
>> --- a/Documentation/user/bootchooser.rst
>> +++ b/Documentation/user/bootchooser.rst
>> @@ -168,6 +168,12 @@ options not specific to any boot target.
>>    set of ``global.bootchooser.<targetname>.<variablename>`` variables must exist.
>>  ``global.bootchooser.last_chosen``
>>    *bootchooser* sets this to the boot target that was chosen on last boot (index).
>> +``global.bootchooser.active_target``
>> +  Set to the active boot target that will boot next.
>> +  Call *bootchooser -n* to update the value.
>> +``global.bootchooser.active_boot``
>> +  Set to the boot device of the active target that will boot next. 
>> +  Call *bootchooser -n* to update the value.
>>  
>>  .. _bootchooser,setup_example:
>>  
>> diff --git a/commands/bootchooser.c b/commands/bootchooser.c index
>> ac763a6..20bd2d6 100644
>> --- a/commands/bootchooser.c
>> +++ b/commands/bootchooser.c
>> @@ -59,8 +59,9 @@ static int do_bootchooser(int argc, char *argv[])
>>  	int info = 0;
>>  	bool done_something = false;
>>  	bool last_boot_successful = false;
>> +	bool active_boot_target = false;
>>  
>> -	while ((opt = getopt(argc, argv, "a:p:is")) > 0) {
>> +	while ((opt = getopt(argc, argv, "a:p:ins")) > 0) {
>>  		switch (opt) {
>>  		case 'a':
>>  			if (!strcmp(optarg, "default"))
>> @@ -77,6 +78,9 @@ static int do_bootchooser(int argc, char *argv[])
>>  		case 'i':
>>  			info = 1;
>>  			break;
>> +		case 'n':
>> +			active_boot_target = true;
>> +			break;
>>  		case 's':
>>  			last_boot_successful = true;
>>  			break;
>> @@ -120,6 +124,11 @@ static int do_bootchooser(int argc, char *argv[])
>>  		done_something = true;
>>  	}
>>  
>> +	if (active_boot_target) {
>> +		bootchooser_active_target(bootchooser);
>> +		done_something = true;
>> +	}
>> +
>>  	if (!done_something) {
>>  		printf("Nothing to do\n");
>>  		ret = COMMAND_ERROR_USAGE;
>> @@ -137,6 +146,7 @@ BAREBOX_CMD_HELP_TEXT("Options:") 
>> BAREBOX_CMD_HELP_OPT ("-a <n|default> [TARGETS]",  "set remaining 
>> attempts of given targets to 'n' or the default attempts") 
>> BAREBOX_CMD_HELP_OPT ("-p <n|default> [TARGETS]",  "set priority of 
>> given targets to 'n' or the default priority")  BAREBOX_CMD_HELP_OPT 
>> ("-i",  "Show information about the bootchooser")
>> +BAREBOX_CMD_HELP_OPT ("-n",  "Save active boot target to 
>> +bootchooser.active_target and active boot device to
>> +bootchooser.active_boot")
>>  BAREBOX_CMD_HELP_OPT ("-s",  "Mark the last boot successful") 
>> BAREBOX_CMD_HELP_END
>>  
>> diff --git a/common/bootchooser.c b/common/bootchooser.c index
>> c08db03..1ec29e6 100644
>> --- a/common/bootchooser.c
>> +++ b/common/bootchooser.c
>> @@ -619,6 +619,27 @@ void bootchooser_info(struct bootchooser *bc)
>>  	       bc->last_chosen->name : "unknown");  }
>>  
>> +/**
>> + * bootchooser_active_target - Save information about active boot 
>> +target
>> + * @bc: The bootchooser
>> + */
>> +void bootchooser_active_target(struct bootchooser *bc) {
>> +	struct bootchooser_target *target;
>> +
>> +	list_for_each_entry(target, &bc->targets, list) {
>> +		if (bootchooser_target_ok(target, NULL)) {
>> +			if (bc->verbose) {
>> +				pr_info("bootchooser.active_target=%s\n", target->name);
>> +				pr_info("bootchooser.active_boot=%s\n", target->boot);
>> +			}
>> +			nvvar_add("bootchooser.active_target", target->name);
>> +			nvvar_add("bootchooser.active_boot", target->boot);
>> +			return;
>> +		}
>> +	}
>> +}
>> +
>>  /**
>>   * bootchooser_get_target - get the target that shall be booted next
>>   * @bc:		The bootchooser
>> diff --git a/include/bootchooser.h b/include/bootchooser.h index
>> 7822c01..9825593 100644
>> --- a/include/bootchooser.h
>> +++ b/include/bootchooser.h
>> @@ -12,6 +12,7 @@ int bootchooser_save(struct bootchooser 
>> *bootchooser);  int bootchooser_put(struct bootchooser *bootchooser);
>>  
>>  void bootchooser_info(struct bootchooser *bootchooser);
>> +void bootchooser_active_target(struct bootchooser *bc);
>>  
>>  int bootchooser_boot(struct bootchooser *bc);
>>  
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: EXTERNAL - [PATCH v1] Add bootchooser command option for active boot target

[Ahmad Fatoum]

Hello Markus,

On 14.04.23 14:41, Burri Markus LabTec wrote:
> Hello Ahmad
> 
> The Idea is to run some scripts stored in the rootfs before booting the system. In our case e.g we enable led according to customer needs. This is stored in to rootfs where the customer is able to update without update barebox

Once you give your customer the ability to run arbitrary code, they will run
arbitrary code, e.g. apply an overlay to the kernel DT or tune kernel
command line argument. With your init script approach, you could apply
old overlays when booting a new kernel, which can lead to very surprising
results.

> The new command option will store this information in a variable where I can use this information in the init script.
> It sound that a script as bootchooser target will fit my requirement. I didn't know that. I will try

I am not opposed to give these scripts knowledge of what bootchooser target
is executed, see untested patch below. But it should be the boot scripts
that run hooks, not an init script on a subsequent boot.

Cheers,
Ahmad

---
 common/bootchooser.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/common/bootchooser.c b/common/bootchooser.c
index 08d05e885250..3d400af08ef1 100644
--- a/common/bootchooser.c
+++ b/common/bootchooser.c
@@ -827,6 +827,7 @@ static int bootchooser_boot_one(struct bootchooser *bc, int *tryagain)
                goto out;
        }

+       globalvar_add_simple("bootchooser.active", target->name);
        system = basprintf("bootchooser.active=%s", target->name);
        globalvar_add_simple("linux.bootargs.bootchooser", system);
        free(system);
@@ -854,6 +855,7 @@ static int bootchooser_boot_one(struct bootchooser *bc, int *tryagain)
        *tryagain = 1;
 out:
        globalvar_set_match("linux.bootargs.bootchooser", NULL);
+       globalvar_set_match("bootchooser.active", NULL);

        bootentries_free(entries);

@@ -976,3 +978,5 @@ BAREBOX_MAGICVAR(global.bootchooser.default_priority,
                 "bootchooser: Default priority for a target");
 BAREBOX_MAGICVAR(global.bootchooser.state_prefix,
                 "bootchooser: state name prefix, empty for nv backend");
+BAREBOX_MAGICVAR(global.bootchooser.active,
+                "bootchooser: holds name of active bootchooser target during boot");

RE: EXTERNAL - [PATCH v1] Add bootchooser command option for active boot target

[Burri Markus LabTec]

Hi Ahmad
I removed my init script and replaced it by a script added to bootchooser.system0.boot=<script>.
This fit my requirements and therefor the patch is obsolete.
Thanks for your support.
Best regards Markus

-----Original Message-----
From: Ahmad Fatoum <a.fatoum@pengutronix.de> 
Sent: Freitag, 14. April 2023 14:47
To: Burri Markus LabTec <Markus.Burri@mt.com>
Cc: barebox@lists.infradead.org
Subject: Re: EXTERNAL - [PATCH v1] Add bootchooser command option for active boot target

Hello Markus,

On 14.04.23 14:41, Burri Markus LabTec wrote:
> Hello Ahmad
> 
> The Idea is to run some scripts stored in the rootfs before booting 
> the system. In our case e.g we enable led according to customer needs. 
> This is stored in to rootfs where the customer is able to update 
> without update barebox

Once you give your customer the ability to run arbitrary code, they will run arbitrary code, e.g. apply an overlay to the kernel DT or tune kernel command line argument. With your init script approach, you could apply old overlays when booting a new kernel, which can lead to very surprising results.

> The new command option will store this information in a variable where I can use this information in the init script.
> It sound that a script as bootchooser target will fit my requirement. 
> I didn't know that. I will try

I am not opposed to give these scripts knowledge of what bootchooser target is executed, see untested patch below. But it should be the boot scripts that run hooks, not an init script on a subsequent boot.

Cheers,
Ahmad

---
 common/bootchooser.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/common/bootchooser.c b/common/bootchooser.c index 08d05e885250..3d400af08ef1 100644
--- a/common/bootchooser.c
+++ b/common/bootchooser.c
@@ -827,6 +827,7 @@ static int bootchooser_boot_one(struct bootchooser *bc, int *tryagain)
                goto out;
        }

+       globalvar_add_simple("bootchooser.active", target->name);
        system = basprintf("bootchooser.active=%s", target->name);
        globalvar_add_simple("linux.bootargs.bootchooser", system);
        free(system);
@@ -854,6 +855,7 @@ static int bootchooser_boot_one(struct bootchooser *bc, int *tryagain)
        *tryagain = 1;
 out:
        globalvar_set_match("linux.bootargs.bootchooser", NULL);
+       globalvar_set_match("bootchooser.active", NULL);

        bootentries_free(entries);

@@ -976,3 +978,5 @@ BAREBOX_MAGICVAR(global.bootchooser.default_priority,
                 "bootchooser: Default priority for a target");  BAREBOX_MAGICVAR(global.bootchooser.state_prefix,
                 "bootchooser: state name prefix, empty for nv backend");
+BAREBOX_MAGICVAR(global.bootchooser.active,
+                "bootchooser: holds name of active bootchooser target 
+during boot");