Support manage-cgroup-mode option for checkpoint by donkomura · Pull Request #3452 · youki-dev/youki

donkomura · 2026-03-13T16:32:25Z

Description

Add support for CRIU's --manage-cgroup-mode option in checkpoint command. This primarily affects restore behavior, and with "ignore" mode, cgroup information will not be dumped.

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

#3394

Additional Context

For detail about the option, see https://criu.org/CGroups.

utam0k

Is it hard to add the unit or e2e test?

donkomura · 2026-03-14T15:55:11Z

I missed that checkpoint tests were implemented in the contest. I have added test cases.

nayuta723

Thanks! I've left some comments. Please take a look when you have a chance.

nayuta723 · 2026-03-15T06:42:19Z

crates/liboci-cli/src/checkpoint.rs

@@ -50,8 +50,8 @@ pub struct Checkpoint {
    /// #[clap(long)]
    /// pub pre_dump: bool,
    /// TODO: Cgroups mode


Please delete TODO comment.

I would like to put this on hold until #3455 is merged.

Is the purpose to prevent conflicts? If so, I will leave it as is.

crates/libcontainer/src/container/container_checkpoint.rs

tests/contest/contest/src/tests/lifecycle/checkpoint.rs

tests/contest/contest/src/tests/lifecycle/container_lifecycle.rs

crates/libcontainer/src/container/container_checkpoint.rs

nayuta723

Thanks for the update! I've reviewed your changes and left a few comments. Please take a look when you have a chance.

tests/contest/contest/src/tests/lifecycle/checkpoint.rs

nayuta723 · 2026-03-17T02:04:33Z

crates/liboci-cli/src/checkpoint.rs

@@ -50,8 +50,8 @@ pub struct Checkpoint {
    /// #[clap(long)]
    /// pub pre_dump: bool,
    /// TODO: Cgroups mode


I would like to put this on hold until #3455 is merged.

crates/libcontainer/src/container/container.rs

tests/contest/contest/src/tests/lifecycle/container_lifecycle.rs

nayuta723 · 2026-03-17T03:02:58Z

tests/contest/contest/src/tests/lifecycle/checkpoint.rs

+    let temp_dir = match tempfile::tempdir() {
+        Ok(td) => td,
+        Err(e) => {
+            return TestResult::Failed(anyhow::anyhow!(
+                "failed creating temporary directory {:?}",
+                e
+            ));
+        }
+    };
+    let image_path = temp_dir.path().join("checkpoint");
+    if let Err(e) = std::fs::create_dir(&image_path) {
+        return TestResult::Failed(anyhow::anyhow!(
+            "failed creating checkpoint directory ({:?}): {}",
+            &image_path,
+            e
+        ));
+    }


Please refactor the common logic into a shared function. BTW, is it necessary to create the image_path there?

Without specifying image_path, we can't control where CRIU writes the image files, and thus can't inspect them.

I understand that the checkpoint function must have an image_path to operate. If it remains an Option<&Path>, the function should internally generate a default path when None is provided. Alternatively, we could remove the Option and make it a required argument passed from the outside.
I recommend the first approach, as providing a default path internally seems more convenient.

Ok, I will set the default path.

crates/liboci-cli/src/checkpoint.rs

donkomura

I'm fixing it now. I'll send a review request when it done.

donkomura · 2026-03-17T16:54:14Z

tests/contest/contest/src/tests/lifecycle/checkpoint.rs

+    let temp_dir = match tempfile::tempdir() {
+        Ok(td) => td,
+        Err(e) => {
+            return TestResult::Failed(anyhow::anyhow!(
+                "failed creating temporary directory {:?}",
+                e
+            ));
+        }
+    };
+    let image_path = temp_dir.path().join("checkpoint");
+    if let Err(e) = std::fs::create_dir(&image_path) {
+        return TestResult::Failed(anyhow::anyhow!(
+            "failed creating checkpoint directory ({:?}): {}",
+            &image_path,
+            e
+        ));
+    }


Without specifying image_path, we can't control where CRIU writes the image files, and thus can't inspect them.

crates/libcontainer/src/container/container.rs

nayuta723

Apologies for the back-and-forth on my end. I’ve added some nit comments based on the latest implementation strategy. Please check them when you can. Thanks again for your flexibility!

nayuta723 · 2026-03-21T16:54:51Z

tests/contest/contest/src/tests/lifecycle/checkpoint.rs

 fn checkpoint(
    project_path: &Path,
    id: &str,
+    image_path: Option<&Path>,


I’m sorry to backtrack on my previous comment, but I think it’s better to remove the Option after all, as there is processing inside the function that relies on image_path.

Suggested change

image_path: Option<&Path>,

image_path: &Path,

nayuta723 · 2026-03-21T17:08:04Z

tests/contest/contest/src/tests/lifecycle/checkpoint.rs

-    if let Err(e) = setup_network_namespace(project_path, id) {
-        return e;
-    }
+) -> Result<(), TestResult> {


I’m also sorry to backtrack again, but I think it’s better to remove the Result as well. Requiring the caller to handle errors with match every time feels a bit excessive/clunky in this case.

Suggested change

) -> Result<(), TestResult> {

) -> TestResult {

donkomura · 2026-03-26T16:32:01Z

Thank you for reviewing, and sorry for the late reply. (I was on vacation.)

nayuta723

LGTM, Thank you for contributions!

saku3 · 2026-04-01T06:34:35Z

crates/youki/src/commands/checkpoint.rs

+        "full" => Ok(rust_criu::CgMode::FULL),
+        "strict" => Ok(rust_criu::CgMode::STRICT),
+        "soft" => Ok(rust_criu::CgMode::SOFT),
+        _ => Err(anyhow::anyhow!("manage-cgroup-mode: {s}")),


Suggested change

_ => Err(anyhow::anyhow!("manage-cgroup-mode: {s}")),

_ => Err(anyhow::anyhow!("invalid manage-cgroups-mode: {s}")),

Alternatively, since this is already validated by clap::builder::PossibleValuesParser, I think the following would also be reasonable.

_ => unreachable!("invalid manage-cgroups-mode: {s}"),

saku3 · 2026-04-01T06:37:58Z

crates/libcontainer/src/container/container.rs

    pub shell_job: bool,
    pub tcp_established: bool,
    pub work_path: Option<PathBuf>,
+    pub manage_cgroup_mode: rust_criu::CgMode,


Overall, I think manage_cgroup_mode should be renamed to manage_cgroups_mode consistently throughout the codebase.

Good catch, I'll fix it.

Signed-off-by: donkomura <koiru3822fs@gmail.com>

saku3

Thanks!

donkomura force-pushed the feat-checkpoint-management-cgroups-mode branch from 4d5a224 to 206dfc1 Compare March 13, 2026 16:33

donkomura mentioned this pull request Mar 14, 2026

Difference between the checkpoint command in runc and youki #3394

Open

nayuta723 added the kind/feature label Mar 14, 2026

utam0k reviewed Mar 14, 2026

View reviewed changes

nayuta723 reviewed Mar 15, 2026

View reviewed changes

donkomura force-pushed the feat-checkpoint-management-cgroups-mode branch from 975e4d6 to d59fa08 Compare March 16, 2026 14:37

donkomura requested a review from nayuta723 March 16, 2026 14:50

nayuta723 suggested changes Mar 17, 2026

View reviewed changes

donkomura force-pushed the feat-checkpoint-management-cgroups-mode branch 3 times, most recently from 14f60d7 to f8d5158 Compare March 17, 2026 23:54

donkomura commented Mar 18, 2026

View reviewed changes

donkomura requested a review from nayuta723 March 18, 2026 14:30

donkomura force-pushed the feat-checkpoint-management-cgroups-mode branch 4 times, most recently from 38588c2 to f00202b Compare March 20, 2026 15:36

nayuta723 reviewed Mar 21, 2026

View reviewed changes

donkomura force-pushed the feat-checkpoint-management-cgroups-mode branch 3 times, most recently from 42a3daa to 4a38926 Compare March 26, 2026 15:23

donkomura requested a review from nayuta723 March 26, 2026 16:32

nayuta723 approved these changes Mar 28, 2026

View reviewed changes

saku3 reviewed Apr 1, 2026

View reviewed changes

donkomura force-pushed the feat-checkpoint-management-cgroups-mode branch 2 times, most recently from 75c73a6 to ba14e65 Compare April 1, 2026 15:07

Support manage-cgroups-mode option for checkpoint

2d23f24

Signed-off-by: donkomura <koiru3822fs@gmail.com>

donkomura force-pushed the feat-checkpoint-management-cgroups-mode branch from ba14e65 to 2d23f24 Compare April 1, 2026 15:21

donkomura requested a review from saku3 April 1, 2026 17:05

saku3 approved these changes Apr 1, 2026

View reviewed changes

saku3 merged commit 58fc350 into youki-dev:main Apr 1, 2026
42 of 43 checks passed

github-actions bot mentioned this pull request Apr 1, 2026

Release for v0.6.1 #3440

Open

	_ => Err(anyhow::anyhow!("manage-cgroup-mode: {s}")),
	_ => Err(anyhow::anyhow!("invalid manage-cgroups-mode: {s}")),

Conversation

donkomura commented Mar 13, 2026

Description

Type of Change

Testing

Related Issues

Additional Context

Uh oh!

utam0k left a comment

Choose a reason for hiding this comment

Uh oh!

donkomura commented Mar 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nayuta723 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nayuta723 Mar 17, 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nayuta723 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nayuta723 Mar 17, 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!

Uh oh!

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!

Uh oh!

donkomura left a comment • 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!

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!

donkomura commented Mar 26, 2026

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!

saku3 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

donkomura commented Mar 14, 2026 •

edited

Loading

nayuta723 Mar 17, 2026 •

edited

Loading

nayuta723 Mar 17, 2026 •

edited

Loading

donkomura left a comment •

edited

Loading