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@xxxxxxxxxxxxxx>
> Sent: Freitag, 14. April 2023 11:49
> To: Burri Markus LabTec <Markus.Burri@xxxxxx>; barebox@xxxxxxxxxxxxxxxxxxx
> 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@xxxxxx>
>>
>> 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 |
