diff --git a/src/libs/kata-sys-util/src/mount.rs b/src/libs/kata-sys-util/src/mount.rs index febdf23e60..61a80f150a 100644 --- a/src/libs/kata-sys-util/src/mount.rs +++ b/src/libs/kata-sys-util/src/mount.rs @@ -225,7 +225,7 @@ pub fn bind_remount>(dst: P, readonly: bool) -> Result<()> { let dst = dst .canonicalize() .map_err(|_e| Error::InvalidPath(dst.to_path_buf()))?; - + do_rebind_mount(dst, readonly, MsFlags::empty()) } diff --git a/src/runtime-rs/crates/resource/src/share_fs/mod.rs b/src/runtime-rs/crates/resource/src/share_fs/mod.rs index e044495eab..e810373358 100644 --- a/src/runtime-rs/crates/resource/src/share_fs/mod.rs +++ b/src/runtime-rs/crates/resource/src/share_fs/mod.rs @@ -15,7 +15,7 @@ pub use utils::{do_get_guest_path, do_get_guest_share_path, get_host_rw_shared_p mod virtio_fs_share_mount; use virtio_fs_share_mount::VirtiofsShareMount; -use std::{path::PathBuf, sync::Arc}; +use std::{fmt::Debug, path::PathBuf, sync::Arc}; use agent::Storage; use anyhow::{anyhow, Context, Ok, Result}; @@ -38,18 +38,26 @@ pub const PASSTHROUGH_FS_DIR: &str = "passthrough"; const RAFS_DIR: &str = "rafs"; #[async_trait] -pub trait ShareFs: Send + Sync { +pub trait ShareFs: Send + Sync + Debug { fn get_share_fs_mount(&self) -> Arc; async fn setup_device_before_start_vm(&self, h: &dyn Hypervisor) -> Result<()>; async fn setup_device_after_start_vm(&self, h: &dyn Hypervisor) -> Result<()>; async fn get_storages(&self) -> Result>; - /// Get mounted info from ShareFs. + /// Get a mounted info from ShareFs. /// The source is an original path on the host (not in the `/run/kata-containers/...`). async fn get_mounted_info(&self, source: &str) -> Option; - /// Set mounted info to ShareFS. - /// The source is an original path on the host (not in the `/run/kata-containers/...`). + /// Set a mounted info to ShareFS. + /// The source is the same as get_mounted_info's. async fn set_mounted_info(&self, source: &str, mounted_info: MountedInfo) -> Result<()>; + /// Remove a mounted info from ShareFs. + /// The source is the same as get_mounted_info's. + async fn rm_mounted_info(&self, source: &str) -> Result>; + /// Get a mounted info by guest path. + async fn get_mounted_info_by_guest_path( + &self, + guest_path: &str, + ) -> Option<(String, MountedInfo)>; } #[derive(Debug)] @@ -109,7 +117,7 @@ impl MountedInfo { } // File/dir name in the form of "sandbox--" - pub fn name(&self) -> Result { + pub fn file_name(&self) -> Result { match self.guest_path.file_name() { Some(file_name) => match file_name.to_str() { Some(file_name) => Ok(file_name.to_owned()), @@ -124,13 +132,15 @@ impl MountedInfo { } #[async_trait] -pub trait ShareFsMount: Send + Sync { +pub trait ShareFsMount: Send + Sync + Debug { async fn share_rootfs(&self, config: ShareFsRootfsConfig) -> Result; async fn share_volume(&self, config: ShareFsVolumeConfig) -> Result; /// Upgrade to readwrite permission async fn upgrade(&self, file_name: &str) -> Result<()>; /// Downgrade to readonly permission async fn downgrade(&self, file_name: &str) -> Result<()>; + /// Umount the volume + async fn umount(&self, file_name: &str) -> Result<()>; } pub fn new(id: &str, config: &SharedFsInfo) -> Result> { diff --git a/src/runtime-rs/crates/resource/src/share_fs/share_virtio_fs_inline.rs b/src/runtime-rs/crates/resource/src/share_fs/share_virtio_fs_inline.rs index 79a2c8ca2d..51a124a8f5 100644 --- a/src/runtime-rs/crates/resource/src/share_fs/share_virtio_fs_inline.rs +++ b/src/runtime-rs/crates/resource/src/share_fs/share_virtio_fs_inline.rs @@ -30,11 +30,12 @@ pub struct ShareVirtioFsInlineConfig { pub id: String, } -#[derive(Default)] +#[derive(Default, Debug)] pub struct ShareVirtioFsInlineInner { mounted_info_set: HashMap, } +#[derive(Debug)] pub struct ShareVirtioFsInline { config: ShareVirtioFsInlineConfig, share_fs_mount: Arc, @@ -90,7 +91,7 @@ impl ShareFs for ShareVirtioFsInline { async fn get_mounted_info(&self, source: &str) -> Option { let inner = self.inner.read().await; - inner.mounted_info_set.get(source).map(|m| m.clone()) + inner.mounted_info_set.get(source).cloned() } async fn set_mounted_info(&self, source: &str, mounted_info: MountedInfo) -> Result<()> { @@ -100,4 +101,21 @@ impl ShareFs for ShareVirtioFsInline { .insert(source.to_owned(), mounted_info.clone()); Ok(()) } + + async fn rm_mounted_info(&self, source: &str) -> Result> { + let mut inner = self.inner.write().await; + Ok(inner.mounted_info_set.remove(source)) + } + + async fn get_mounted_info_by_guest_path( + &self, + guest_path: &str, + ) -> Option<(String, MountedInfo)> { + let inner = self.inner.read().await; + inner + .mounted_info_set + .iter() + .find(|m| m.1.guest_path.as_os_str().to_str().unwrap() == guest_path) + .map(|m| (m.0.to_owned(), m.1.clone())) + } } diff --git a/src/runtime-rs/crates/resource/src/share_fs/share_virtio_fs_standalone.rs b/src/runtime-rs/crates/resource/src/share_fs/share_virtio_fs_standalone.rs index d3992ac834..05165fc04b 100644 --- a/src/runtime-rs/crates/resource/src/share_fs/share_virtio_fs_standalone.rs +++ b/src/runtime-rs/crates/resource/src/share_fs/share_virtio_fs_standalone.rs @@ -38,11 +38,13 @@ pub struct ShareVirtioFsStandaloneConfig { pub virtio_fs_extra_args: Vec, } -#[derive(Default)] +#[derive(Default, Debug)] struct ShareVirtioFsStandaloneInner { pid: Option, mounted_info_set: HashMap, } + +#[derive(Debug)] pub(crate) struct ShareVirtioFsStandalone { inner: Arc>, config: ShareVirtioFsStandaloneConfig, @@ -176,7 +178,7 @@ impl ShareFs for ShareVirtioFsStandalone { async fn get_mounted_info(&self, source: &str) -> Option { let inner = self.inner.read().await; - inner.mounted_info_set.get(source).map(|m| m.clone()) + inner.mounted_info_set.get(source).cloned() } async fn set_mounted_info(&self, source: &str, mounted_info: MountedInfo) -> Result<()> { @@ -186,4 +188,21 @@ impl ShareFs for ShareVirtioFsStandalone { .insert(source.to_owned(), mounted_info.clone()); Ok(()) } + + async fn rm_mounted_info(&self, source: &str) -> Result> { + let mut inner = self.inner.write().await; + Ok(inner.mounted_info_set.remove(source)) + } + + async fn get_mounted_info_by_guest_path( + &self, + guest_path: &str, + ) -> Option<(String, MountedInfo)> { + let inner = self.inner.read().await; + inner + .mounted_info_set + .iter() + .find(|m| m.1.guest_path.as_os_str().to_str().unwrap() == guest_path) + .map(|m| (m.0.to_owned(), m.1.clone())) + } } diff --git a/src/runtime-rs/crates/resource/src/share_fs/utils.rs b/src/runtime-rs/crates/resource/src/share_fs/utils.rs index 4109a0a71a..115bdd1463 100644 --- a/src/runtime-rs/crates/resource/src/share_fs/utils.rs +++ b/src/runtime-rs/crates/resource/src/share_fs/utils.rs @@ -115,17 +115,3 @@ pub(crate) fn do_get_host_path( }; path.to_str().unwrap().to_string() } - -// /// Get the bind mounted path on the host that will be shared to the guest in -// /// **sandbox level**. -// /// The filename is in format of "sandbox-{uuid}-examplename". -// pub(crate) fn do_get_sandbox_level_host_path(sid: &str, filename: &str, readonly: bool) -> String { -// do_get_host_path(filename, sid, "", true, readonly) -// } - -// /// Get the bind mounted path on the guest that will be shared from the host in -// /// **sandbox level**. -// /// The filename is in format of "sandbox-{uuid}-examplename". -// pub(crate) fn do_get_sandbox_level_guest_path(filename: &str) -> String { -// do_get_guest_any_path(filename, "", true, false) -// } diff --git a/src/runtime-rs/crates/resource/src/share_fs/virtio_fs_share_mount.rs b/src/runtime-rs/crates/resource/src/share_fs/virtio_fs_share_mount.rs index 923be39156..9f9778eeb2 100644 --- a/src/runtime-rs/crates/resource/src/share_fs/virtio_fs_share_mount.rs +++ b/src/runtime-rs/crates/resource/src/share_fs/virtio_fs_share_mount.rs @@ -7,7 +7,7 @@ use agent::Storage; use anyhow::{anyhow, Context, Result}; use async_trait::async_trait; -use kata_sys_util::mount::bind_remount; +use kata_sys_util::mount::{bind_remount, umount_timeout}; use kata_types::k8s::is_watchable_mount; use kata_types::mount; use nix::sys::stat::stat; @@ -25,6 +25,7 @@ use super::{ KATA_GUEST_SHARE_DIR, PASSTHROUGH_FS_DIR, }; +#[derive(Debug)] pub struct VirtiofsShareMount { id: String, } @@ -192,4 +193,12 @@ impl ShareFsMount for VirtiofsShareMount { .context("remount readonly directory with readonly permission")?; Ok(()) } + + async fn umount(&self, file_name: &str) -> Result<()> { + let host_dest = do_get_host_path(file_name, &self.id, "", true, false); + umount_timeout(&host_dest, 0).context("Umount readonly host dest")?; + let host_dest = do_get_host_path(file_name, &self.id, "", true, true); + umount_timeout(&host_dest, 0).context("Umount readwrite host dest")?; + Ok(()) + } } diff --git a/src/runtime-rs/crates/resource/src/volume/block_volume.rs b/src/runtime-rs/crates/resource/src/volume/block_volume.rs index f015c92785..67d0bc7af8 100644 --- a/src/runtime-rs/crates/resource/src/volume/block_volume.rs +++ b/src/runtime-rs/crates/resource/src/volume/block_volume.rs @@ -5,9 +5,11 @@ // use anyhow::Result; +use async_trait::async_trait; use super::Volume; +#[derive(Debug)] pub(crate) struct BlockVolume {} /// BlockVolume: block device volume @@ -17,6 +19,7 @@ impl BlockVolume { } } +#[async_trait] impl Volume for BlockVolume { fn get_volume_mount(&self) -> anyhow::Result> { todo!() @@ -26,8 +29,9 @@ impl Volume for BlockVolume { todo!() } - fn cleanup(&self) -> Result<()> { - todo!() + async fn cleanup(&self) -> Result<()> { + warn!(sl!(), "Cleaning up BlockVolume is still unimplemented."); + Ok(()) } } diff --git a/src/runtime-rs/crates/resource/src/volume/default_volume.rs b/src/runtime-rs/crates/resource/src/volume/default_volume.rs index 3b7752a4e7..bc14ba959f 100644 --- a/src/runtime-rs/crates/resource/src/volume/default_volume.rs +++ b/src/runtime-rs/crates/resource/src/volume/default_volume.rs @@ -5,9 +5,11 @@ // use anyhow::Result; +use async_trait::async_trait; use super::Volume; +#[derive(Debug)] pub(crate) struct DefaultVolume { mount: oci::Mount, } @@ -21,6 +23,7 @@ impl DefaultVolume { } } +#[async_trait] impl Volume for DefaultVolume { fn get_volume_mount(&self) -> anyhow::Result> { Ok(vec![self.mount.clone()]) @@ -30,7 +33,8 @@ impl Volume for DefaultVolume { Ok(vec![]) } - fn cleanup(&self) -> Result<()> { - todo!() + async fn cleanup(&self) -> Result<()> { + warn!(sl!(), "Cleaning up DefaultVolume is still unimplemented."); + Ok(()) } } diff --git a/src/runtime-rs/crates/resource/src/volume/mod.rs b/src/runtime-rs/crates/resource/src/volume/mod.rs index 53c737c79c..d5d85c46f5 100644 --- a/src/runtime-rs/crates/resource/src/volume/mod.rs +++ b/src/runtime-rs/crates/resource/src/volume/mod.rs @@ -8,6 +8,7 @@ mod block_volume; mod default_volume; mod share_fs_volume; mod shm_volume; +use async_trait::async_trait; use std::{sync::Arc, vec::Vec}; @@ -16,10 +17,11 @@ use tokio::sync::RwLock; use crate::share_fs::ShareFs; -pub trait Volume: Send + Sync { +#[async_trait] +pub trait Volume: Send + Sync + std::fmt::Debug { fn get_volume_mount(&self) -> Result>; fn get_storage(&self) -> Result>; - fn cleanup(&self) -> Result<()>; + async fn cleanup(&self) -> Result<()>; } #[derive(Default)] diff --git a/src/runtime-rs/crates/resource/src/volume/share_fs_volume.rs b/src/runtime-rs/crates/resource/src/volume/share_fs_volume.rs index 5b768c5dac..5d5f0e4bd6 100644 --- a/src/runtime-rs/crates/resource/src/volume/share_fs_volume.rs +++ b/src/runtime-rs/crates/resource/src/volume/share_fs_volume.rs @@ -7,10 +7,11 @@ use std::{ path::{Path, PathBuf}, str::FromStr, - sync::Arc, + sync::{Arc, Weak}, }; use anyhow::{anyhow, Context, Result}; +use async_trait::async_trait; use super::Volume; use crate::share_fs::{MountedInfo, ShareFs, ShareFsVolumeConfig}; @@ -22,7 +23,9 @@ use kata_types::mount; // only regular files in /dev. It does not make sense to pass the host // device nodes to the guest. // skip the volumes whose source had already set to guest share dir. +#[derive(Debug)] pub(crate) struct ShareFsVolume { + share_fs: Option>, mounts: Vec, storages: Vec, } @@ -38,7 +41,7 @@ impl ShareFsVolume { let file_name = generate_mount_path("sandbox", file_name); let mut volume = Self { - // share_fs: share_fs.as_ref().map(Arc::clone), + share_fs: share_fs.as_ref().map(Arc::downgrade), mounts: vec![], storages: vec![], }; @@ -84,7 +87,11 @@ impl ShareFsVolume { "The mount will be upgraded, mount = {:?}, cid = {}", m, cid ); share_fs_mount - .upgrade(&mounted_info.name().context("get name of mounted info")?) + .upgrade( + &mounted_info + .file_name() + .context("get name of mounted info")?, + ) .await .context("upgrade mount")?; } @@ -145,6 +152,7 @@ impl ShareFsVolume { } } +#[async_trait] impl Volume for ShareFsVolume { fn get_volume_mount(&self) -> anyhow::Result> { Ok(self.mounts.clone()) @@ -154,8 +162,76 @@ impl Volume for ShareFsVolume { Ok(self.storages.clone()) } - fn cleanup(&self) -> Result<()> { - todo!() + async fn cleanup(&self) -> Result<()> { + if self.share_fs.is_none() { + return Ok(()); + } + let share_fs = match self.share_fs.as_ref().unwrap().upgrade() { + Some(share_fs) => share_fs, + None => return Err(anyhow!("The share_fs was released unexpectedly")), + }; + + for m in self.mounts.iter() { + let (host_source, mut mounted_info) = + match share_fs.get_mounted_info_by_guest_path(&m.source).await { + Some(entry) => entry, + None => { + warn!( + sl!(), + "The mounted info for guest path {} not found", m.source + ); + continue; + } + }; + + let old_readonly = mounted_info.readonly(); + + if m.options.iter().any(|opt| *opt == "ro") { + mounted_info.ro_ref_count -= 1; + } else { + mounted_info.rw_ref_count -= 1; + } + + debug!( + sl!(), + "Ref count for {} was updated to {} due to volume cleanup", + host_source, + mounted_info.ref_count() + ); + let share_fs_mount = share_fs.get_share_fs_mount(); + let file_name = mounted_info.file_name()?; + + if mounted_info.ref_count() > 0 { + // Downgrade to readonly if no container needs readwrite permission + if !old_readonly && mounted_info.readonly() { + info!(sl!(), "Downgrade {} to readonly due to no container that needs readwrite permission", host_source); + share_fs_mount + .downgrade(&file_name) + .await + .context("Downgrade volume")?; + } + share_fs + .set_mounted_info(&host_source, mounted_info) + .await + .context("Update mounted info")?; + } else { + info!( + sl!(), + "The path will be umounted due to no references, host_source = {}", host_source + ); + share_fs + .rm_mounted_info(&host_source) + .await + .context("Rm mounted info due to no reference")?; + // Umount the volume + share_fs_mount + .umount(&file_name) + .await + .context("Umount volume")? + } + } + + Ok(()) } } diff --git a/src/runtime-rs/crates/resource/src/volume/shm_volume.rs b/src/runtime-rs/crates/resource/src/volume/shm_volume.rs index c1c9df993f..53f3addf81 100644 --- a/src/runtime-rs/crates/resource/src/volume/shm_volume.rs +++ b/src/runtime-rs/crates/resource/src/volume/shm_volume.rs @@ -7,6 +7,7 @@ use std::path::Path; use anyhow::Result; +use async_trait::async_trait; use super::Volume; use crate::share_fs::DEFAULT_KATA_GUEST_SANDBOX_DIR; @@ -19,6 +20,7 @@ pub const DEFAULT_SHM_SIZE: u64 = 65536 * 1024; // KATA_EPHEMERAL_DEV_TYPE creates a tmpfs backed volume for sharing files between containers. pub const KATA_EPHEMERAL_DEV_TYPE: &str = "ephemeral"; +#[derive(Debug)] pub(crate) struct ShmVolume { mount: oci::Mount, storage: Option, @@ -82,6 +84,7 @@ impl ShmVolume { } } +#[async_trait] impl Volume for ShmVolume { fn get_volume_mount(&self) -> anyhow::Result> { Ok(vec![self.mount.clone()]) @@ -96,8 +99,9 @@ impl Volume for ShmVolume { Ok(s) } - fn cleanup(&self) -> Result<()> { - todo!() + async fn cleanup(&self) -> Result<()> { + warn!(sl!(), "Cleaning up ShmVolume is still unimplemented."); + Ok(()) } } diff --git a/src/runtime-rs/crates/runtimes/virt_container/src/container_manager/container.rs b/src/runtime-rs/crates/runtimes/virt_container/src/container_manager/container.rs index 2e2a025f07..e947d2068c 100644 --- a/src/runtime-rs/crates/runtimes/virt_container/src/container_manager/container.rs +++ b/src/runtime-rs/crates/runtimes/virt_container/src/container_manager/container.rs @@ -258,7 +258,7 @@ impl Container { signal: u32, all: bool, ) -> Result<()> { - let inner = self.inner.read().await; + let mut inner = self.inner.write().await; inner.signal_process(container_process, signal, all).await } diff --git a/src/runtime-rs/crates/runtimes/virt_container/src/container_manager/container_inner.rs b/src/runtime-rs/crates/runtimes/virt_container/src/container_manager/container_inner.rs index 88f7d7ab71..0c7ac64340 100644 --- a/src/runtime-rs/crates/runtimes/virt_container/src/container_manager/container_inner.rs +++ b/src/runtime-rs/crates/runtimes/virt_container/src/container_manager/container_inner.rs @@ -180,8 +180,6 @@ impl ContainerInner { } })?; - // TODO(justxuewei): clean mount - // close the exit channel to wakeup wait service // send to notify watchers who are waiting for the process exit self.init_process.stop().await; @@ -235,7 +233,7 @@ impl ContainerInner { } pub(crate) async fn signal_process( - &self, + &mut self, process: &ContainerProcess, signal: u32, all: bool, @@ -249,6 +247,9 @@ impl ContainerInner { self.agent .signal_process(agent::SignalProcessRequest { process_id, signal }) .await?; + + self.clean_volumes().await.context("clean volumes")?; + Ok(()) } @@ -270,4 +271,18 @@ impl ContainerInner { Ok(()) } + + async fn clean_volumes(&mut self) -> Result<()> { + let mut unhandled = Vec::new(); + for v in self.volumes.iter() { + if let Err(err) = v.cleanup().await { + unhandled.push(Arc::clone(v)); + warn!(sl!(), "Failed to clean volume {:?}, error = {:?}", v, err); + } + } + if !unhandled.is_empty() { + self.volumes = unhandled; + } + Ok(()) + } }