CFE-90: reconcile storage mount options when they drift (opt-in remount)#6222
Draft
nickanderson wants to merge 5 commits into
Draft
CFE-90: reconcile storage mount options when they drift (opt-in remount)#6222nickanderson wants to merge 5 commits into
nickanderson wants to merge 5 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Member
Author
|
@cf-bottom jenkins please |
nickanderson
added a commit
to nickanderson/core
that referenced
this pull request
Jul 9, 2026
- Remove unnecessary NULL guards before free() in DeleteMountInfo(). free(NULL) is a C-standard no-op; bare calls match the surrounding convention (FreeOptionsList, VerifyMount, etc.). - Add assert(a != NULL) to LiveMountConverged, ReconcileMountOptions, FileSystemMountedCorrectly, and VerifyMountPromise to resolve the 15 null-dereference warnings. This follows the existing convention in VerifyInFstab, VerifyMount, and VerifyUnmount which already had these asserts.
Member
Author
|
@cf-bottom jenkins please |
|
Sure, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/14143/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-14143/ |
Member
Author
|
@cf-bottom jenkins please |
|
Sure, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/14145/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-14145/ |
Member
Author
|
@cf-bottom jenkins please |
|
Sure, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/14148/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-14148/ |
nickanderson
added a commit
to nickanderson/core
that referenced
this pull request
Jul 11, 2026
- Remove unnecessary NULL guards before free() in DeleteMountInfo(). free(NULL) is a C-standard no-op; bare calls match the surrounding convention (FreeOptionsList, VerifyMount, etc.). - Add assert(a != NULL) to LiveMountConverged, ReconcileMountOptions, FileSystemMountedCorrectly, and VerifyMountPromise to resolve the 15 null-dereference warnings. This follows the existing convention in VerifyInFstab, VerifyMount, and VerifyUnmount which already had these asserts.
nickanderson
force-pushed
the
CFE-90/master_mount_options
branch
from
9de8d09 to
4aedfe0
Compare
Member
Author
|
@cf-bottom jenkins please |
|
Alright, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/14151/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-14151/ |
nickanderson
force-pushed
the
CFE-90/master_mount_options
branch
5 times, most recently
from
cfab7e5 to
bbfbe27
Compare
Member
Author
|
@cf-bottom jenkins please |
|
Sure, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/14155/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-14155/ |
nickanderson
force-pushed
the
CFE-90/master_mount_options
branch
2 times, most recently
from
fb6ed82 to
ed434fe
Compare
Previously mount_options only affected the initial fstab write and the
initial mount; a filesystem already mounted with the wrong options was
never corrected, because `mount -a` skips already-mounted filesystems.
This adds opt-in reconciliation of a live mount when its options drift
from the promise.
New mount body attributes:
- remount (default false): manage the options of an already-mounted
filesystem. When false a mounted filesystem with the correct source is
considered kept regardless of option drift (backwards compatible;
options still drive the initial mount and, with edit_fstab, the fstab
entry).
- remount_methods (default { remount }): ordered mechanisms tried in turn,
re-reading the live mount after each to verify it satisfies the promise.
Defaults to the non-disruptive in-place remount only; the disruptive
unmount_mount (which tears the filesystem down and back up, interrupting
anything using it) is opt-in and is required to change options a live
remount cannot, e.g. NFS-negotiated vers=/rsize= or the server. The
kernel returns success from a remount even when it silently ignores
NFS-negotiated options, so the resulting state is verified rather than
the command exit status trusted.
- remount_timeout: bounds the unmount/mount path against a hung or
unreachable server.
When reconciliation is enabled the live mount is corrected first; if
edit_fstab is set the fstab entry is then updated regardless of the live
outcome (recording intent moves the system closer to the desired state,
and the live result is reported as its own promise outcome).
The promise is matched against the live mount by first resolving it with
util-linux "last wins" semantics - exactly as `mount -o` applies a list, a
later option overrides an earlier conflicting one (so "defaults,ro" is a
read-only mount and "ro,rw" is rw) - and then checking each surviving option.
Only the options the promise names are enforced; any option it does not name
is left unpoliced, because its provenance is unknown (kernel-negotiated like
vers=/rsize=, or added by a prior manual mount, indistinguishable). An option
the promise does specify - including a negotiated one such as rsize=8192 -
must be present in the live mount, with its inverse absent. Inverse pairs
(noatime/relatime, hard/soft, ro/rw, sync/async, generic no<opt>/<opt>) and
tcp/udp<->proto= aliases are recognized. The "defaults" pseudo-option (never
echoed by the kernel) expands to its checkable components rw,suid,dev,exec,
async (mount(8): defaults = rw,suid,dev,exec,auto,nouser,async; auto/nouser
are fstab/permission concepts, not runtime state, so they are not enforced) -
so "defaults" holds unless ro/nosuid/nodev/noexec/sync is present, and a later
explicit option can override any of those components. A correctly-mounted
filesystem thus converges instead of being reported changed every run. When a
later option overrides an earlier conflicting one, that is logged at verbose.
When reconciling via an in-place remount, "defaults" is likewise expanded to
"rw,suid,dev,exec,async" in the mount command (util-linux does not apply the
options implied by a bare "defaults" on a remount), so a filesystem that has
drifted to read-only (or nosuid, etc.) is restored in place rather than only
via the disruptive unmount_mount; mount's own last-wins then applies any
trailing override (e.g. "defaults,ro" remounts as ...,ro).
Mount and unmount side effects are gated on the promise action
(MakingInternalChanges), not a bare !DONTDO: a dry-run (-n) or
action_policy => "warn" promise now reports WARN without defining the
classes body's promise_repaired set, so a dependent promise keyed on
those classes no longer fires on a no-op run (CFE-3366). A live remount
is a real side effect outside the simulate sandbox, so the normal-mode
gate is used.
Also corrects the mount-info handling this relies on:
- GetFstabEntryOptions returned the fstab type field instead of the
options field, causing a spurious fstab rewrite and reported change
every run.
- ReplaceFstabEntry leaked the previous entry string.
- The mounted-FS scan no longer dropped the fstype used by
IsForeignFileSystem; fstype and kernel-resolved options are now stored
separately (options vs raw_opts).
- Restored unconditional creation of the mount point directory before
mounting, and stopped arming `mount -a` for an already-mounted
filesystem.
- "device busy" interruptions are logged at LOG_LEVEL_ERR (the outcome is
INTERRUPTED, cf. RecordInterruption), and the leaked options strings on
the error paths are freed.
References for the kernel/OS behavior asserted in the option-matching and
convergence code: mount(8) (`mount -a` skips already-mounted filesystems;
`defaults` = rw,suid,dev,exec,auto,nouser,async) and nfs(5) (options
negotiated by client and server are reported in /proc/mounts; NFS-specific
options cannot be changed by a remount).
Ticket: CFE-90
Ticket: CFE-1864
Ticket: CFE-3366
Changelog: Title
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Covers OptionsSubsetMatches: named-option-only enforcement (kernel-added options ignored), order independence for non-conflicting options, inverse pairs (noatime/relatime, ro/rw, hard/soft, sync/async, generic no<opt>/<opt>), tcp/udp<->proto= aliases, the "defaults" expansion (held unless ro/nosuid/nodev/noexec/sync is present), and the "last wins" resolution of conflicting options (defaults,ro -> read-only; ro,rw -> rw; a later explicit option overrides a "defaults" component). Also covers RemountOptionString (expanding "defaults" to rw,suid,dev,exec,async for the remount command). Runs unprivileged in CI via "make -C tests/unit check"; the behavioral mount/remount test belongs in the system-testing repo (needs root + a real NFS server). Ticket: CFE-90 Changelog: None Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A storage promise whose filesystem was already mounted with the correct source reported "mounted as promised" and never touched fstab, so a missing fstab entry was not restored and an options change was not written to fstab (CFE-1539). VerifyInFstab now also runs on the mounted-correctly path (when edit_fstab is set), independent of the opt-in live 'remount': keeping fstab correct is the documented behavior of mount_options, while remounting a live filesystem stays the disruptive, opt-in part gated by 'remount'. The fstab options comparison is left exact (order-sensitive): fstab option order is significant for duplicated/conflicting options, and the earlier GetFstabEntryOptions fix already stops the every-run rewrites. Ticket: CFE-1539 Changelog: Title
A storage promise for a not-yet-mounted filesystem armed CF_MOUNTALL, so CFEngine ran 'mount -a'/'mount -va' at the end of the pass. That mounts every unmounted entry in fstab - unrelated devices and foreign filesystem types included - as a side effect of a single promise (CFE-1863). The not-mounted path now mounts just the promised filesystem with VerifyMount (the existing surgical primitive), then persists it to fstab when edit_fstab is set. CF_MOUNTALL / MountAll are kept for the explicit 'mountfilesystems' agent control, which still means "mount everything in fstab". Ticket: CFE-1863 Changelog: Title
An unmount promise that named mount_source/mount_server was warned about as "probably an error", and the server was never used to select which mount to act on - so a policy could not unmount one specific mount (e.g. from a server being migrated away) without affecting others (CFE-2350). The bogus warning is removed, and the server (host) is now part of the "mounted correctly" identity check, gated on remount or unmount so it only engages when the promise opts into disruptive mount management. An unmount promise that finds a *different* filesystem at the mount point now leaves it - and its fstab entry - untouched. LiveMountConverged likewise treats the server as part of identity, so a remount-in-place that cannot change the server escalates to unmount_mount. Ticket: CFE-2350 Changelog: Title
nickanderson
force-pushed
the
CFE-90/master_mount_options
branch
from
ed434fe to
0669916
Compare
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
3 participants
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.
Storage/mount promises only applied
mount_optionsto the initial mount and (ifedit_fstabis enabled) the fstab entry. A filesystem already mounted with different options was never corrected, becausemount -askips already-mounted filesystems. This PR adds opt-in reconciliation of a live mount when its options drift from the promise, and along the way fixes several long-standing storage-promise bugs (CFE-1539, CFE-1863, CFE-2350, CFE-3366).Opt-in live remount reconciliation (CFE-90, CFE-1864)
New
mountbody attributes:remount(defaultfalse) — reconcile a live mount's options when they differ. When false, a mounted filesystem with the correct source is kept regardless of option drift (backwards compatible; options still drive the initial mount and, withedit_fstab, the fstab entry).remount_methods(default{ remount }) — mechanisms tried in order, re-reading the live mount after each (the kernel returns success from a remount even when it silently ignores NFS-negotiated options). Defaults to the non-disruptive in-placeremountonly; the disruptiveunmount_mount(which tears the filesystem down and back up) is opt-in, and is required to change options a live remount cannot — e.g. NFS-negotiatedvers=/rsize=, or the server.remount_timeout— protect against a hung or unreachable server.Option comparison (live mount). Only the options the promise names are enforced; any option it does not name is left unpoliced — its provenance is unknown (kernel-negotiated like
vers=/rsize=/proto=/sec=, or added by a prior manualmount -o, indistinguishable). The promise is first resolved with util-linux "last wins" semantics — exactly asmount -oapplies a list, a later option overrides an earlier conflicting one, sodefaults,rois a read-only mount andro,rwisrw. Each surviving option must then hold on the live mount: its inverse absent, and it either present or a default-on flag. Inverse pairs (noatime/relatime,hard/soft,ro/rw,sync/async, genericno<opt>/<opt>) andtcp/udp↔proto=aliases are recognized; an option the promise specifies (including a negotiated one such asrsize=8192) must be present. A correctly-mounted filesystem converges instead of being reported changed every run, and an override (roshadowing an earlierrw) is logged at verbose.The
defaultspseudo-option.defaults(=rw,suid,dev,exec,auto,nouser,async, per mount(8)) is never echoed by the kernel, so it's expanded to its checkable componentsrw,suid,dev,exec,asyncand subjected to the same last-wins resolution. It holds unless one of the violating negatives is present —ro(vsrw),nosuid/nodev/noexec(vssuid/dev/exec), orsync(vsasync) — and a later explicit option overrides the matching component (sodefaults,rorequires read-only,defaults,nosuidallowsnosuid).auto/nouserare fstab / mount-permission concepts, not runtime state, so they aren't enforced. When reconciling a drifteddefaultsmount in place, the remount command likewise usesrw,suid,dev,exec,async(util-linux doesn't apply the options implied by a baredefaultson a remount), and mount's own last-wins applies any trailing override — so a mount that drifted toro/nosuid/etc. is restored non-disruptively rather than only viaunmount_mount.fstab maintenance for already-mounted filesystems (CFE-1539)
Previously a filesystem already mounted with the correct source was reported "mounted as promised" and fstab was never consulted, so a missing fstab entry was not restored and an options change was not written until the mount happened to be redone.
VerifyInFstabnow runs on the mounted-correctly path too (whenedit_fstab => "true"), deliberately independent of the opt-in liveremount: keeping fstab correct is the documented behavior ofmount_options. fstab option comparison usesstrcmpbecause option order matters.Surgical single-filesystem mount, not
mount -a(CFE-1863)A storage promise for a not-yet-mounted filesystem used to arm
mount -a(mount -vaon Linux), which mounts every unmounted fstab entry — unrelated devices and foreign filesystem types included — as a side effect of a single promise. The not-mounted path now mounts just the promised filesystem surgically (VerifyMount), then persists it to fstab. Themount -amechanism (MountAll) is retained only for the explicitmountfilesystemsagent-control attribute, which still means "mount everything in fstab".Target a specific mount on unmount (CFE-2350)
An unmount promise that named
mount_source/mount_serverwas logged as "probably an error", and the server was never used to pick which mount to act on — so you couldn't unmount one specific mount (e.g. from a server being migrated away) without affecting others. The bogus warning is removed, and the server (host) is now part of the "mounted correctly" identity check, gated onremountorunmountso it only engages when the promise opts into disruptive mount management. An unmount promise that finds a different filesystem at the mount point leaves it — and its fstab entry — untouched, andLiveMountConvergedtreats the server as identity so a remount-in-place that can't change it escalates tounmount_mount.Correct dry-run / warn reporting (CFE-3366)
The mount, unmount and remount outcomes are based on the promise action (
MakingInternalChanges) rather than a bare!DONTDO. A dry-run (-n) orwarnpromise now reportsWARNwithout definingpromise_repaired, so dependent promises no longer fire on a no-op run.Supporting mount-info fixes
GetFstabEntryOptionsreturned the fstab type field instead of the options field (spurious rewrite every run).ReplaceFstabEntryleaked the previous entry string.optionsvsraw_opts).LOG_LEVEL_ERR(the outcome isINTERRUPTED), and leaked options strings on the error paths are freed.Testing
Unit coverage in
tests/unit/nfs_test.c(option subset matching, inverse/alias pairs, thedefaultsnegative-violation check, contradiction detection, and thedefaults→rw,suid,dev,exec,asyncremount expansion), runnable unprivileged viamake -C tests/unit check. The behavioral NFS reconcile/escalation, fstab maintenance, and surgical single-filesystem mount — which need root and a real NFS server — are covered by the system-testing PR and were exercised against a loopback NFS export during development.Commits
References
mount -askipping already-mounted filesystems, anddefaults=rw,suid,dev,exec,auto,nouser,async— mount(8)/proc/mounts) and NFS-specific options not modifiable on remount — nfs(5)Resolves CFE-90 (and its duplicate CFE-1864), CFE-1539 (fstab maintenance), CFE-1863 (
mount -ascope), CFE-2350 (target a specific mount on unmount), and CFE-3366 (dry-run/warn definingpromise_repaired) for storage mount promises.Ticket: https://northerntech.atlassian.net/browse/CFE-90
Together with: https://github.com/cfengine/system-testing/pull/693