Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --detach -d option #628

Closed
adrianreber opened this issue Jan 21, 2022 · 2 comments · Fixed by #1656
Closed

Add --detach -d option #628

adrianreber opened this issue Jan 21, 2022 · 2 comments · Fixed by #1656
Assignees

Comments

@adrianreber
Copy link
Contributor

To implement checkpoint/restore containers need to be started with --detach. This is currently missing.

The important part for checkpoint/restore is that --detach does setsid(). Hardcoding setsid() already helped me to be able to checkpoint a container.

diff --git a/crates/libcontainer/src/process/container_intermediate_process.rs b/crates/libcontainer/src/process/container_intermediate_process.rs
index b2a5e0d..a9e1f00 100644
--- a/crates/libcontainer/src/process/container_intermediate_process.rs
+++ b/crates/libcontainer/src/process/container_intermediate_process.rs
@@ -95,6 +95,8 @@ pub fn container_intermediate_process(
         intermediate_sender
             .close()
             .context("failed to close sender in the intermediate process")?;
+        nix::unistd::setsid()?;
+
         container_init_process(args, main_sender, init_receiver)
     })?;
     // Once we fork the container init process, the job for intermediate process

I am not sure that is the right location, but that works for me. Having it done correctly in --detach is probably much better than what I have done.

@Furisto
Copy link
Collaborator

Furisto commented Jan 22, 2022

We already call setsid(), but only if we allocate a pseudo terminal. Just looking through the runc code i see that they always seem to call setsid regardless of whether --detach is specified or not. Crun does the same.

@yihuaf yihuaf self-assigned this Mar 8, 2023
@yihuaf
Copy link
Collaborator

yihuaf commented Mar 8, 2023

I will take care of the detached and foreground mode. Although the core of this issue seems to be unrelated to detached anymore lol.

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 a pull request may close this issue.

3 participants