sysroot: Support boot counting for boot entries #3310

igoropaniuk · 2024-09-19T14:28:18Z

Add support for boot counting for bootloader entries [1].
The boot counting data is stored in the name of the boot loader entry.
A boot loader entry file name may contain a plus (+) followed by a number.
This may optionally be followed by a minus (-) followed by a second number.
The dot (.) and file name suffix (conf or efi) must immediately follow.

The feature is enabled via sysroot configuration:
[sysroot]
boot-counting-tries=3

Testing:
$ ostree admin deploy 91fc19319be9e79d07159303dff125f40f10e5c25614630dcbed23d95e36f907
Copying /etc changes: 2 modified, 3 removed, 4 added
bootfs is sufficient for calculated new size: 0 bytes
Transaction complete; bootconfig swap: yes; bootversion: boot.0.1, deployment count change: 1

$ ls /boot/loader/entries
ostree-1.conf ostree-2+3.conf

[1] https://uapi-group.org/specifications/specs/boot_loader_specification/#boot-counting

openshift-ci · 2024-09-19T14:28:28Z

Hi @igoropaniuk. Thanks for your PR.

I'm waiting for a ostreedev member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

igoropaniuk · 2024-09-19T14:30:26Z

This PR was created for initial feedback, I'll address the naming in the tests if needed and re-push

cgwalters · 2024-09-19T20:40:16Z

I love the idea of integrating here, thanks for starting this! This came up before in at least #3032

Hmm don't we need to consider how we make this configurable? Currently systemd kernel-install parses /etc/kernel/tries. (I think we should also support a /usr/lib variant of this, but that's mostly an aside). I guess we could in theory just start honoring that file too?

igoropaniuk · 2024-09-23T12:33:13Z

@cgwalters thanks for the feedback, my concern regarding parsing /etc/kernel/tries is that it's systemd-specific config, which is not described in https://uapi-group.org/specifications/specs/boot_loader_specification/

A can add an optional autoconf flag for that

igoropaniuk · 2024-10-16T14:14:24Z

@cgwalters added support for /etc/kernel/tries as suggested
I also wrapped the functionality with an autoconf flag (--enable-boot-count), which is disabled by default.

cgwalters

Thanks so much for working on this!

configure.ac

src/libostree/ostree-sysroot-deploy.c

cgwalters · 2024-10-18T13:35:27Z

BTW, which bootloader are you looking at using this with?

igoropaniuk · 2024-10-18T15:04:08Z

@cgwalters EDK2 + systemd-boot

igoropaniuk · 2024-10-30T15:53:44Z

@cgwalters I've addressed all comments, thanks!

igoropaniuk · 2024-10-30T17:18:00Z

@cgwalters looks like some CI tests are still failing, but all of them are unrelated to this change, for example:

2024-10-30T16:06:09.9538403Z ERROR: tests/test-commit-sign.sh - too few tests run (expected 7, got 2)
2024-10-30T16:06:09.9539247Z ERROR: tests/test-commit-sign.sh - exited with status 1

cgwalters · 2024-10-31T13:56:14Z

Yes CI failures are unrelated, will look at this soon

man/ostree.repo-config.xml

src/libostree/ostree-repo.c

src/libostree/ostree-sysroot-deploy.c

igoropaniuk · 2024-11-20T13:28:03Z

The PR is not stalled, I'll get back to it tomorrow

igoropaniuk · 2024-12-04T22:18:33Z

@jlebon @cgwalters all comments addressed, please take a look

src/libostree/ostree-sysroot-deploy.c

igoropaniuk · 2025-06-18T10:28:14Z

@cgwalters Apologies for the delay. I've addressed all the comments except the last one concerning the "usage" variable.

Could we get this PR to land first (as it seems closer to being merged in terms of its maturity), and then I'll focus on addressing all issues in the other one (#3310) afterwards?

cgwalters · 2025-07-09T19:52:15Z

/gemini review

gemini-code-assist

Code Review

This pull request adds support for boot counting for bootloader entries, aligning with the Boot Loader Specification. The changes are comprehensive, touching on API documentation, configuration parsing, and bootloader entry filename generation. The implementation is mostly solid, but I've found a few issues, including a critical bug in the boot counter parsing logic and a discrepancy in the configuration handling for boot-counting-tries. I've also pointed out some minor documentation and code style issues. Once these are addressed, the PR should be in good shape.

src/libostree/ostree-bootconfig-parser.c

src/libostree/ostree-repo.c

src/libostree/ostree-bootconfig-parser.c

cgwalters · 2025-07-09T20:37:34Z

OK I

