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

[Burri Markus LabTec <Markus.Burri@mt.com>]

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 |