criu icon indicating copy to clipboard operation
criu copied to clipboard

Fix trailing / when a file is bind-mounted

Open eagleonhill opened this issue 10 months ago • 1 comments

Fix an issue where a trailing / is added when a file bind mount is used in check-pointing. E.g. I have a /etc/hosts in workspace mounted from the host, and get the following message.

(00.141008)      1: mnt-v2: Create plain mountpoint /tmp/.criu.mntns.K1biY1/mnt-0000000938 for 938
(00.141546)      1: mnt-v2:     Mounting unsupported @938 (0)
(00.141887)      1: mnt-v2:     Bind /tmp/agent/1-d8c746c6fda3a8b2/workspace/etc/hosts/ to /tmp/.criu.mntns.K1biY1/mnt-0000000938
(00.142179)      1: Error (criu/mount-v2.c:319): mnt-v2: Failed to open_tree /tmp/agent/1-d8c746c6fda3a8b2/workspace/etc/hosts/: Not a directory
(00.143774) Error (criu/cr-restore.c:2320): Restoring FAILED.

eagleonhill avatar Jun 13 '25 06:06 eagleonhill

Code looks correct. But maybe it's better to instead remove trailing slashes in plain_mountpoint in or just after detect_is_dir() where we know if it's file or directory bindmount.

There is no signed-off-by in the commit message, @eagleonhill can you please update the commit to have:

Signed-off-by: Chuan Qiu <[email protected]>

see https://github.com/checkpoint-restore/criu/blob/criu-dev/CONTRIBUTING.md#developers-certificate-of-origin-11

Also while on it, please add "mount: " prefix to your commit subject and put PR description into commit message to explain things. see https://github.com/checkpoint-restore/criu/blob/criu-dev/CONTRIBUTING.md#describe-your-changes

I believe fix is simple enough to merge without test (if it's confirmed that it helps in this situation). But we need a regression test in zdtm so that we know that we don't break this case in future.

Snorch avatar Jun 18 '25 05:06 Snorch

I've rewrote the test

  • we should not use shared mounts when we don't need them
  • in your case auto mount detection code will probably not be hit
  • test should be in separate commit

Snorch avatar Jun 20 '25 06:06 Snorch

LGTM. Thanks a lot.

avagin avatar Jun 20 '25 18:06 avagin

Still one test fails:

image https://github.com/checkpoint-restore/criu/actions/runs/15791341032/job/44517392026?pr=2682 https://productionresultssa12.blob.core.windows.net/actions-results/f7c2ad75-0ecb-419d-94d8-737e32f582a3/workflow-job-run-a4cfd10f-f68f-50f4-9e49-82e92fd1600e/logs/job/job-logs.txt?rsct=text%2Fplain&se=2025-06-21T04%3A03%3A40Z&sig=agduxqQ1D4f%2B4gPCd63PAj9bW7Q7WRJ%2B%2FVjmDWQoiKc%3D&ske=2025-06-21T13%3A03%3A03Z&skoid=ca7593d4-ee42-46cd-af88-8b886a2f84eb&sks=b&skt=2025-06-21T01%3A03%3A03Z&sktid=398a6654-997b-47e9-b12b-9515b896b4de&skv=2025-05-05&sp=r&spr=https&sr=b&st=2025-06-21T03%3A53%3A35Z&sv=2025-05-05

2025-06-21T03:18:18.4387595Z + ./test/zdtm.py run -t zdtm/static/env00 --freezecg zdtm:t --fault 137
2025-06-21T03:18:18.5195779Z userns is supported
...
2025-06-21T03:18:19.2358397Z ========================= Run zdtm/static/env00 in uns =========================
2025-06-21T03:18:19.3086196Z Start test
2025-06-21T03:18:19.3572268Z ./env00 --pidfile=env00.pid --outfile=env00.out --envname=ENV_00_TEST
...
2025-06-21T03:18:19.6194172Z =[log]=> dump/zdtm/static/env00/197/1/restore.log
2025-06-21T03:18:19.6194515Z ------------------------ grep Error ------------------------
2025-06-21T03:18:19.6195032Z b'profiling:/home/runner/work/criu/criu/images/core-mips.gcda:Cannot open'
2025-06-21T03:18:19.6195562Z b'profiling:/home/runner/work/criu/criu/images/core-x86.gcda:Cannot open'
2025-06-21T03:18:19.6196073Z b'profiling:/home/runner/work/criu/criu/images/core.gcda:Cannot open'
2025-06-21T03:18:19.6196562Z b'profiling:/home/runner/work/criu/criu/images/stats.gcda:Cannot open'
2025-06-21T03:18:19.6196954Z b'Error: ipv4: Address already assigned.'
2025-06-21T03:18:19.6197433Z b'Error: ipv6: address already assigned.'
2025-06-21T03:18:19.6197753Z b'(00.067006)      1: mnt-v2: Mount 690 is detected as dir-mount'
2025-06-21T03:18:19.6198227Z b'(00.067008)      1: mnt-v2: Create plain mountpoint /tmp/.criu.mntns.CH1Sn5/mnt-0000000690 for 690'
2025-06-21T03:18:19.6198701Z b'(00.067013)      1: mnt-v2: \tMounting unsupported @690 (0)'
2025-06-21T03:18:19.6199189Z b'(00.067015)      1: mnt-v2: \tBind /home/runner/work/criu/criu to /tmp/.criu.mntns.CH1Sn5/mnt-0000000690'
2025-06-21T03:18:19.6199928Z b'(00.067025)      1: Error (criu/mount-v2.c:319): mnt-v2: Failed to open_tree /home/runner/work/criu/criu: Invalid argument'
2025-06-21T03:18:19.6200427Z b'(00.067742) uns: calling exit_usernsd (-1, 1)'
2025-06-21T03:18:19.6200751Z b'(00.067769) uns: daemon calls 0x557ef0d92410 (222, -1, 1)'
2025-06-21T03:18:19.6201057Z b'(00.067773) uns: `- daemon exits w/ 0'
2025-06-21T03:18:19.6201325Z b'(00.071092) uns: daemon stopped'
2025-06-21T03:18:19.6201630Z b'(00.071101) Error (criu/cr-restore.c:2324): Restoring FAILED.'
2025-06-21T03:18:19.6202094Z b'(00.074514) Error (criu/cgroup.c:1998): cg: cgroupd: recv req error: No such file or directory'
2025-06-21T03:18:19.6202543Z ------------------------ ERROR OVER ------------------------
2025-06-21T03:18:19.6202916Z ################# Test zdtm/static/env00 FAIL at CRIU restore ##################

I will look next week, but this does not reproduce on my node (also I'm not sure how fix in resolve_external_mounts can affect env00 test without automatic external mount resolving...).

Snorch avatar Jun 21 '25 04:06 Snorch

I've rewrote the test

  • we should not use shared mounts when we don't need them
  • in your case auto mount detection code will probably not be hit
  • test should be in separate commit

Thanks for updating the test. Just to confirm this updated test will repro the original issue before the fix.

eagleonhill avatar Jun 21 '25 04:06 eagleonhill