Addressed the gemini review comments
Added unit tests for the tries parsing
Added a unit-admin test that verifies we write the bootloader entry files w/boot counting initalized

cgwalters · 2025-07-09T20:37:40Z

/gemini review

gemini-code-assist

Code Review

This pull request adds support for boot counting for bootloader entries, following the Boot Loader Specification. The implementation includes parsing the boot count from filenames, configuring the number of tries, and generating appropriate bootloader entry filenames. The changes are well-structured and include documentation and tests.

My main feedback is on the boot counter parsing logic in ostree-bootconfig-parser.c, which appears to be more lenient than the specification and contains a minor bug. I've left detailed comments on this. I've also pointed out a minor documentation issue in the same file.

src/libostree/ostree-bootconfig-parser.c

cgwalters

Forgot to also mention I went ahead and rebased this on git main.

I have just a few other minor comments but I think we can get this in by tomorrow - what do you think?

src/libostree/libostree-devel.sym

src/libostree/ostree-bootconfig-parser.h

src/libostree/ostree-bootconfig-parser.c

cgwalters · 2025-07-09T21:34:52Z

OK I pushed more changes to resolve review comments

igoropaniuk · 2025-07-09T21:40:17Z

@cgwalters, it seems we are simultaneously working to address comments :)
Would you like me to finish addressing all the issues, or would you like to handle that on your own?

cgwalters · 2025-07-09T21:46:56Z

@cgwalters, it seems we are simultaneously working to address comments :)

Ahhh sorry! Based on your location I thought you'd be sleeping now 😄

OK I did just one more force push but I will hand over the PR to you! What do you think of the changes?

igoropaniuk · 2025-07-09T21:53:05Z

@cgwalters

Ahhh sorry! Based on your location I thought you'd be sleeping now 😄
OK I did just one more force push but I will hand over the PR to you! What do you think of the changes?

Great, thanks for covering this functionality with unit tests and addressing the reported issues by Gemini!
Please let me know if you have any additional comments; I'll review them tomorrow morning and try to address them right away + address build issues reported by CI as well (so everything is ready when you return to this PR)

cgwalters · 2025-07-09T22:01:36Z

Offhand I don't have any other issues!

It looks like I broke CI by adding ostree_bootconfig_parser_get_tries_done to public API without also listing it in apidoc/ - that should be an easy fix.

Add support for boot counting for bootloader entries [1]. The boot counting data is stored in the name of the boot loader entry. A boot loader entry file name may contain a plus (+) followed by a number. This may optionally be followed by a minus (-) followed by a second number. The dot (.) and file name suffix (conf or efi) must immediately follow. The feature is enabled via sysroot configuration: [sysroot] boot-counting-tries=3 Testing: $ ostree admin deploy 91fc19319be9e79d07159303dff125f40f10e5c25614630dcbed23d95e36f907 Copying /etc changes: 2 modified, 3 removed, 4 added bootfs is sufficient for calculated new size: 0 bytes Transaction complete; bootconfig swap: yes; bootversion: boot.0.1, deployment count change: 1 $ ls /boot/loader/entries ostree-1.conf ostree-2+3.conf [1] https://uapi-group.org/specifications/specs/boot_loader_specification/#boot-counting Signed-off-by: Igor Opaniuk <igor.opaniuk@foundries.io> Signed-off-by: Colin Walters <walters@verbum.org>

igoropaniuk · 2025-07-10T07:24:49Z

It looks like I broke CI by adding ostree_bootconfig_parser_get_tries_done to public API without also listing it in apidoc/ - that should be an easy fix.

Fixed, thanks for the hint!

cgwalters · 2025-07-10T11:24:00Z

/override ci/prow/fcos-e2e
I'll probably drop this one as it's always duplicated coreos ci jenkins

openshift-ci · 2025-07-10T11:24:06Z

@cgwalters: Overrode contexts on behalf of cgwalters: ci/prow/fcos-e2e

In response to this:

/override ci/prow/fcos-e2e
I'll probably drop this one as it's always duplicated coreos ci jenkins

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

openshift-ci bot added the needs-ok-to-test label Sep 19, 2024

igoropaniuk force-pushed the boot_count branch from 6b33424 to 8ea23c7 Compare October 16, 2024 14:13

igoropaniuk changed the title ~~[RFC] sysroot: Support boot counting for boot entries~~ sysroot: Support boot counting for boot entries Oct 16, 2024

cgwalters reviewed Oct 18, 2024

View reviewed changes

configure.ac Outdated Show resolved Hide resolved

src/libostree/ostree-sysroot-deploy.c Outdated Show resolved Hide resolved

