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 e810373358..96f6dc32f1 100644 --- a/src/runtime-rs/crates/resource/src/share_fs/mod.rs +++ b/src/runtime-rs/crates/resource/src/share_fs/mod.rs @@ -11,11 +11,12 @@ use share_virtio_fs_inline::ShareVirtioFsInline; mod share_virtio_fs_standalone; use share_virtio_fs_standalone::ShareVirtioFsStandalone; mod utils; +use tokio::sync::Mutex; pub use utils::{do_get_guest_path, do_get_guest_share_path, get_host_rw_shared_path}; mod virtio_fs_share_mount; use virtio_fs_share_mount::VirtiofsShareMount; -use std::{fmt::Debug, path::PathBuf, sync::Arc}; +use std::{collections::HashMap, fmt::Debug, path::PathBuf, sync::Arc}; use agent::Storage; use anyhow::{anyhow, Context, Ok, Result}; @@ -38,26 +39,12 @@ pub const PASSTHROUGH_FS_DIR: &str = "passthrough"; const RAFS_DIR: &str = "rafs"; #[async_trait] -pub trait ShareFs: Send + Sync + Debug { +pub trait ShareFs: Send + Sync { 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 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 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)>; + fn mounted_info_set(&self) -> Arc>>; } #[derive(Debug)] @@ -132,13 +119,13 @@ impl MountedInfo { } #[async_trait] -pub trait ShareFsMount: Send + Sync + Debug { +pub trait ShareFsMount: Send + Sync { 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<()>; + async fn upgrade_to_rw(&self, file_name: &str) -> Result<()>; /// Downgrade to readonly permission - async fn downgrade(&self, file_name: &str) -> Result<()>; + async fn downgrade_to_ro(&self, file_name: &str) -> Result<()>; /// Umount the volume async fn umount(&self, file_name: &str) -> 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 51a124a8f5..5dddefbfdd 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 @@ -11,7 +11,7 @@ use anyhow::{Context, Result}; use async_trait::async_trait; use hypervisor::Hypervisor; use kata_types::config::hypervisor::SharedFsInfo; -use tokio::sync::RwLock; +use tokio::sync::Mutex; use super::{ share_virtio_fs::{ @@ -30,16 +30,10 @@ pub struct ShareVirtioFsInlineConfig { pub id: String, } -#[derive(Default, Debug)] -pub struct ShareVirtioFsInlineInner { - mounted_info_set: HashMap, -} - -#[derive(Debug)] pub struct ShareVirtioFsInline { config: ShareVirtioFsInlineConfig, share_fs_mount: Arc, - inner: Arc>, + mounted_info_set: Arc>>, } impl ShareVirtioFsInline { @@ -47,7 +41,7 @@ impl ShareVirtioFsInline { Ok(Self { config: ShareVirtioFsInlineConfig { id: id.to_string() }, share_fs_mount: Arc::new(VirtiofsShareMount::new(id)), - inner: Arc::new(RwLock::new(ShareVirtioFsInlineInner::default())), + mounted_info_set: Arc::new(Mutex::new(HashMap::new())), }) } } @@ -89,33 +83,7 @@ impl ShareFs for ShareVirtioFsInline { Ok(storages) } - async fn get_mounted_info(&self, source: &str) -> Option { - let inner = self.inner.read().await; - inner.mounted_info_set.get(source).cloned() - } - - async fn set_mounted_info(&self, source: &str, mounted_info: MountedInfo) -> Result<()> { - let mut inner = self.inner.write().await; - inner - .mounted_info_set - .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())) + fn mounted_info_set(&self) -> Arc>> { + self.mounted_info_set.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 05165fc04b..98d51392dd 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 @@ -16,7 +16,7 @@ use tokio::{ process::{Child, Command}, sync::{ mpsc::{channel, Receiver, Sender}, - RwLock, + Mutex, RwLock, }, }; @@ -41,14 +41,13 @@ pub struct ShareVirtioFsStandaloneConfig { #[derive(Default, Debug)] struct ShareVirtioFsStandaloneInner { pid: Option, - mounted_info_set: HashMap, } -#[derive(Debug)] pub(crate) struct ShareVirtioFsStandalone { inner: Arc>, config: ShareVirtioFsStandaloneConfig, share_fs_mount: Arc, + mounted_info_set: Arc>>, } impl ShareVirtioFsStandalone { @@ -63,6 +62,7 @@ impl ShareVirtioFsStandalone { virtio_fs_extra_args: config.virtio_fs_extra_args.clone(), }, share_fs_mount: Arc::new(VirtiofsShareMount::new(id)), + mounted_info_set: Arc::new(Mutex::new(HashMap::new())), }) } @@ -176,33 +176,7 @@ impl ShareFs for ShareVirtioFsStandalone { Ok(vec![]) } - async fn get_mounted_info(&self, source: &str) -> Option { - let inner = self.inner.read().await; - inner.mounted_info_set.get(source).cloned() - } - - async fn set_mounted_info(&self, source: &str, mounted_info: MountedInfo) -> Result<()> { - let mut inner = self.inner.write().await; - inner - .mounted_info_set - .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())) + fn mounted_info_set(&self) -> Arc>> { + self.mounted_info_set.clone() } } 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 9f9778eeb2..c1d999cfb0 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 @@ -170,7 +170,7 @@ impl ShareFsMount for VirtiofsShareMount { }) } - async fn upgrade(&self, file_name: &str) -> Result<()> { + async fn upgrade_to_rw(&self, file_name: &str) -> Result<()> { // Remount readonly directory with readwrite permission let host_dest = do_get_host_path(file_name, &self.id, "", true, true); bind_remount(&host_dest, false) @@ -182,7 +182,7 @@ impl ShareFsMount for VirtiofsShareMount { Ok(()) } - async fn downgrade(&self, file_name: &str) -> Result<()> { + async fn downgrade_to_ro(&self, file_name: &str) -> Result<()> { // Remount readwrite directory with readonly permission let host_dest = do_get_host_path(file_name, &self.id, "", true, false); bind_remount(&host_dest, true) @@ -195,10 +195,9 @@ impl ShareFsMount for VirtiofsShareMount { } 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")?; + // Umount event will be propagated to ro directory Ok(()) } } diff --git a/src/runtime-rs/crates/resource/src/volume/mod.rs b/src/runtime-rs/crates/resource/src/volume/mod.rs index d5d85c46f5..684b76431b 100644 --- a/src/runtime-rs/crates/resource/src/volume/mod.rs +++ b/src/runtime-rs/crates/resource/src/volume/mod.rs @@ -18,7 +18,7 @@ use tokio::sync::RwLock; use crate::share_fs::ShareFs; #[async_trait] -pub trait Volume: Send + Sync + std::fmt::Debug { +pub trait Volume: Send + Sync { fn get_volume_mount(&self) -> Result>; fn get_storage(&self) -> Result>; async fn cleanup(&self) -> Result<()>; 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 5d5f0e4bd6..794f775c44 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 @@ -23,7 +23,6 @@ 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, @@ -71,7 +70,9 @@ impl ShareFsVolume { let readonly = m.options.iter().any(|opt| opt == "ro"); let share_fs_mount = share_fs.get_share_fs_mount(); - if let Some(mut mounted_info) = share_fs.get_mounted_info(&m.source).await { + let mounted_info_set = share_fs.mounted_info_set(); + let mut mounted_info_set = mounted_info_set.lock().await; + if let Some(mut mounted_info) = mounted_info_set.get(&m.source).cloned() { // Mounted at least once let guest_path = mounted_info .guest_path @@ -87,7 +88,7 @@ impl ShareFsVolume { "The mount will be upgraded, mount = {:?}, cid = {}", m, cid ); share_fs_mount - .upgrade( + .upgrade_to_rw( &mounted_info .file_name() .context("get name of mounted info")?, @@ -100,10 +101,7 @@ impl ShareFsVolume { } else { mounted_info.rw_ref_count += 1; } - share_fs - .set_mounted_info(&m.source, mounted_info) - .await - .context("set mounted info")?; + mounted_info_set.insert(m.source.clone(), mounted_info); volume.mounts.push(oci::Mount { destination: m.destination.clone(), @@ -131,10 +129,7 @@ impl ShareFsVolume { .context("convert guest path")?, readonly, ); - share_fs - .set_mounted_info(&m.source, mounted_info) - .await - .context("set mounted info")?; + mounted_info_set.insert(m.source.clone(), mounted_info); // set storages for the volume volume.storages = mount_result.storages; @@ -171,18 +166,23 @@ impl Volume for ShareFsVolume { None => return Err(anyhow!("The share_fs was released unexpectedly")), }; + let mounted_info_set = share_fs.mounted_info_set(); + let mut mounted_info_set = mounted_info_set.lock().await; 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 (host_source, mut mounted_info) = match mounted_info_set + .iter() + .find(|entry| entry.1.guest_path.as_os_str().to_str().unwrap() == m.source) + .map(|entry| (entry.0.to_owned(), entry.1.clone())) + { + Some(entry) => entry, + None => { + warn!( + sl!(), + "The mounted info for guest path {} not found", m.source + ); + continue; + } + }; let old_readonly = mounted_info.readonly(); @@ -206,23 +206,17 @@ impl Volume for ShareFsVolume { 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) + .downgrade_to_ro(&file_name) .await .context("Downgrade volume")?; } - share_fs - .set_mounted_info(&host_source, mounted_info) - .await - .context("Update mounted info")?; + mounted_info_set.insert(host_source.clone(), 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")?; + mounted_info_set.remove(&host_source); // Umount the volume share_fs_mount .umount(&file_name) 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 0c7ac64340..4e694f2e41 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 @@ -277,7 +277,12 @@ impl ContainerInner { 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); + warn!( + sl!(), + "Failed to clean volume {:?}, error = {:?}", + v.get_volume_mount(), + err + ); } } if !unhandled.is_empty() {