kernel: add helpers for recording and checking modules/firmware tree version by bboozzoo · Pull Request #17680 · canonical/snapd

bboozzoo · 2026-09-21T08:34:32Z

Introduce versioning of the organization of the kernel modules &
firmware trees. The patch adds a helper to record the version of the
layout and another helper to check and compare indicating whether the
tree was set up by older version of the code.

Both are essential for being able to determine if the tree needs to be
rebuilt/updated when updating snapd.

Cherry picked from #17646

Related: SNAPDENG-37501

Copilot

Copilot review overview

🟢 Approval recommended

The implementation is functionally sound; remaining feedback concerns minor documentation, spelling, and test cleanup.

Review effort: Balanced
Findings: 1 · 4

Open (5)

Restore generator version after assertion failure · New
Fix grammar in exported function documentation · New
Correct “have been build” to “have been built” · New
Fix comment typo: “he” to “the” · New
Remove or add the missing write-site rationale comment · New

What changed in this PR

Adds version metadata for kernel module and firmware trees, enabling future regeneration when layouts become stale.

Changes:

Records generator version metadata after successful tree creation.
Detects missing, corrupt, older, and newer metadata versions.
Adds unit and spread coverage.

File	Description
`kernel/kernel_drivers.go`	Implements metadata recording and version checking.
`kernel/export_test.go`	Exposes metadata test helpers.
`kernel/kernel_drivers_test.go`	Tests metadata and write failures.
`tests/core/basic20plus/task.yaml`	Verifies metadata on Core systems.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

codecov · 2026-09-21T08:58:27Z

Codecov Report

❌ Patch coverage is 86.27451% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.39%. Comparing base (2cbd01b) to head (0bea319).
⚠️ Report is 26 commits behind head on master.

Files with missing lines	Patch %	Lines
kernel/kernel_drivers.go	86.27%	5 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #17680      +/-   ##
==========================================
+ Coverage   78.35%   78.39%   +0.04%     
==========================================
  Files        1416     1417       +1     
  Lines      200464   200741     +277     
  Branches     2502     2503       +1     
==========================================
+ Hits       157070   157369     +299     
+ Misses      33972    33940      -32     
- Partials     9422     9432      +10

