Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src/context: get current bootslot on custom bootloader #1215

Merged
merged 4 commits into from
Aug 30, 2023

Conversation

angeloc
Copy link
Contributor

@angeloc angeloc commented Aug 11, 2023

The Nvidia Tegra UEFI offers its own method to enable A/B redundancy, that it's handled internally by the L4T UEFI.

The nvbootctrl command can be used to do maintenance on A/B mechanism, inspect the status and change the next reboot booting slot.

It's not possible also, to modify the kernel cmdline from inside Linux because it is hardcoded in the bootloader at compile time.
To handle the A/B booting process, the system doesn't add the root= option to the cmdline, there is indeed a script which inspects the UEFI variables statuses and mount the correct rootfs at the right moment.

Therefore, using the cmdline to understand on which slot the system is running is not possible on this family platforms.

Using the custom backend is the only alternative.

I tested this on a jetson agx xavier board.

src/context.c Outdated Show resolved Hide resolved
@angeloc angeloc force-pushed the master branch 3 times, most recently from c0a2ed6 to f2a45ec Compare August 11, 2023 14:58
@codecov
Copy link

codecov bot commented Aug 12, 2023

Codecov Report

Patch coverage: 81.81% and project coverage change: +0.01% 🎉

Comparison is base (59d9e04) 79.59% compared to head (d238ad6) 79.60%.
Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1215      +/-   ##
==========================================
+ Coverage   79.59%   79.60%   +0.01%     
==========================================
  Files          64       64              
  Lines       19217    19269      +52     
==========================================
+ Hits        15295    15340      +45     
- Misses       3922     3929       +7     
Files Changed Coverage Δ
src/context.c 75.69% <75.75%> (+<0.01%) ⬆️
src/config_file.c 85.76% <100.00%> (+0.02%) ⬆️
test/context.c 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@angeloc
Copy link
Contributor Author

angeloc commented Aug 28, 2023

@ejoerns I think the PR should pass all the tests now

@ejoerns
Copy link
Member

ejoerns commented Aug 28, 2023

@ejoerns I think the PR should pass all the tests now

@angeloc Thanks! 👍 I've triggered the workflow. I will have a look as soon as I am a little less busy here.

@angeloc
Copy link
Contributor Author

angeloc commented Aug 28, 2023

@ejoerns Sorry, I fixed the remaining issue, now it should really pass all the tests!

@angeloc
Copy link
Contributor Author

angeloc commented Aug 28, 2023

@ejoerns For the codecov, I'm not sure on how to proceed. Can you share some hints?

Copy link
Member

@ejoerns ejoerns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have made a number of comments, mainly referring to testing and error handling.

docs/integration.rst Outdated Show resolved Hide resolved
docs/integration.rst Outdated Show resolved Hide resolved
src/context.c Outdated Show resolved Hide resolved
test/context.c Outdated Show resolved Hide resolved
test/test-custom.conf Show resolved Hide resolved
src/context.c Show resolved Hide resolved
src/context.c Show resolved Hide resolved
src/context.c Outdated Show resolved Hide resolved
src/context.c Outdated Show resolved Hide resolved
src/context.c Outdated Show resolved Hide resolved
@ejoerns
Copy link
Member

ejoerns commented Aug 29, 2023

@ejoerns For the codecov, I'm not sure on how to proceed. Can you share some hints?

The loss in coverage should be resolved by actually testing the added code. See my comments.

@angeloc angeloc force-pushed the master branch 2 times, most recently from f7114a8 to cdbdaba Compare August 29, 2023 14:46
@ejoerns ejoerns added the enhancement Adds new functionality or enhanced handling to RAUC label Aug 29, 2023
@angeloc
Copy link
Contributor Author

angeloc commented Aug 29, 2023

@ejoerns fixed the distcheck now, sorry!

@angeloc
Copy link
Contributor Author

angeloc commented Aug 29, 2023

@ejoerns ./uncrustify.sh is not failing on my machine and not sure why it is failing here

@angeloc
Copy link
Contributor Author

angeloc commented Aug 29, 2023

@ejoerns ./uncrustify.sh is not failing on my machine and not sure why it is failing here

ok, found and fixed

@angeloc angeloc force-pushed the master branch 2 times, most recently from 89a49d9 to 0b996c6 Compare August 29, 2023 17:20
@angeloc
Copy link
Contributor Author

angeloc commented Aug 29, 2023

I sincerely hope all the issues are solved now!

@angeloc angeloc requested a review from ejoerns August 30, 2023 07:34
src/context.c Show resolved Hide resolved
Copy link
Member

@ejoerns ejoerns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already looks quite good now, especially testing changes.

Just some minor comments you might want to address.

docs/integration.rst Outdated Show resolved Hide resolved
ejoerns
ejoerns previously approved these changes Aug 30, 2023
Copy link
Member

@ejoerns ejoerns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine from my site now, thanks for your effort!

On some setup, current bootslot could not be present on the kernel
command line (ex: nvidia tegra L4T UEFI), therefore it is needed to use
the custom bootloader backend to retrive the current bootslot.

Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
Adding the "get-current" argument handling to test the current
booted slot on a custom backend.

Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
This test requires a configuration file with custom bootloader enabled.

Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
custom_bootloader_backend must be freed as other resources when cleaning
RaucConfig struct.

Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
@jluebbe jluebbe self-requested a review August 30, 2023 10:20
@jluebbe jluebbe merged commit 3ad4979 into rauc:master Aug 30, 2023
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Adds new functionality or enhanced handling to RAUC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants