Skip to content

fix(libcontainer): preserve sync and dirsync on fd-based mounts - #3838

Open
dongxuelian2 wants to merge 1 commit into
youki-dev:mainfrom
dongxuelian2:codex/fix-sync-dirsync-mounts
Open

dongxuelian2 wants to merge 1 commit into
youki-dev:mainfrom
dongxuelian2:codex/fix-sync-dirsync-mounts

Conversation

@dongxuelian2

Copy link
Copy Markdown

Description

Non-bind mounts created through fsopen/fsconfig silently lose sync and dirsync. The OCI option parser already records MS_SYNCHRONOUS and MS_DIRSYNC, but the fd-based mount path only forwards filesystem data to fsconfig; neither flag can be applied by mount_setattr.

Pass the enabled flags as FSCONFIG_SET_FLAG options before creating the filesystem. This uses the existing parsed flags, so async still clears sync in option order. The existing legacy fallback continues to receive the original mount flags and propagate mount errors.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Testing

  • Added new unit tests
  • Added new integration tests (opt-in privileged kernel test)
  • Ran existing relevant tests
  • Tested manually by a human

Automated validation on Ubuntu 24.04, x86_64, Linux 6.17.0-1022-azure:

  • The new syscall-flow regression test fails against the original production code with the missing sync fsconfig call, and passes with the fix.
  • Relevant default-feature tests: 46 passed, with the privileged test ignored in that ordinary run.
  • Rootfs tests with all libcontainer features: 42 passed, with the privileged test ignored in that ordinary run.
  • The privileged kernel test was then run explicitly as root in a private mount namespace: 1 passed, covering four fresh tmpfs configurations.
  • The issue's complete static-BusyBox OCI fixture was run before and after the fix, with fresh bundles and state directories: seven valid option/control cases matched the expected mountinfo observations, and an unsupported filesystem option failed creation with EINVAL in both versions.
  • cargo +nightly fmt --all -- --check: passed.
  • cargo +1.96.0 clippy -p libcontainer --all-targets --all-features --locked -- -D warnings: passed with the repository's pinned toolchain.
  • cargo +1.96.0 clippy --workspace --all-targets --all-features --locked -- -D warnings: passed.
  • cargo +stable test --workspace --all-features --locked --no-fail-fast -- --test-threads=1: 570 passed, 1 ignored (the privileged test described above), using the documented serial setting and a fresh devpts instance in a private mount namespace.
  • git diff --check: passed.

The original parallel workspace test command timed out in process::fork::test::test_container_err_fork. A serial run on the runner's original PTY setup failed two TTY tests; the same two failures were reproduced on unmodified upstream. The complete serial suite passed after creating a devpts instance with ptmxmode=0666 inside a private mount namespace. No tests were skipped or assertions relaxed to obtain that result.

Current stable 1.99.1 workspace clippy still reports two manual_filter errors in unchanged syscall/linux.rs at lines 722 and 730. The pinned 1.96.0 check passes. No lint suppression or unrelated source changes were added.

Mount and OCI validation logs, passing full workspace tests and upstream PTY comparison, workspace clippy results.

The tmpfs checks establish flag retention, not disk durability or performance. Other kernels, filesystems and architectures were not exercised.

Related Issues

Fixes #3818

Additional Context

AI assistance was used for investigation, implementation, regression tests, validation, and drafting this description. The reported checks were executed through automated tools; no human review is claimed. The contribution is one commit signed off using the author's existing Git identity. Validation-only workflow and fixture files are kept on a separate fork branch and are excluded from this PR.

Signed-off-by: dongxuelian2 <2984896280@qq.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused fix preserves existing mount semantics and fallback behavior, with appropriate regression coverage and no unresolved findings.

No open issues 🎉

What changed in this PR

Fixes #3818 by preserving sync and dirsync on non-bind mounts created through the file-descriptor-based mount path.

Changes:

  • Forwards enabled superblock flags through fsconfig before filesystem creation.
  • Preserves parsed async ordering and existing legacy fallback behavior.
  • Adds parser, syscall-flow, fallback, and opt-in privileged kernel regression tests.
File Description
crates/​libcontainer/​src/​rootfs/​utils.rs Adds tests for parsing sync flags and option ordering.
crates/​libcontainer/​src/​rootfs/​mount.rs Applies superblock flags through fsconfig and adds regression coverage.

Review effort: Balanced

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: youki silently drops sync and dirsync on tmpfs mounts

3 participants