feat: implement OCI per-mount propagation options by saku3 · Pull Request #3699 · youki-dev/youki

saku3 · 2026-08-22T08:41:46Z

Description

implement OCI per-mount propagation options

Type of Change

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update
Refactoring (no functional changes)
Performance improvement
Test updates
CI/CD related changes
Other (please describe):

Testing

Added new unit tests
Added new integration tests
Ran existing test suite
Tested manually (please provide steps)

Related Issues

Fixes #3604

Additional Context

nayuta723 · 2026-09-06T02:57:22Z

+        Ok(())
+    }
+
+    fn apply_mount_propagation(


apply_mount_propagation uses mount_setattr(2) for propagation flags (MS_SHARED, MS_SLAVE, etc.). However, mount_setattr(2) was added in Linux 5.12 (https://man7.org/linux/man-pages/man2/mount_setattr.2.html), and is not available on kernels still widely in use — notably Ubuntu 20.04 LTS (5.4), Debian 11 (5.10), Amazon Linux 2 (5.10), and RHEL 8 (4.18).

Neither runc nor crun uses mount_setattr(2) for propagation:

runc uses mount_setattr(2) exclusively for RecAttr (setRecAttr, rootfs_linux.go:1475) — attributes that cannot be applied recursively and atomically via mount(2). All other mount attributes (MS_RDONLY, MS_NOSUID, etc.) are set via mount(2) flags. Propagation flags are also applied via mount(2) in mountPropagate() (rootfs_linux.go:1458):

for _, pflag := range m.PropagationFlags { mountViaFds("", nil, m.Destination, dstFd, "", uintptr(pflag), "") }

crun similarly retains a mount(2) path for propagation inside do_mount() (linux.c:1317-1322):

if (mountflags & ALL_PROPAGATIONS_NO_REC) mount(NULL, real_target, NULL, mountflags & ALL_PROPAGATIONS, NULL);

Note that youki already uses mount_setattr(2) in other parts of the mount flow. For mount attribute setting (L671, L760), youki uses mount_setattr as part of its new mount API (open_tree → mount_setattr → move_mount), which has no equivalent in runc (runc uses mount(2) flags for attributes). For RecAttr (L681, L770), the AT_RECURSIVE usage is consistent with runc's setRecAttr. The issue is specifically that PR #3699 extends mount_setattr(2) to propagation flags, whereas both runc and crun use mount(2) for that purpose.

Since MS_REC can be passed directly to mount(2), using mount(NULL, dest, NULL, propagation_flag, NULL) would be sufficient and avoids the Linux 5.12 requirement for this step.

In terms of avoiding the Linux 5.12 requirement, doesn’t that have little practical meaning for youki as a whole, since mount_setattr is already used in other parts of the runtime?

If the only reason is simply to align the implementation with runc and crun, I’m not particularly convinced that this is worth doing.

If we switch this back to mount(2), how do we preserve the fd-based path resolution guarantees we get from mount_setattr? As far as I can tell, youki does not currently have a helper equivalent to runc's handling for safely applying a legacy mount operation to an already-resolved fd-backed target.

Even if we decide to implement this using mount, I think it should be treated as a fallback path and addressed in a separate issue.

Thanks, this is a good point. Sorry, I realize that my comment may not have made my intention clear.

My concern was not simply to align with runc or crun. I think we should separately discuss which kernel versions youki itself should support. Since youki already uses mount_setattr(2) in other parts of the runtime, switching this path to mount(2) would not meaningfully change the minimum kernel version for youki as a whole.

For now, I think it is reasonable to support kernel 5.12 and later. My main goal was to make sure we were aligned on the support policy.

For this PR, I’m fine with keeping mount_setattr(2) and considering the legacy mount(2) fallback, including safe fd-based path resolution, out of scope. We can discuss that separately in another issue.

I created an issue here.

#3726

nayuta723

I left some comments. Please take a look.

nayuta723 · 2026-09-06T15:03:16Z

+            let propagation = *flags
+                & (MsFlags::MS_SHARED
+                    | MsFlags::MS_SLAVE
+                    | MsFlags::MS_PRIVATE
+                    | MsFlags::MS_UNBINDABLE
+                    | MsFlags::MS_REC);
+            if propagation.is_empty() {
+                continue;
+            }


As far as I understand, this operation is a no-op. Are there any cases where flags other than this one can be set?

Thanks. Fixed.

nayuta723 · 2026-09-06T15:16:51Z

+                    MountOption::Unbindable(_, _) => {
+                        propagation_flags.push(MsFlags::MS_UNBINDABLE);
+                        continue;
+                    }


Is there any reason why we can’t write it this way?

Suggested change

MountOption::Unbindable(_, _) => {

propagation_flags.push(MsFlags::MS_UNBINDABLE);

continue;

}

MountOption::Unbindable(_, flag) => {

propagation_flags.push(flag);

continue;

}

nayuta723 · 2026-09-06T15:20:07Z

                    | MsFlags::MS_NODIRATIME
                    | MsFlags::MS_BIND
-                    | MsFlags::MS_UNBINDABLE,
+                    | MsFlags::MS_REC,


Does changing this mean that the existing behavior will also change? Is this change acceptable?

MsFlags::MS_REC is included here because of rbind.

rbind reads the flags directly, so there is no issue:

youki/crates/libcontainer/src/rootfs/mount.rs

Line 640 in a46e6a1

.map(|v| v.iter().any(|o| o == "rbind"))

MS_REC is supposed to be included here. Previously, however, propagation flags were not handled correctly, so it was missing.

The mount_recursive tests ensure that this change does not alter the existing behavior.

https://github.com/youki-dev/youki/tree/main/tests/contest/contest/src/tests/mounts_recursive

nayuta723 · 2026-09-06T15:22:58Z

+
+    test_group.add(vec![
+        Box::new(shared_test),
+        Box::new(rshared_test),


Please add the test case for slave.

Signed-off-by: Yusuke Sakurai <yusuke.sakurai@3-shake.com>

nayuta723

Sorry for the delay. LGTM!

saku3 added the kind/bug label Aug 22, 2026

saku3 force-pushed the feat-mount-propagation branch 2 times, most recently from 06d8a11 to be6fbe6 Compare August 22, 2026 08:48

nayuta723 reviewed Sep 6, 2026

View reviewed changes

saku3 requested a review from nayuta723 September 6, 2026 13:12

nayuta723 requested changes Sep 6, 2026

View reviewed changes

feat: implement OCI per-mount propagation options

be292ba

Signed-off-by: Yusuke Sakurai <yusuke.sakurai@3-shake.com>

saku3 force-pushed the feat-mount-propagation branch from be6fbe6 to be292ba Compare September 7, 2026 01:24

saku3 requested a review from nayuta723 September 7, 2026 01:35

nayuta723 approved these changes Sep 17, 2026

View reviewed changes

saku3 merged commit 7cd4328 into youki-dev:main Sep 18, 2026
30 checks passed

saku3 deleted the feat-mount-propagation branch September 18, 2026 11:23

github-actions Bot mentioned this pull request Sep 14, 2026

Release for v0.7.1 #3666

Open

ARMeeru mentioned this pull request Sep 19, 2026

test(contest): add cgroup v2 memory integration tests #3725

Merged

1 task

Conversation

saku3 commented Aug 22, 2026

Description

Type of Change

Testing

Related Issues

Additional Context

Uh oh!

nayuta723 Sep 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

nayuta723 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!

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!

nayuta723 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nayuta723 Sep 6, 2026 •

edited

Loading