Skip to content

Conversation

eagleonhill
Copy link
Contributor

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.

@avagin avagin requested a review from Snorch June 17, 2025 14:11
@Snorch
Copy link
Member

Snorch commented Jun 18, 2025

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.

@eagleonhill eagleonhill force-pushed the c/fix-tailing-slash branch 6 times, most recently from 273c529 to 898d79f Compare June 19, 2025 06:30
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.

Signed-off-by: Chuan Qiu <[email protected]>
@Snorch Snorch force-pushed the c/fix-tailing-slash branch from 898d79f to dc830c3 Compare June 20, 2025 06:32
@Snorch
Copy link
Member

Snorch commented Jun 20, 2025

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

@avagin
Copy link
Member

avagin commented Jun 20, 2025

LGTM. Thanks a lot.

The test creates a file bindmount in criu mntns and binds it into test
mntns, this external file bindmount is autodetected and restored via
"--external mnt[]" criu option.

Note: In previous patch we fix the problem on this code path where file
bindmount restore fails as there is excess "/" in source path.

Signed-off-by: Pavel Tikhomirov <[email protected]>
@Snorch Snorch force-pushed the c/fix-tailing-slash branch from dc830c3 to dfee193 Compare June 21, 2025 02:51
@Snorch
Copy link
Member

Snorch commented Jun 21, 2025

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...).

@eagleonhill
Copy link
Contributor Author

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.

@avagin avagin merged commit 1bfa74d into checkpoint-restore:criu-dev Jun 25, 2025
38 of 45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants