sandbox: Optimize sandbox lifecycle management

1. Add many roll back handlers in the process of starting sandbox.
2. Move setup network to Start sandbox as it should be called near the vm startup.
3. Only monit running sandbox and dump it status in time.
4. Sandbox stop should make sure the sandbox is stopped successfully, so wait it
to stop for 10s.
5. If the VM process terminated suddenly and the containerd has no idea to stop
sandbox, destroy network should be done in monit thread.
6. Forcefully kill vmm process in stopping sandbox.

Signed-off-by: Zhang Tianyang <[email protected]>

Author: Burning1020

vmm/sandbox/src/cloud_hypervisor/mod.rs

@@ -14,12 +14,13 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-use std::{os::unix::io::RawFd, process::Stdio};
+use std::{os::unix::io::RawFd, process::Stdio, time::Duration};
 
 use anyhow::anyhow;
 use async_trait::async_trait;
 use containerd_sandbox::error::{Error, Result};
-use log::{debug, error, info};
+use log::{debug, error, info, warn};
+use nix::{errno::Errno::ESRCH, sys::signal, unistd::Pid};
 use serde::{Deserialize, Serialize};
 use time::OffsetDateTime;
 use tokio::{
@@ -41,10 +42,10 @@ use crate::{
         },
     },
     device::{BusType, DeviceInfo},
-    impl_recoverable, load_config,
+    load_config,
     param::ToCmdLineParams,
     sandbox::KuasarSandboxer,
-    utils::{read_std, set_cmd_fd, set_cmd_netns, wait_pid, write_file_atomic},
+    utils::{read_std, set_cmd_fd, set_cmd_netns, wait_channel, wait_pid, write_file_atomic},
     vm::{Pids, VcpuThreads, VM},
 };
 
@@ -145,13 +146,25 @@ impl CloudHypervisorVM {
         self.fds.push(fd);
         self.fds.len() - 1 + 3
     }
+
+    async fn wait_stop(&mut self, t: Duration) -> Result<()> {
+        if let Some(rx) = self.wait_channel().await {
+            let (_, ts) = *rx.borrow();
+            if ts == 0 {
+                wait_channel(t, rx).await?;
+            }
+        }
+        Ok(())
+    }
 }
 
 #[async_trait]
 impl VM for CloudHypervisorVM {
     async fn start(&mut self) -> Result<u32> {
         create_dir_all(&self.base_dir).await?;
         let virtiofsd_pid = self.start_virtiofsd().await?;
+        // TODO: add child virtiofsd process
+        self.pids.affiliated_pids.push(virtiofsd_pid);
         let mut params = self.config.to_cmdline_params("--");
         for d in self.devices.iter() {
             params.extend(d.to_cmdline_params("--"));
@@ -174,31 +187,57 @@ impl VM for CloudHypervisorVM {
             .spawn()
             .map_err(|e| anyhow!("failed to spawn cloud hypervisor command: {}", e))?;
         let pid = child.id();
+        self.pids.vmm_pid = pid;
         let pid_file = format!("{}/pid", self.base_dir);
-        let (tx, rx) = tokio::sync::watch::channel((0u32, 0i128));
+        let (tx, rx) = channel((0u32, 0i128));
+        self.wait_chan = Some(rx);
         spawn_wait(
             child,
             format!("cloud-hypervisor {}", self.id),
             Some(pid_file),
             Some(tx),
         );
-        self.client = Some(self.create_client().await?);
-        self.wait_chan = Some(rx);
 
-        // update vmm related pids
-        self.pids.vmm_pid = pid;
-        self.pids.affiliated_pids.push(virtiofsd_pid);
-        // TODO: add child virtiofsd process
+        match self.create_client().await {
+            Ok(client) => self.client = Some(client),
+            Err(e) => {
+                if let Err(re) = self.stop(true).await {
+                    warn!("roll back in create clh api client: {}", re);
+                    return Err(e);
+                }
+                return Err(e);
+            }
+        };
         Ok(pid.unwrap_or_default())
     }
 
     async fn stop(&mut self, force: bool) -> Result<()> {
-        let pid = self.pid()?;
-        if pid == 0 {
-            return Ok(());
+        let signal = if force {
+            signal::SIGKILL
+        } else {
+            signal::SIGTERM
+        };
+
+        let pids = self.pids();
+        if let Some(vmm_pid) = pids.vmm_pid {
+            if vmm_pid > 0 {
+                // TODO: Consider pid reused
+                match signal::kill(Pid::from_raw(vmm_pid as i32), signal) {
+                    Err(e) => {
+                        if e != ESRCH {
+                            return Err(anyhow!("kill vmm process {}: {}", vmm_pid, e).into());
+                        }
+                    }
+                    Ok(_) => self.wait_stop(Duration::from_secs(10)).await?,
+                }
+            }
+        }
+        for affiliated_pid in pids.affiliated_pids {
+            if affiliated_pid > 0 {
+                // affiliated process may exits automatically, so it's ok not handle error
+                signal::kill(Pid::from_raw(affiliated_pid as i32), signal).unwrap_or_default();
+            }
         }
-        let signal = if force { 9 } else { 15 };
-        unsafe { nix::libc::kill(pid as i32, signal) };
 
         Ok(())
     }
@@ -288,7 +327,20 @@ impl VM for CloudHypervisorVM {
     }
 }
 
-impl_recoverable!(CloudHypervisorVM);
+#[async_trait]
+impl crate::vm::Recoverable for CloudHypervisorVM {
+    async fn recover(&mut self) -> Result<()> {
+        self.client = Some(self.create_client().await?);
+        let pid = self.pid()?;
+        let (tx, rx) = channel((0u32, 0i128));
+        tokio::spawn(async move {
+            let wait_result = wait_pid(pid as i32).await;
+            tx.send(wait_result).unwrap_or_default();
+        });
+        self.wait_chan = Some(rx);
+        Ok(())
+    }
+}
 
 macro_rules! read_stdio {
     ($stdio:expr, $cmd_name:ident) => {

vmm/sandbox/src/network/link.rs

@@ -574,6 +574,11 @@ async fn get_pci_driver(bdf: &str) -> Result<String> {
 }
 
 async fn bind_device_to_driver(driver: &str, bdf: &str) -> Result<()> {
+    // 0. Check the current driver
+    if get_pci_driver(bdf).await? == driver {
+        return Ok(());
+    }
+
     // 1. Switch the device driver
     let driver_override_path = format!("/sys/bus/pci/devices/{}/driver_override", bdf);
     write_file_async(&driver_override_path, driver).await?;
@@ -589,23 +594,13 @@ async fn bind_device_to_driver(driver: &str, bdf: &str) -> Result<()> {
     write_file_async(probe_path, bdf).await?;
 
     // 4. Check the result
-    let driver_link = format!("/sys/bus/pci/devices/{}/driver", bdf);
-    let driver_path = tokio::fs::read_link(&*driver_link).await?;
-
-    let result_driver = driver_path.file_name().ok_or(anyhow!(
-        "failed to get driver name from {}",
-        driver_path.display()
-    ))?;
-    let result_driver = result_driver.to_str().ok_or(anyhow!(
-        "failed to convert the driver {} to str",
-        result_driver.to_string_lossy()
-    ))?;
+    let result_driver = get_pci_driver(bdf).await?;
     if result_driver != driver {
         return Err(anyhow!(
-            "device {} driver is {} after executing bind to {}",
+            "device {} driver is expected to {} but got to {}",
             bdf,
-            result_driver,
-            driver
+            driver,
+            result_driver
         )
         .into());
     }