Merge pull request #112 from YJDoc2/exec-test-2

Alternative approach to solving the exec issue

crates/libcontainer/src/container/builder_impl.rs

@@ -122,7 +122,8 @@ impl<'a> ContainerBuilderImpl<'a> {
             cgroup_manager: cmanager,
         };
 
-        let init_pid = process::container_main_process::container_main_process(&container_args)?;
+        let (intermediate, init_pid) =
+            process::container_main_process::container_main_process(&container_args, !self.init)?;
 
         // if file to write the pid to is specified, write pid of the child
         if let Some(pid_file) = &self.pid_file {
@@ -139,7 +140,7 @@ impl<'a> ContainerBuilderImpl<'a> {
                 .context("Failed to save container state")?;
         }
 
-        Ok(init_pid)
+        Ok(intermediate)
     }
 
     fn cleanup_container(&self) -> Result<()> {

crates/libcontainer/src/process/container_init_process.rs

@@ -15,10 +15,7 @@ use nix::unistd::setsid;
 use nix::unistd::{self, Gid, Uid};
 use oci_spec::runtime::{LinuxNamespaceType, Spec, User};
 use std::collections::HashMap;
-use std::fs::File;
-use std::io::Write;
 use std::os::unix::io::AsRawFd;
-use std::os::unix::prelude::FromRawFd;
 use std::{
     env, fs,
     path::{Path, PathBuf},
@@ -162,7 +159,6 @@ pub fn container_init_process(
     args: &ContainerArgs,
     main_sender: &mut channel::MainSender,
     init_receiver: &mut channel::InitReceiver,
-    fifo_fd: i32,
 ) -> Result<()> {
     let syscall = args.syscall;
     let spec = args.spec;
@@ -417,12 +413,7 @@ pub fn container_init_process(
 
     if proc.args().is_some() {
         ExecutorManager::exec(spec)?;
-        if fifo_fd != 0 {
-            let f = &mut unsafe { File::from_raw_fd(fifo_fd) };
-            // TODO: impl
-            write!(f, "1")?;
-        }
-        Ok(())
+        unreachable!("process image should have been replaced after exec");
     } else {
         bail!("on non-Windows, at least one process arg entry is required")
     }

crates/libcontainer/src/process/container_intermediate_process.rs

@@ -1,6 +1,7 @@
 use crate::{namespaces::Namespaces, process::channel, process::fork};
 use anyhow::{Context, Error, Result};
 use libcgroups::common::CgroupManager;
+use nix::sys::wait::{waitpid, WaitStatus};
 use nix::unistd::{Gid, Pid, Uid};
 use oci_spec::runtime::{LinuxNamespaceType, LinuxResources};
 use procfs::process::Process;
@@ -14,8 +15,8 @@ pub fn container_intermediate_process(
     intermediate_chan: &mut (channel::IntermediateSender, channel::IntermediateReceiver),
     init_chan: &mut (channel::InitSender, channel::InitReceiver),
     main_sender: &mut channel::MainSender,
-    fifo_fd: i32,
-) -> Result<()> {
+    wait: bool,
+) -> Result<WaitStatus> {
     let (inter_sender, inter_receiver) = intermediate_chan;
     let (init_sender, init_receiver) = init_chan;
     let command = &args.syscall;
@@ -96,7 +97,8 @@ pub fn container_intermediate_process(
         inter_sender
             .close()
             .context("failed to close sender in the intermediate process")?;
-        container_init_process(args, main_sender, init_receiver, fifo_fd)
+        container_init_process(args, main_sender, init_receiver)?;
+        Ok(0)
     })?;
     // Once we fork the container init process, the job for intermediate process
     // is done. We notify the container main process about the pid we just
@@ -116,7 +118,13 @@ pub fn container_intermediate_process(
         .close()
         .context("failed to close unused init sender")?;
 
-    Ok(())
+    if wait {
+        Ok(waitpid(pid, None)?)
+    } else {
+        // we don't actually want to wait, so
+        // pid and status doesn't really matter
+        Ok(WaitStatus::Exited(Pid::from_raw(0), 0))
+    }
 }
 
 fn apply_cgroups<C: CgroupManager + ?Sized>(

crates/libcontainer/src/process/container_main_process.rs

@@ -8,14 +8,14 @@ use anyhow::{Context, Result};
 use nix::{
     sys::{
         socket::{self, UnixAddr},
-        stat,
+        wait::WaitStatus,
     },
-    unistd::{self, mkfifo, Pid},
+    unistd::{self, Pid},
 };
 use oci_spec::runtime;
 use std::{io::IoSlice, path::Path};
 
-pub fn container_main_process(container_args: &ContainerArgs) -> Result<Pid> {
+pub fn container_main_process(container_args: &ContainerArgs, wait: bool) -> Result<(Pid, Pid)> {
     // We use a set of channels to communicate between parent and child process.
     // Each channel is uni-directional. Because we will pass these channel to
     // forked process, we have to be deligent about closing any unused channel.
@@ -25,33 +25,19 @@ pub fn container_main_process(container_args: &ContainerArgs) -> Result<Pid> {
     let inter_chan = &mut channel::intermediate_channel()?;
     let init_chan = &mut channel::init_channel()?;
 
-    // TODO: implement Option version
-    let mut fifo_fd = 0;
-    // let container_root = &container_args
-    //     .container
-    //     .as_ref()
-    //     .context("container state is required")?
-    //     .root;
-    let container_root = &std::path::Path::new("/run/youki/tutorial_container/");
-    let fifo_path = container_root.join("state.fifo");
-    if container_args.init {
-        mkfifo(&fifo_path, stat::Mode::S_IRWXU).context("failed to create the fifo file.")?;
-    }
-
-    let mut open_flags = nix::fcntl::OFlag::empty();
-    open_flags.insert(nix::fcntl::OFlag::O_PATH);
-    open_flags.insert(nix::fcntl::OFlag::O_CLOEXEC);
-    fifo_fd = nix::fcntl::open(&fifo_path, open_flags, stat::Mode::S_IRWXU)?;
-    log::debug!("fifo_fd: {}", fifo_fd);
-
     let intermediate_pid = fork::container_fork(|| {
-        container_intermediate_process::container_intermediate_process(
+        let t = container_intermediate_process::container_intermediate_process(
             container_args,
             inter_chan,
             init_chan,
             main_sender,
-            fifo_fd,
-        )
+            wait,
+        )?;
+        match t {
+            WaitStatus::Exited(_, s) => Ok(s),
+            WaitStatus::Signaled(_, sig, _) => Ok(sig as i32),
+            _ => Ok(0),
+        }
     })?;
     // Close down unused fds. The corresponding fds are duplicated to the
     // child process during fork.
@@ -113,7 +99,7 @@ pub fn container_main_process(container_args: &ContainerArgs) -> Result<Pid> {
 
     log::debug!("init pid is {:?}", init_pid);
 
-    Ok(init_pid)
+    Ok((intermediate_pid, init_pid))
 }
 
 fn sync_seccomp(

crates/libcontainer/src/process/fork.rs

@@ -8,15 +8,16 @@ use nix::unistd::Pid;
 // using clone, we would have to manually make sure all the variables are
 // correctly send to the new process, especially Rust borrow checker will be a
 // lot of hassel to deal with every details.
-pub fn container_fork<F: FnOnce() -> Result<()>>(cb: F) -> Result<Pid> {
+pub fn container_fork<F: FnOnce() -> Result<i32>>(cb: F) -> Result<Pid> {
     match unsafe { unistd::fork()? } {
         unistd::ForkResult::Parent { child } => Ok(child),
         unistd::ForkResult::Child => {
-            let ret = if let Err(error) = cb() {
-                log::debug!("failed to run fork: {:?}", error);
-                -1
-            } else {
-                0
+            let ret = match cb() {
+                Err(error) => {
+                    log::debug!("failed to run fork: {:?}", error);
+                    -1
+                }
+                Ok(ec) => ec,
             };
             std::process::exit(ret);
         }
@@ -31,7 +32,7 @@ mod test {
 
     #[test]
     fn test_container_fork() -> Result<()> {
-        let pid = container_fork(|| Ok(()))?;
+        let pid = container_fork(|| Ok(0))?;
         match waitpid(pid, None).expect("wait pid failed.") {
             WaitStatus::Exited(p, status) => {
                 assert_eq!(pid, p);

crates/youki/src/commands/exec.rs

@@ -1,17 +1,11 @@
-use anyhow::{bail, Context, Result};
-use nix::{
-    libc,
-    poll::{PollFd, PollFlags},
-};
-use std::{fs::OpenOptions, io::Read, os::unix::prelude::RawFd, path::PathBuf};
+use anyhow::Result;
+use nix::sys::wait::{waitpid, WaitStatus};
+use std::path::PathBuf;
 
 use libcontainer::{container::builder::ContainerBuilder, syscall::syscall::create_syscall};
 use liboci_cli::Exec;
 
-use super::load_container;
-
 pub fn exec(args: Exec, root_path: PathBuf) -> Result<i32> {
-    let container = load_container(&root_path, &args.container_id)?;
     let syscall = create_syscall();
     let pid = ContainerBuilder::new(args.container_id.clone(), syscall.as_ref())
         .with_root_path(root_path)?
@@ -25,26 +19,9 @@ pub fn exec(args: Exec, root_path: PathBuf) -> Result<i32> {
         .with_container_args(args.command.clone())
         .build()?;
 
-    let pidfd = pidfd_open(pid.as_raw(), 0)?;
-    let poll_fd = PollFd::new(pidfd, PollFlags::POLLIN);
-    nix::poll::poll(&mut [poll_fd], -1).context("failed to wait for the container id")?;
-
-    let fifo_path = &container.root.join("state.fifo");
-    println!("fifo_path: {:?}", fifo_path);
-    let mut f = OpenOptions::new().read(true).open(fifo_path)?;
-    let mut contents = String::new();
-    f.read_to_string(&mut contents)?;
-    println!("get the value: {:?}", contents);
-
-    // TODO
-    Ok(0)
-}
-
-fn pidfd_open(pid: libc::pid_t, flags: libc::c_uint) -> Result<RawFd> {
-    let fd = unsafe { libc::syscall(libc::SYS_pidfd_open, pid, flags) };
-    if fd == -1 {
-        bail!("faild to pifd_open syscall")
-    } else {
-        Ok(fd as RawFd)
+    match waitpid(pid, None)? {
+        WaitStatus::Exited(_, status) => Ok(status),
+        WaitStatus::Signaled(_, sig, _) => Ok(sig as i32),
+        _ => Ok(0),
     }
 }