igoropaniuk force-pushed the boot_count branch from 8ea23c7 to 4bce924 Compare October 30, 2024 15:52

igoropaniuk force-pushed the boot_count branch from 4bce924 to 57e988e Compare October 30, 2024 15:58

igoropaniuk requested a review from cgwalters October 31, 2024 13:50

jlebon reviewed Oct 31, 2024

View reviewed changes

igoropaniuk force-pushed the boot_count branch 2 times, most recently from 030b79c to 8802316 Compare December 4, 2024 22:16

igoropaniuk requested a review from jlebon December 4, 2024 22:18

igoropaniuk force-pushed the boot_count branch 2 times, most recently from 905317a to 9ab2679 Compare December 4, 2024 22:21

igoropaniuk commented Dec 5, 2024

View reviewed changes

src/libostree/ostree-sysroot-deploy.c Outdated Show resolved Hide resolved

igoropaniuk mentioned this pull request Dec 5, 2024

systemd-boot support #1719

Open

igoropaniuk force-pushed the boot_count branch 3 times, most recently from 37ba251 to 087e056 Compare December 5, 2024 21:15

igoropaniuk force-pushed the boot_count branch 2 times, most recently from dfd586a to 6362318 Compare May 14, 2025 16:23

igoropaniuk force-pushed the boot_count branch from 6362318 to 14a6d8e Compare June 18, 2025 10:19

igoropaniuk requested a review from cgwalters June 18, 2025 10:28

gemini-code-assist bot reviewed Jul 9, 2025

View reviewed changes

cgwalters force-pushed the boot_count branch from 14a6d8e to 933ded1 Compare July 9, 2025 20:36

gemini-code-assist bot reviewed Jul 9, 2025

View reviewed changes

src/libostree/ostree-bootconfig-parser.c Outdated Show resolved Hide resolved

src/libostree/ostree-bootconfig-parser.c Outdated Show resolved Hide resolved

cgwalters requested changes Jul 9, 2025

View reviewed changes

src/libostree/libostree-devel.sym Outdated Show resolved Hide resolved

src/libostree/ostree-bootconfig-parser.h Show resolved Hide resolved

src/libostree/ostree-bootconfig-parser.c Outdated Show resolved Hide resolved

igoropaniuk force-pushed the boot_count branch 3 times, most recently from 8b16858 to 21866a9 Compare July 9, 2025 21:30

cgwalters force-pushed the boot_count branch from 21866a9 to 05264a8 Compare July 9, 2025 21:34

cgwalters force-pushed the boot_count branch from 05264a8 to ddb73b5 Compare July 9, 2025 21:46

cgwalters mentioned this pull request Jul 9, 2025

Release 2025.3 #3466

Closed

33 tasks

igoropaniuk force-pushed the boot_count branch from ddb73b5 to 0848709 Compare July 10, 2025 06:58

cgwalters approved these changes Jul 10, 2025

View reviewed changes

cgwalters merged commit f4bc422 into ostreedev:main Jul 10, 2025
25 checks passed

sysroot: Support boot counting for boot entries #3310

sysroot: Support boot counting for boot entries #3310

Uh oh!

Conversation

igoropaniuk commented Sep 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Sep 19, 2024

Uh oh!

igoropaniuk commented Sep 19, 2024

Uh oh!

cgwalters commented Sep 19, 2024

Uh oh!

igoropaniuk commented Sep 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

igoropaniuk commented Oct 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cgwalters left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cgwalters commented Oct 18, 2024

Uh oh!

igoropaniuk commented Oct 18, 2024

Uh oh!

igoropaniuk commented Oct 30, 2024

Uh oh!

igoropaniuk commented Oct 30, 2024

Uh oh!

cgwalters commented Oct 31, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

igoropaniuk commented Nov 20, 2024

Uh oh!

igoropaniuk commented Dec 4, 2024

Uh oh!

Uh oh!

igoropaniuk commented Jun 18, 2025

Uh oh!

cgwalters commented Jul 9, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cgwalters commented Jul 9, 2025

Uh oh!

cgwalters commented Jul 9, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

cgwalters left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cgwalters commented Jul 9, 2025

Uh oh!

igoropaniuk commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cgwalters commented Jul 9, 2025

Uh oh!

igoropaniuk commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

igoropaniuk commented Sep 19, 2024 •

edited

Loading

igoropaniuk commented Sep 23, 2024 •

edited

Loading

igoropaniuk commented Oct 16, 2024 •

edited

Loading

igoropaniuk commented Jul 9, 2025 •

edited

Loading

igoropaniuk commented Jul 9, 2025 •

edited

Loading