github/kata-containers
1
0
Fork 1
mirror of https://github.com/kata-containers/kata-containers.git synced 2025-07-14 15:44:58 +00:00
Code Issues Projects Releases Wiki Activity

agent: refine implementation of mount related code

Browse Source 
Refine implementation of mount by:
- log message with `path.display()` instead of `{:?}`
- add prefix "_" to unused variables
- pass by reference instead of by value to avoid creating redundant
  array
- exactly matching prefix "fsgid=" instead of "fsgid"
- avoid redundant clone() operations

Signed-off-by: Jiang Liu <gerry@linux.alibaba.com>
This commit is contained in:
Jiang Liu 2023-08-06 11:59:54 +08:00
parent 98ba211a34
commit baabfa9f1f
1 changed files with 76 additions and 102 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

142
src/agent/src/mount.rs
View File

@ -4,8 +4,8 @@
// //
use std::collections::HashMap; use std::collections::HashMap;
use std::fs; use std::fmt::Debug;
use std::fs::{File, OpenOptions}; use std::fs::{self, File, OpenOptions};
use std::io::{BufRead, BufReader, Write}; use std::io::{BufRead, BufReader, Write};
use std::iter; use std::iter;
use std::os::unix::fs::{MetadataExt, PermissionsExt}; use std::os::unix::fs::{MetadataExt, PermissionsExt};
@ -13,12 +13,13 @@ use std::path::Path;
use std::str::FromStr; use std::str::FromStr;
use std::sync::Arc; use std::sync::Arc;
use tokio::sync::Mutex; use anyhow::{anyhow, Context, Result};
use nix::mount::MsFlags; use nix::mount::MsFlags;
use nix::unistd::{Gid, Uid}; use nix::unistd::{Gid, Uid};
use regex::Regex; use regex::Regex;
use slog::Logger;
use tokio::sync::Mutex;
use tracing::instrument;
use crate::device::{ use crate::device::{
get_scsi_device_name, get_virtio_blk_pci_device_name, get_virtio_mmio_device_name, get_scsi_device_name, get_virtio_blk_pci_device_name, get_virtio_mmio_device_name,
@ -34,17 +35,13 @@ use crate::protocols::types::FSGroupChangePolicy;
use crate::Sandbox; use crate::Sandbox;
#[cfg(target_arch = "s390x")] #[cfg(target_arch = "s390x")]
use crate::{ccw, device::get_virtio_blk_ccw_device_name}; use crate::{ccw, device::get_virtio_blk_ccw_device_name};
use anyhow::{anyhow, Context, Result};
use slog::Logger;
use tracing::instrument;
pub const TYPE_ROOTFS: &str = "rootfs"; pub const TYPE_ROOTFS: &str = "rootfs";
const SYS_FS_HUGEPAGES_PREFIX: &str = "/sys/kernel/mm/hugepages";
pub const MOUNT_GUEST_TAG: &str = "kataShared"; pub const MOUNT_GUEST_TAG: &str = "kataShared";
// Allocating an FSGroup that owns the pod's volumes // Allocating an FSGroup that owns the pod's volumes
const FS_GID: &str = "fsgid"; const FS_GID: &str = "fsgid";
const SYS_FS_HUGEPAGES_PREFIX: &str = "/sys/kernel/mm/hugepages";
const RW_MASK: u32 = 0o660; const RW_MASK: u32 = 0o660;
const RO_MASK: u32 = 0o440; const RO_MASK: u32 = 0o440;
@ -218,9 +215,9 @@ pub fn baremount(
) )
.map_err(|e| { .map_err(|e| {
anyhow!( anyhow!(
"failed to mount {:?} to {:?}, with error: {}", "failed to mount {} to {}, with error: {}",
source, source.display(),
destination, destination.display(),
e e
) )
}) })
@ -230,7 +227,7 @@ pub fn baremount(
async fn ephemeral_storage_handler( async fn ephemeral_storage_handler(
logger: &Logger, logger: &Logger,
storage: &Storage, storage: &Storage,
sandbox: &Arc<Mutex<Sandbox>>, _sandbox: &Arc<Mutex<Sandbox>>,
) -> Result<String> { ) -> Result<String> {
// hugetlbfs // hugetlbfs
if storage.fstype == FS_TYPE_HUGETLB { if storage.fstype == FS_TYPE_HUGETLB {
@ -238,21 +235,19 @@ async fn ephemeral_storage_handler(
} }
// normal ephemeral storage // normal ephemeral storage
fs::create_dir_all(Path::new(&storage.mount_point))?; fs::create_dir_all(&storage.mount_point)?;
if !storage.options.is_empty() {
// By now we only support one option field: "fsGroup" which // By now we only support one option field: "fsGroup" which
// isn't an valid mount option, thus we should remove it when // isn't an valid mount option, thus we should remove it when
// do mount. // do mount.
if !storage.options.is_empty() {
// ephemeral_storage didn't support mount options except fsGroup.
let mut new_storage = storage.clone(); let mut new_storage = storage.clone();
new_storage.options = Default::default(); new_storage.options = Default::default();
common_storage_handler(logger, &new_storage)?; common_storage_handler(logger, &new_storage)?;
let opts_vec: Vec<String> = storage.options.to_vec(); let opts = parse_options(&storage.options);
let opts = parse_options(opts_vec);
// ephemeral_storage didn't support mount options except fsGroup.
if let Some(fsgid) = opts.get(FS_GID) { if let Some(fsgid) = opts.get(FS_GID) {
let gid = fsgid.parse::<u32>()?; let gid = fsgid.parse::<u32>()?;
@ -278,10 +273,10 @@ async fn ephemeral_storage_handler(
pub async fn update_ephemeral_mounts( pub async fn update_ephemeral_mounts(
logger: Logger, logger: Logger,
storages: Vec<Storage>, storages: Vec<Storage>,
sandbox: &Arc<Mutex<Sandbox>>, _sandbox: &Arc<Mutex<Sandbox>>,
) -> Result<()> { ) -> Result<()> {
for (_, storage) in storages.iter().enumerate() { for (_, storage) in storages.iter().enumerate() {
let handler_name = storage.driver.clone(); let handler_name = &storage.driver;
let logger = logger.new(o!( let logger = logger.new(o!(
"msg" => "updating tmpfs storage", "msg" => "updating tmpfs storage",
"subsystem" => "storage", "subsystem" => "storage",
@ -289,24 +284,25 @@ pub async fn update_ephemeral_mounts(
match handler_name.as_str() { match handler_name.as_str() {
DRIVER_EPHEMERAL_TYPE => { DRIVER_EPHEMERAL_TYPE => {
fs::create_dir_all(Path::new(&storage.mount_point))?; fs::create_dir_all(&storage.mount_point)?;
if storage.options.is_empty() { if storage.options.is_empty() {
continue; continue;
} else { } else {
// assume that fsGid has already been set // assume that fsGid has already been set
let mut opts = Vec::<&str>::new(); let mut opts = Vec::new();
for (_, opt) in storage.options.iter().enumerate() { for (_, opt) in storage.options.iter().enumerate() {
if opt.starts_with(FS_GID) { let fields: Vec<&str> = opt.split('=').collect();
if fields.len() == 2 && fields[0] == FS_GID {
continue; continue;
} }
opts.push(opt) opts.push(opt.as_str())
} }
let (flags, options) = parse_mount_flags_and_options(&opts);
let mount_path = Path::new(&storage.mount_point); let mount_path = Path::new(&storage.mount_point);
let src_path = Path::new(&storage.source); let src_path = Path::new(&storage.source);
let (flags, options) = parse_mount_flags_and_options(opts);
info!(logger, "mounting storage"; info!(logger, "mounting storage";
"mount-source" => src_path.display(), "mount-source" => src_path.display(),
"mount-destination" => mount_path.display(), "mount-destination" => mount_path.display(),
@ -327,7 +323,7 @@ pub async fn update_ephemeral_mounts(
_ => { _ => {
return Err(anyhow!( return Err(anyhow!(
"Unsupported storage type for syncing mounts {}. Only ephemeral storage update is supported", "Unsupported storage type for syncing mounts {}. Only ephemeral storage update is supported",
storage.driver.to_owned() storage.driver
)); ));
} }
}; };
@ -374,16 +370,14 @@ async fn overlayfs_storage_handler(
async fn local_storage_handler( async fn local_storage_handler(
_logger: &Logger, _logger: &Logger,
storage: &Storage, storage: &Storage,
sandbox: &Arc<Mutex<Sandbox>>, _sandbox: &Arc<Mutex<Sandbox>>,
) -> Result<String> { ) -> Result<String> {
fs::create_dir_all(&storage.mount_point).context(format!( fs::create_dir_all(&storage.mount_point).context(format!(
"failed to create dir all {:?}", "failed to create dir all {:?}",
&storage.mount_point &storage.mount_point
))?; ))?;
let opts_vec: Vec<String> = storage.options.to_vec(); let opts = parse_options(&storage.options);
let opts = parse_options(opts_vec);
let mut need_set_fsgid = false; let mut need_set_fsgid = false;
if let Some(fsgid) = opts.get(FS_GID) { if let Some(fsgid) = opts.get(FS_GID) {
@ -563,7 +557,6 @@ async fn virtio_blk_storage_handler(
if storage.source.starts_with("/dev") { if storage.source.starts_with("/dev") {
let metadata = fs::metadata(&storage.source) let metadata = fs::metadata(&storage.source)
.context(format!("get metadata on file {:?}", &storage.source))?; .context(format!("get metadata on file {:?}", &storage.source))?;
let mode = metadata.permissions().mode(); let mode = metadata.permissions().mode();
if mode & libc::S_IFBLK == 0 { if mode & libc::S_IFBLK == 0 {
return Err(anyhow!("Invalid device {}", &storage.source)); return Err(anyhow!("Invalid device {}", &storage.source));
@ -620,12 +613,9 @@ async fn virtio_scsi_storage_handler(
#[instrument] #[instrument]
fn common_storage_handler(logger: &Logger, storage: &Storage) -> Result<String> { fn common_storage_handler(logger: &Logger, storage: &Storage) -> Result<String> {
// Mount the storage device.
let mount_point = storage.mount_point.to_string();
mount_storage(logger, storage)?; mount_storage(logger, storage)?;
set_ownership(logger, storage)?; set_ownership(logger, storage)?;
Ok(mount_point) Ok(storage.mount_point.clone())
} }
// nvdimm_storage_handler handles the storage for NVDIMM driver. // nvdimm_storage_handler handles the storage for NVDIMM driver.
@ -666,9 +656,8 @@ async fn bind_watcher_storage_handler(
fn mount_storage(logger: &Logger, storage: &Storage) -> Result<()> { fn mount_storage(logger: &Logger, storage: &Storage) -> Result<()> {
let logger = logger.new(o!("subsystem" => "mount")); let logger = logger.new(o!("subsystem" => "mount"));
// Check share before attempting to mount to see if the destination is already a mount point. // There's a special mechanism to create mountpoint from a `sharedfs` instance before
// If so, skip doing the mount. This facilitates mounting the sharedfs automatically // starting the kata-agent. Check for such cases.
// in the guest before the agent service starts.
if storage.source == MOUNT_GUEST_TAG && is_mounted(&storage.mount_point)? { if storage.source == MOUNT_GUEST_TAG && is_mounted(&storage.mount_point)? {
warn!( warn!(
logger, logger,
@ -680,27 +669,23 @@ fn mount_storage(logger: &Logger, storage: &Storage) -> Result<()> {
let mount_path = Path::new(&storage.mount_point); let mount_path = Path::new(&storage.mount_point);
let src_path = Path::new(&storage.source); let src_path = Path::new(&storage.source);
if storage.fstype == "bind" && !src_path.is_dir() { if storage.fstype == "bind" && !src_path.is_dir() {
ensure_destination_file_exists(mount_path) ensure_destination_file_exists(mount_path).context("Could not create mountpoint file")?;
} else { } else {
fs::create_dir_all(mount_path).map_err(anyhow::Error::from) fs::create_dir_all(mount_path)
} .map_err(anyhow::Error::from)
.context("Could not create mountpoint")?; .context("Could not create mountpoint")?;
}
let options_vec = storage.options.to_vec(); let (flags, options) = parse_mount_flags_and_options(&storage.options);
let options_vec = options_vec.iter().map(String::as_str).collect();
let (flags, options) = parse_mount_flags_and_options(options_vec);
let source = Path::new(&storage.source);
info!(logger, "mounting storage"; info!(logger, "mounting storage";
"mount-source" => source.display(), "mount-source" => src_path.display(),
"mount-destination" => mount_path.display(), "mount-destination" => mount_path.display(),
"mount-fstype" => storage.fstype.as_str(), "mount-fstype" => storage.fstype.as_str(),
"mount-options" => options.as_str(), "mount-options" => options.as_str(),
); );
baremount( baremount(
source, src_path,
mount_path, mount_path,
storage.fstype.as_str(), storage.fstype.as_str(),
flags, flags,
@ -717,14 +702,9 @@ pub fn set_ownership(logger: &Logger, storage: &Storage) -> Result<()> {
if storage.fs_group.is_none() { if storage.fs_group.is_none() {
return Ok(()); return Ok(());
} }
let fs_group = storage.fs_group(); let fs_group = storage.fs_group();
let read_only = storage.options.contains(&String::from("ro"));
let mut read_only = false;
let opts_vec: Vec<String> = storage.options.to_vec();
if opts_vec.contains(&String::from("ro")) {
read_only = true;
}
let mount_path = Path::new(&storage.mount_point); let mount_path = Path::new(&storage.mount_point);
let metadata = mount_path.metadata().map_err(|err| { let metadata = mount_path.metadata().map_err(|err| {
error!(logger, "failed to obtain metadata for mount path"; error!(logger, "failed to obtain metadata for mount path";
@ -822,11 +802,12 @@ pub fn is_mounted(mount_point: &str) -> Result<bool> {
} }
#[instrument] #[instrument]
fn parse_mount_flags_and_options(options_vec: Vec<&str>) -> (MsFlags, String) { fn parse_mount_flags_and_options<S: AsRef<str> + Debug>(options_vec: &[S]) -> (MsFlags, String) {
let mut flags = MsFlags::empty(); let mut flags = MsFlags::empty();
let mut options: String = "".to_string(); let mut options: String = "".to_string();
for opt in options_vec { for opt in options_vec {
let opt = opt.as_ref();
if !opt.is_empty() { if !opt.is_empty() {
match FLAGS.get(opt) { match FLAGS.get(opt) {
Some(x) => { Some(x) => {
@ -881,22 +862,20 @@ pub async fn add_storages(
} }
let res = match handler_name.as_str() { let res = match handler_name.as_str() {
DRIVER_BLK_TYPE => virtio_blk_storage_handler(&logger, &storage, sandbox).await, DRIVER_BLK_TYPE => virtio_blk_storage_handler(&logger, storage, sandbox).await,
DRIVER_BLK_CCW_TYPE => virtio_blk_ccw_storage_handler(&logger, &storage, sandbox).await, DRIVER_BLK_CCW_TYPE => virtio_blk_ccw_storage_handler(&logger, storage, sandbox).await,
DRIVER_9P_TYPE => virtio9p_storage_handler(&logger, &storage, sandbox).await, DRIVER_9P_TYPE => virtio9p_storage_handler(&logger, storage, sandbox).await,
DRIVER_VIRTIOFS_TYPE => virtiofs_storage_handler(&logger, &storage, sandbox).await, DRIVER_VIRTIOFS_TYPE => virtiofs_storage_handler(&logger, storage, sandbox).await,
DRIVER_EPHEMERAL_TYPE => ephemeral_storage_handler(&logger, &storage, sandbox).await, DRIVER_EPHEMERAL_TYPE => ephemeral_storage_handler(&logger, storage, sandbox).await,
DRIVER_OVERLAYFS_TYPE => { DRIVER_OVERLAYFS_TYPE => {
overlayfs_storage_handler(&logger, &storage, cid.as_deref(), sandbox).await overlayfs_storage_handler(&logger, storage, cid.as_deref(), sandbox).await
} }
DRIVER_MMIO_BLK_TYPE => { DRIVER_MMIO_BLK_TYPE => virtiommio_blk_storage_handler(&logger, storage, sandbox).await,
virtiommio_blk_storage_handler(&logger, &storage, sandbox).await DRIVER_LOCAL_TYPE => local_storage_handler(&logger, storage, sandbox).await,
} DRIVER_SCSI_TYPE => virtio_scsi_storage_handler(&logger, storage, sandbox).await,
DRIVER_LOCAL_TYPE => local_storage_handler(&logger, &storage, sandbox).await, DRIVER_NVDIMM_TYPE => nvdimm_storage_handler(&logger, storage, sandbox).await,
DRIVER_SCSI_TYPE => virtio_scsi_storage_handler(&logger, &storage, sandbox).await,
DRIVER_NVDIMM_TYPE => nvdimm_storage_handler(&logger, &storage, sandbox).await,
DRIVER_WATCHABLE_BIND_TYPE => { DRIVER_WATCHABLE_BIND_TYPE => {
bind_watcher_storage_handler(&logger, &storage, sandbox, cid.clone()).await?; bind_watcher_storage_handler(&logger, storage, sandbox, cid.clone()).await?;
// Don't register watch mounts, they're handled separately by the watcher. // Don't register watch mounts, they're handled separately by the watcher.
Ok(String::new()) Ok(String::new())
} }
@ -933,11 +912,9 @@ pub async fn add_storages(
#[instrument] #[instrument]
fn mount_to_rootfs(logger: &Logger, m: &InitMount) -> Result<()> { fn mount_to_rootfs(logger: &Logger, m: &InitMount) -> Result<()> {
let options_vec: Vec<&str> = m.options.clone(); let (flags, options) = parse_mount_flags_and_options(&m.options);
let (flags, options) = parse_mount_flags_and_options(options_vec); fs::create_dir_all(m.dest).context("could not create directory")?;
fs::create_dir_all(Path::new(m.dest)).context("could not create directory")?;
let source = Path::new(m.src); let source = Path::new(m.src);
let dest = Path::new(m.dest); let dest = Path::new(m.dest);
@ -1142,17 +1119,14 @@ fn ensure_destination_file_exists(path: &Path) -> Result<()> {
} }
#[instrument] #[instrument]
fn parse_options(option_list: Vec<String>) -> HashMap<String, String> { fn parse_options(option_list: &[String]) -> HashMap<String, String> {
let mut options = HashMap::new(); let mut options = HashMap::new();
for opt in option_list.iter() { for opt in option_list.iter() {
let fields: Vec<&str> = opt.split('=').collect(); let fields: Vec<&str> = opt.split('=').collect();
if fields.len() != 2 { if fields.len() == 2 {
continue;
}
options.insert(fields[0].to_string(), fields[1].to_string()); options.insert(fields[0].to_string(), fields[1].to_string());
} }
}
options options
} }
@ -2069,7 +2043,7 @@ mod tests {
for (i, d) in tests.iter().enumerate() { for (i, d) in tests.iter().enumerate() {
let msg = format!("test[{}]: {:?}", i, d); let msg = format!("test[{}]: {:?}", i, d);
let result = parse_mount_flags_and_options(d.options_vec.clone()); let result = parse_mount_flags_and_options(&d.options_vec);
let msg = format!("{}: result: {:?}", msg, result); let msg = format!("{}: result: {:?}", msg, result);