network: fix private sandbox netns

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.affilicated_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());
     }