runtime-rs: fix the issues mentioned in the code review

In order to avoid cloning, changed the signature of
`ShareFsMount::share_rootfs`, `ShareFsMount::share_volume`, and
`ShareFsMount::umount_rootfs` to receive a reference to a config.

Fixes: #5898

Signed-off-by: Xuewei Niu <niuxuewei.nxw@antgroup.com>
This commit is contained in:
Xuewei Niu 2022-12-15 11:22:03 +08:00
parent 0e69207909
commit fd77eebd4d
9 changed files with 41 additions and 24 deletions

View File

@ -1223,6 +1223,7 @@ dependencies = [
"seccompiler",
"serde",
"serde_json",
"shim-interface",
"slog",
"slog-scope",
"thiserror",
@ -1919,6 +1920,7 @@ dependencies = [
"safe-path",
"serde",
"serde_json",
"shim-interface",
]
[[package]]
@ -2343,6 +2345,7 @@ dependencies = [
"logging",
"oci",
"persist",
"shim-interface",
"slog",
"slog-scope",
"tokio",
@ -2474,6 +2477,7 @@ dependencies = [
"logging",
"persist",
"runtimes",
"shim-interface",
"slog",
"slog-scope",
"tokio",
@ -2539,12 +2543,23 @@ dependencies = [
name = "shim-ctl"
version = "0.1.0"
dependencies = [
"anyhow",
"common",
"logging",
"runtimes",
"tokio",
]
[[package]]
name = "shim-interface"
version = "0.1.0"
dependencies = [
"anyhow",
"hyper",
"hyperlocal",
"tokio",
]
[[package]]
name = "signal-hook-registry"
version = "1.4.0"

View File

@ -42,19 +42,11 @@ impl NydusRootfs {
cid: &str,
rootfs: &Mount,
) -> Result<Self> {
let share_fs = Arc::clone(share_fs);
let share_fs_mount = share_fs.get_share_fs_mount();
let extra_options =
NydusExtraOptions::new(rootfs).context("failed to parse nydus extra options")?;
info!(sl!(), "extra_option {:?}", &extra_options);
let rafs_meta = &extra_options.source;
let config = ShareFsRootfsConfig {
cid: cid.to_string(),
source: extra_options.snapshot_dir.clone(),
target: SNAPSHOT_DIR.to_string(),
readonly: true,
is_rafs: false,
};
let (rootfs_storage, rootfs_guest_path) = match extra_options.fs_version.as_str() {
// both nydus v5 and v6 can be handled by the builtin nydus in dragonball by using the rafs mode.
// nydus v6 could also be handled by the guest kernel as well, but some kernel patch is not support in the upstream community. We will add an option to let runtime-rs handle nydus v6 in the guest kernel optionally once the patch is ready
@ -81,7 +73,13 @@ impl NydusRootfs {
let rootfs_guest_path = do_get_guest_path(ROOTFS, cid, false, false);
// bind mount the snapshot dir under the share directory
share_fs_mount
.share_rootfs(config.clone())
.share_rootfs(&ShareFsRootfsConfig {
cid: cid.to_string(),
source: extra_options.snapshot_dir.clone(),
target: SNAPSHOT_DIR.to_string(),
readonly: true,
is_rafs: false,
})
.await
.context("share nydus rootfs")?;
let mut options: Vec<String> = Vec::new();
@ -148,7 +146,8 @@ impl Rootfs for NydusRootfs {
}
async fn cleanup(&self) -> Result<()> {
warn!(sl!(), "Cleaning up Nydus Rootfs is still unimplemented.");
// TODO: Clean up NydusRootfs after the container is killed
warn!(sl!(), "Cleaning up NydusRootfs is still unimplemented.");
Ok(())
}
}

View File

@ -48,7 +48,7 @@ impl ShareFsRootfs {
};
let mount_result = share_fs_mount
.share_rootfs(config.clone())
.share_rootfs(&config)
.await
.context("share rootfs")?;
@ -78,7 +78,7 @@ impl Rootfs for ShareFsRootfs {
// Umount the mount point shared to guest
let share_fs_mount = self.share_fs.get_share_fs_mount();
share_fs_mount
.umount_rootfs(self.config.clone())
.umount_rootfs(&self.config)
.await
.context("umount shared rootfs")?;

View File

@ -120,8 +120,8 @@ impl MountedInfo {
#[async_trait]
pub trait ShareFsMount: Send + Sync {
async fn share_rootfs(&self, config: ShareFsRootfsConfig) -> Result<ShareFsMountResult>;
async fn share_volume(&self, config: ShareFsVolumeConfig) -> Result<ShareFsMountResult>;
async fn share_rootfs(&self, config: &ShareFsRootfsConfig) -> Result<ShareFsMountResult>;
async fn share_volume(&self, config: &ShareFsVolumeConfig) -> Result<ShareFsMountResult>;
/// Upgrade to readwrite permission
async fn upgrade_to_rw(&self, file_name: &str) -> Result<()>;
/// Downgrade to readonly permission
@ -129,7 +129,7 @@ pub trait ShareFsMount: Send + Sync {
/// Umount the volume
async fn umount_volume(&self, file_name: &str) -> Result<()>;
/// Umount the rootfs
async fn umount_rootfs(&self, config: ShareFsRootfsConfig) -> Result<()>;
async fn umount_rootfs(&self, config: &ShareFsRootfsConfig) -> Result<()>;
}
pub fn new(id: &str, config: &SharedFsInfo) -> Result<Arc<dyn ShareFs>> {

View File

@ -38,7 +38,7 @@ impl VirtiofsShareMount {
#[async_trait]
impl ShareFsMount for VirtiofsShareMount {
async fn share_rootfs(&self, config: ShareFsRootfsConfig) -> Result<ShareFsMountResult> {
async fn share_rootfs(&self, config: &ShareFsRootfsConfig) -> Result<ShareFsMountResult> {
// TODO: select virtiofs or support nydus
let guest_path = utils::share_to_guest(
&config.source,
@ -56,7 +56,7 @@ impl ShareFsMount for VirtiofsShareMount {
})
}
async fn share_volume(&self, config: ShareFsVolumeConfig) -> Result<ShareFsMountResult> {
async fn share_volume(&self, config: &ShareFsVolumeConfig) -> Result<ShareFsMountResult> {
let mut guest_path = utils::share_to_guest(
&config.source,
&config.target,
@ -103,7 +103,7 @@ impl ShareFsMount for VirtiofsShareMount {
source: guest_path,
fs_type: String::from("bind"),
fs_group: None,
options: config.mount_options,
options: config.mount_options.clone(),
mount_point: watchable_guest_mount.clone(),
};
@ -211,7 +211,7 @@ impl ShareFsMount for VirtiofsShareMount {
Ok(())
}
async fn umount_rootfs(&self, config: ShareFsRootfsConfig) -> Result<()> {
async fn umount_rootfs(&self, config: &ShareFsRootfsConfig) -> Result<()> {
let host_dest = do_get_host_path(&config.target, &self.id, &config.cid, false, false);
umount_timeout(&host_dest, 0).context("umount rootfs")?;

View File

@ -30,6 +30,7 @@ impl Volume for BlockVolume {
}
async fn cleanup(&self) -> Result<()> {
// TODO: Clean up BlockVolume
warn!(sl!(), "Cleaning up BlockVolume is still unimplemented.");
Ok(())
}

View File

@ -34,6 +34,7 @@ impl Volume for DefaultVolume {
}
async fn cleanup(&self) -> Result<()> {
// TODO: Clean up DefaultVolume
warn!(sl!(), "Cleaning up DefaultVolume is still unimplemented.");
Ok(())
}

View File

@ -112,7 +112,7 @@ impl ShareFsVolume {
} else {
// Not mounted ever
let mount_result = share_fs_mount
.share_volume(ShareFsVolumeConfig {
.share_volume(&ShareFsVolumeConfig {
// The scope of shared volume is sandbox
cid: String::from(""),
source: m.source.clone(),
@ -158,10 +158,10 @@ impl Volume for ShareFsVolume {
}
async fn cleanup(&self) -> Result<()> {
if self.share_fs.is_none() {
return Ok(());
}
let share_fs = self.share_fs.as_ref().unwrap();
let share_fs = match self.share_fs.as_ref() {
Some(fs) => fs,
None => return Ok(()),
};
let mounted_info_set = share_fs.mounted_info_set();
let mut mounted_info_set = mounted_info_set.lock().await;

View File

@ -100,6 +100,7 @@ impl Volume for ShmVolume {
}
async fn cleanup(&self) -> Result<()> {
// TODO: Clean up ShmVolume
warn!(sl!(), "Cleaning up ShmVolume is still unimplemented.");
Ok(())
}