fix: link remap restore on tmpfs#50
Conversation
Kimchi Code Review
Summary📊 Review Score: 94/100 (overall code quality — 0 lowest, 100 highest) 🧪 Tests: yes — A new ZDTM test No significant issues found. LGTM! 🎉 What to expectKimchi will analyze the changes in this pull request and post:
The review typically completes within a few minutes. This comment will be updated once the review is ready. Interact with Kimchi
ConfigurationReviews are configured by your organization admin. Powered by Kimchi — AI-powered code review by CAST AI |
![kimchi-review[bot]](https://avatars.githubusercontent.com/in/3223788?s=60&v=4){kind=small}
There was a problem hiding this comment.
📊 Review Score: 94/100 (overall code quality — 0 lowest, 100 highest)
⏱️ Estimated effort to review: 2/5 (1 = trivial, 5 = very complex)
🧪 Tests: yes — A new ZDTM test tmpfs_link_remap was added alongside the fix. It reproduces the exact POSIX-semaphore-on-tmpfs pattern (file on /dev/shm, mmapped, hard-linked, then unlinked) that triggers the buggy dump_linked_remap path. The test verifies both mmap content integrity via CRC and fd metadata after restore, providing solid coverage for the new TMPFS_MAGIC fallback branch.
No significant issues found. LGTM! 🎉
|
Ack |
The tmpfs_link_remap test was hardcoding /dev/shm paths which don't exist in the ns test flavor's mount namespace. Rewrite to accept --dirname, mount tmpfs on it, and create test files there. Move from TST_NOFILE back to TST_DIR accordingly. Co-Authored-By: Kimchi <noreply@kimchi.dev>
The test needs CAP_SYS_ADMIN to call mount(). Add 'flags': 'suid' to the desc file, matching tempfs.desc and mntns_link_remap.desc. Co-Authored-By: Kimchi <noreply@kimchi.dev>
The blanket TMPFS_MAGIC guard broke mntns_link_remap which uses dump_linked_remap on CRIU-managed tmpfs (saved via tar). The link_remap.N file IS preserved in the tar archive for managed mounts, so dump_linked_remap works correctly there. Narrow the guard to only trigger for external tmpfs mounts (e.g. /dev/shm provided by the container runtime), where CRIU does not dump the mount contents and link_remap.N would be lost on the destination. Co-Authored-By: Kimchi <noreply@kimchi.dev>
3 participants
Kimchi Summary
What changed
Fixes restore failures for mmapped files on external tmpfs mounts (such as
/dev/shmin containers) that have been unlinked but retainst_nlink >= 1by falling back todump_ghost_remap()to embed the file content directly in the checkpoint image. Adds a ZDTM regression test reproducing the POSIX semaphore pattern.Why
On external tmpfs mounts CRIU does not dump mount contents, so
link_remap.Nfiles created bydump_linked_remap()are lost during migration; the destination node restores with an empty tmpfs, causing ENOENT and broken VMAs. This change detects external tmpfs mounts and stores the file as a ghost remap instead.Key changes
criu/files-reg.c: Incheck_path_remap(), added aTMPFS_MAGICcheck that falls back todump_ghost_remap()when the mount is external (mi->external) or an external bind (mnt_is_external_bind(mi)).test/zdtm/static/Makefile: Addedtmpfs_link_remaptest target.test/zdtm/static/tmpfs_link_remap.c: Added new regression test that creates a tmpfs file,mmaps it, hard-links and unlinks the original name (leavingst_nlink=1), then verifies content integrity across checkpoint/restore.test/zdtm/static/tmpfs_link_remap.desc: Added test descriptor requiring--link-remap.Impact