o/snapshotstate, o/s/backend: saving snapshot excludes all mount points#17182
Open
Mohit-Chachada wants to merge 4 commits into
Open
o/snapshotstate, o/s/backend: saving snapshot excludes all mount points#17182Mohit-Chachada wants to merge 4 commits into
Mohit-Chachada wants to merge 4 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR broadens snapshot creation behavior in overlord/snapshotstate so that any active mount points under a snap’s data directories (global and per-user) are automatically added to the snapshot’s dynamic exclude list, preventing mounted-in data from being captured.
Changes:
- Replace the mount-control/systemd-based mount discovery with
/proc/self/mountinfoparsing and map mounts under$SNAP_DATA,$SNAP_COMMON,$SNAP_USER_DATA,$SNAP_USER_COMMONinto snapshot excludes. - Add backend helper to map absolute snap data directories to the corresponding
$SNAP_*variables, including user directories (selected users or all users). - Update unit tests accordingly and replace the old spread test with a new one that covers both snapctl mounts and user-created mounts (where applicable).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/main/snapshot-excludes-mounts/task.yaml | New spread test validating that mount points under snap data dirs are excluded from snapshots (including user dirs for snap save). |
| tests/main/snapshot-excludes-mount-control-mounts/task.yaml | Removed older spread test limited to mount-control mounts; superseded by the new broader test. |
| overlord/snapshotstate/snapshotmgr.go | Implement mount exclusion via osutil.LoadMountInfo() and generalized mapping to $SNAP_* excludes for global + user dirs. |
| overlord/snapshotstate/snapshotmgr_test.go | Update tests to use mocked mountinfo and validate mapping/exclusion behavior across global + user dirs and error paths. |
| overlord/snapshotstate/export_test.go | Export updated mapping helper and add a mock hook for backend dir→var mapping. |
| overlord/snapshotstate/backend/mountunit.go | Removed systemd mount-unit helper (no longer needed). |
| overlord/snapshotstate/backend/mountunit_test.go | Removed tests for the deleted systemd mount-unit helper. |
| overlord/snapshotstate/backend/backend.go | Add MapSnapDataDirToSnapVar helper used by snapshotstate to build exclude patterns. |
| overlord/snapshotstate/backend/backend_test.go | Add coverage for MapSnapDataDirToSnapVar behavior (all users, selected users, and error handling). |
Mohit-Chachada
force-pushed
the
snapshot-excludes-all-mounts
branch
from
4531d32 to
a933b67
Compare
|
Mon Jun 15 15:00:44 UTC 2026 Failures:Preparing:
Executing:
Restoring:
Skipped tests from snapd-testing-skipIf you wish to have any of the below tests run in your PR, in your PR description, add 'unskip:' followed by a copy-and-pasted list (without variants) of the below tests you wish to run (unskip plus test list must be valid yaml)
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #17182 +/- ##
==========================================
+ Coverage 79.18% 79.19% +0.01%
==========================================
Files 1374 1376 +2
Lines 193037 193329 +292
Branches 2466 2466
==========================================
+ Hits 152855 153111 +256
- Misses 30994 31028 +34
- Partials 9188 9190 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
bboozzoo
reviewed
Jun 15, 2026
Comment on lines
+221
to
+225
| old := backendMapSnapDataDirToSnapVar | ||
| backendMapSnapDataDirToSnapVar = f | ||
| return func() { | ||
| backendMapSnapDataDirToSnapVar = old | ||
| } |
Contributor
There was a problem hiding this comment.
Suggested change
| old := backendMapSnapDataDirToSnapVar | |
| backendMapSnapDataDirToSnapVar = f | |
| return func() { | |
| backendMapSnapDataDirToSnapVar = old | |
| } | |
| return testutil.Backup(&backendMapSnapDataDirToSnapVar, f) |
Contributor
Author
There was a problem hiding this comment.
Thanks, i think you were referring to testutil.Mock.
While at it, I updated all mocks in this file to use testutil.Mock in edd726e
Comment on lines
+294
to
+296
| for i, e := range entries { | ||
| mountPts[i] = e.MountDir | ||
| } |
Contributor
There was a problem hiding this comment.
could we exclude mount points that do not have the prefixes we care about?
Contributor
Author
There was a problem hiding this comment.
that's actually done in mapMountPointsInDataDirsToExcludes in a single loop.
I wanted to avoid doing very similar prefix matching for [ filtering ] and then for [ snap variable replacement ], so instead it is combined in a single loop.
Had also added Paths outside the snap's data directories are skipped because they are not included in the snapshot. to the method doc.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follows #17119
Extends the previously added exclusion of snapctl mounts to now exclude all mount points under snap data dirs while saving a snapshot.
Excludepattern and appended toSnapshotOptions's dynamic Exclude listReview hint: due to spread test renaming, the overall diff shows the old file as deleted and new one as added. For reviewing just the diff of the spread test, look at commit a933b67
SNAPDENG-37006