Flag	Coverage Δ
unittests	`78.39% <86.27%> (+0.04%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Copilot review overview

🟡 Changes recommended

The forward-version test compares equal versions and does not exercise its intended behavior.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2

Open (2)

Restore running version to cover newer-on-disk branch · New
Remove or add the missing write-site rationale comment

Resolved since last review (4)

Restore generator version after assertion failure
Fix comment typo: “he” to “the”
Correct “have been build” to “have been built”
Fix grammar in exported function documentation

…version Introduce versioning of the organization of the kernel modules & firmware trees. The patch adds a helper to record the version of the layout and another helper to check and compare indicating whether the tree was set up by older version of the code. Both are essential for being able to determine if the tree needs to be rebuilt/updated when updating snapd. Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>

Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>

Copilot

Copilot review overview

🟢 Approval recommended

The versioning behavior, failure handling, and relevant edge cases are implemented consistently and adequately tested.

Review effort: Balanced
Findings: None

Resolved since last review (2)

Restore running version to cover newer-on-disk branch
Remove or add the missing write-site rationale comment

github-actions · 2026-09-21T14:36:38Z

Thu Sep 24 15:08:06 UTC 2026
The following results are from: https://github.com/canonical/snapd/actions/runs/36011311318

No spread result artifacts found

No spread results JSON artifacts were available for this workflow run, so spread failures (if any) could not be reported.

pedronis

did a pass, some minor comments

pedronis · 2026-09-23T12:44:47Z

+func readDriversTreeGeneratorMeta(destDir string) (driversTreeMeta, error) {
+	data, err := os.ReadFile(driversTreeMetaPath(destDir))
+	if errors.Is(err, fs.ErrNotExist) {
+		return driversTreeMeta{}, nil


should this have:

GeneratorVersion: 0

to be explicit?

pedronis · 2026-09-23T12:45:55Z

+	var meta driversTreeMeta
+	if err := json.Unmarshal(data, &meta); err != nil {
+		// Treat unparseable metadata the same as missing: needs a check, or
+		// could be corrupted?


should this emphasize that the likely result of that is rebuilding the tree?

could also make it an error and turn into false a level above for clarity?

pedronis · 2026-09-23T12:47:18Z

+// DriversTreeNeedsCheck returns true when the kernel modules & firmware tree at
+// destDir was built by an older version of the generator code, indicating it
+// may need to be checked or rebuilt.
+func DriversTreeNeedsCheck(destDir string) (bool, error) {


/NeedsCheck/Outdated perhaps?

…e tree version

pedronis

thank you

…e tree version

andrewphelpsj

Thanks!

andrewphelpsj · 2026-09-24T12:13:54Z

        stat /run/systemd/system/"sysroot-writable-system\x2ddata-snap-$device\x2dkernel-"*

+        # Check the generator version
+        gojq '.["generator-version"]'  /var/lib/snapd/kernel/"$device"-kernel/x1/kernel.json | \


x1? I'm prob missing something but I don't see where this test installs a local kernel.

this is the core suite, so running on UC and we repack the kernel in prepare, so it's always unasserted AFAIU

garden:ubuntu-core-26-64 .../tests/core/basic20plus# snap list pc-kernel Name Version Rev Tracking Publisher Notes pc-kernel 7.0.0-38.38r1 x1 - - kernel,components[13]

Mohit-Chachada

Thanks!

Mohit-Chachada · 2026-09-24T13:23:33Z

+	// A tree marked with a version *newer* than what is currently running (e.g.
+	// after a snapd revert) is deliberately NOT considered outdated: rebuilding
+	// it with older, possibly-buggy logic could regress a fix already applied
+	// by the newer generator. Only consider the kernel tree to be outdated of


Suggested change

// by the newer generator. Only consider the kernel tree to be outdated of

// by the newer generator. Only consider the kernel tree to be outdated if

…e tree version

bboozzoo requested a balanced review from Copilot September 21, 2026 08:34

bboozzoo added the Run nested The PR also runs tests inluded in nested suite label Sep 21, 2026

Copilot started reviewing on behalf of bboozzoo September 21, 2026 08:35 View session

Copilot AI reviewed Sep 21, 2026

View reviewed changes

Comment thread kernel/kernel_drivers_test.go Outdated

Comment thread kernel/kernel_drivers.go Outdated

Comment thread kernel/kernel_drivers.go Outdated

Comment thread kernel/kernel_drivers.go Outdated

Comment thread kernel/kernel_drivers_test.go Outdated

bboozzoo force-pushed the bboozzoo/kernel-tree-generation-version branch from cae70a5 to 8a66a70 Compare September 21, 2026 09:43

bboozzoo requested a balanced review from Copilot September 21, 2026 09:43

Copilot started reviewing on behalf of bboozzoo September 21, 2026 09:44 View session

Copilot AI reviewed Sep 21, 2026

View reviewed changes

Comment thread kernel/kernel_drivers_test.go Outdated

bboozzoo added 2 commits September 21, 2026 12:41

tests/core/basic20plus: check kernel drivers tree metadata

ffeae86

Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>

bboozzoo force-pushed the bboozzoo/kernel-tree-generation-version branch from 8a66a70 to ffeae86 Compare September 21, 2026 10:41

bboozzoo requested a balanced review from Copilot September 21, 2026 10:42

Copilot started reviewing on behalf of bboozzoo September 21, 2026 10:42 View session

Copilot AI reviewed Sep 21, 2026

View reviewed changes

bboozzoo requested review from andrewphelpsj and pedronis September 21, 2026 10:58

bboozzoo added the Auto rerun spread Auto reruns spread up to 4 times in non-draft PRs w/ >=1 approval and <20 fails in any fund. system label Sep 23, 2026

bboozzoo closed this Sep 23, 2026

bboozzoo reopened this Sep 23, 2026

bboozzoo requested a review from miguelpires September 23, 2026 04:36

pedronis reviewed Sep 23, 2026

View reviewed changes

bboozzoo mentioned this pull request Sep 23, 2026

kernel: require a valid marker before skipping a KernelInstall rebuild #17701

Draft

fixup! kernel: add helpers for recording and checking modules/firmwar…

3776763

…e tree version

bboozzoo requested a review from pedronis September 23, 2026 13:47

pedronis approved these changes Sep 23, 2026

View reviewed changes

Mohit-Chachada reviewed Sep 24, 2026

View reviewed changes

fixup! kernel: add helpers for recording and checking modules/firmwar…

f11bdaa

…e tree version

bboozzoo requested a review from Mohit-Chachada September 24, 2026 12:37

andrewphelpsj approved these changes Sep 24, 2026

View reviewed changes

Mohit-Chachada approved these changes Sep 24, 2026

View reviewed changes

fixup! kernel: add helpers for recording and checking modules/firmwar…

0bea319

…e tree version

	// by the newer generator. Only consider the kernel tree to be outdated of
	// by the newer generator. Only consider the kernel tree to be outdated if

Conversation

bboozzoo commented Sep 21, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

github-actions Bot commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

No spread result artifacts found

Uh oh!

pedronis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pedronis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andrewphelpsj left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mohit-Chachada left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bboozzoo commented Sep 21, 2026 •

edited by atlassian Bot

Loading

codecov Bot commented Sep 21, 2026 •

edited

Loading

github-actions Bot commented Sep 21, 2026 •

edited

Loading