env: detect the lowerdir overlay mount options at runtime #1192
Conversation
| @@ -1073,11 +1076,22 @@ impl OverlayMountOptions { | |||
| fn new(rt: &runtime::Runtime) -> Self { | |||
| Self { | |||
| read_only: !rt.status.editable, | |||
| lowerdir_append: true, | |||
| break_hardlinks: true, | |||
| metadata_copy_up: true, | |||
| } | |||
There was a problem hiding this comment.
Slightly related -- one observation is that break_hardlinks is always true yet to_options() has behavior that checks whether it's false (which can never happen).
In an earlier iteration I was looking at whether we can toggle all of these bools, not just lowerdir_append, inside query() so that to_options() can be simplified. The rationale was to avoid having to query the params hashset in both query() and to_options() so that we can do it just once inside query().
This is the check inside of to_options() that I'm referring to:
if !self.break_hardlinks && params.contains(OVERLAY_ARGS_INDEX) {
opts.push(OVERLAY_ARGS_INDEX_ON);
}
This seems like dead code because break_hardlinks is never false. I figured that maybe this was left this way for possible future changes where some other component might be toggling break_hardlinks, but I just wanted to point this out in case this is something that was an oversight that needs some cleanup to remove the break_hardlinks bool.
In any case, let me know if we think that adjusting more of these bools inside of query() with the goal of simplifying to_options() and removing the call to let params = runtime::overlayfs::overlayfs_available_options(); is worth doing.
This PR is currently a minimal but slightly less efficient approach. If we did want to toggle these other bools inside of query(), and/or cleanup the break_hardlinks bools, then that could be a separate follow-up PR.
There was a problem hiding this comment.
IIRC the break_hardlinks was something that I had working and either it didn't work the way that I thought or there was some adjustment that we made to the logic which has orphaned it as you note. Definitely worth looking into as a separate effort
davvid
force-pushed
the
detect-mount-options
branch
from
bedc232 to
a5bd78e
Compare
Replace the "legacy-mount-options" feature flag with runtime detection of this feature. We assume that systems with kernel version v6.8 and newer are capable of using the "lowerdir+" mount option. Related-to: spkenv#968 Related-to: spkenv#1164
davvid
force-pushed
the
detect-mount-options
branch
from
a5bd78e to
d14cc2b
Compare
Replace the "legacy-mount-options" feature flag with runtime detection of this feature. We assume that systems with kernel version v6.8 and newer are capable of using the "lowerdir+" mount option.
Related-to: #968
Related-to: #1164