From 327e94d45e029b73bec458f9648e554c99039d45 Mon Sep 17 00:00:00 2001 From: utam0k Date: Sun, 8 Aug 2021 16:17:57 +0900 Subject: [PATCH 1/2] pass only the bare necessities in ContainerInitArgs. --- src/container/builder_impl.rs | 2 +- src/process/init.rs | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/container/builder_impl.rs b/src/container/builder_impl.rs index 6fb7b5cf1..07b285230 100644 --- a/src/container/builder_impl.rs +++ b/src/container/builder_impl.rs @@ -70,7 +70,7 @@ impl ContainerBuilderImpl { spec: self.spec.clone(), rootfs: self.rootfs.clone(), console_socket: self.console_socket.clone(), - rootless: self.rootless.clone(), + is_rootless: self.rootless.is_some(), notify_path: self.notify_path.clone(), preserve_fds: self.preserve_fds, child, diff --git a/src/process/init.rs b/src/process/init.rs index fdaa21e3c..7e17bbb9e 100644 --- a/src/process/init.rs +++ b/src/process/init.rs @@ -13,7 +13,6 @@ use crate::{ notify_socket::NotifyListener, process::child, rootfs, - rootless::Rootless, stdio::FileDescriptor, syscall::{linux::LinuxSyscall, Syscall}, tty, utils, @@ -96,7 +95,7 @@ pub struct ContainerInitArgs { /// Socket to communicate the file descriptor of the ptty pub console_socket: Option, /// Options for rootless containers - pub rootless: Option, + pub is_rootless: bool, /// Path to the Unix Domain Socket to communicate container start pub notify_path: PathBuf, /// File descriptos preserved/passed to the container init process. @@ -133,7 +132,7 @@ pub fn container_init(args: ContainerInitArgs) -> Result<()> { // namespace will be created, check // https://man7.org/linux/man-pages/man7/user_namespaces.7.html for more // information - if args.rootless.is_some() { + if args.is_rootless { // child needs to be dumpable, otherwise the non root parent is not // allowed to write the uid/gid maps prctl::set_dumpable(true).unwrap(); From 787a5505c2c23062e7c7c254902c181e061d4a2a Mon Sep 17 00:00:00 2001 From: utam0k Date: Sun, 8 Aug 2021 17:37:13 +0900 Subject: [PATCH 2/2] reduce the number of clones by introducing lifetime to rootless. --- src/container/builder_impl.rs | 10 +++++----- src/container/init_builder.rs | 2 +- src/container/tenant_builder.rs | 2 +- src/process/mod.rs | 4 ++-- src/process/parent.rs | 22 +++++++++++----------- src/rootless.rs | 14 +++++++------- 6 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/container/builder_impl.rs b/src/container/builder_impl.rs index 07b285230..1ea62ef01 100644 --- a/src/container/builder_impl.rs +++ b/src/container/builder_impl.rs @@ -14,7 +14,7 @@ use crate::{ use super::{Container, ContainerStatus}; -pub(super) struct ContainerBuilderImpl { +pub(super) struct ContainerBuilderImpl<'a> { /// Flag indicating if an init or a tenant container should be created pub init: bool, /// Interface to operating system primitives @@ -24,7 +24,7 @@ pub(super) struct ContainerBuilderImpl { /// Id of the container pub container_id: String, /// OCI complient runtime spec - pub spec: Spec, + pub spec: &'a Spec, /// Root filesystem of the container pub rootfs: PathBuf, /// File which will be used to communicate the pid of the @@ -33,7 +33,7 @@ pub(super) struct ContainerBuilderImpl { /// Socket to communicate the file descriptor of the ptty pub console_socket: Option, /// Options for rootless containers - pub rootless: Option, + pub rootless: Option>, /// Path to the Unix Domain Socket to communicate container start pub notify_path: PathBuf, /// Container state @@ -42,7 +42,7 @@ pub(super) struct ContainerBuilderImpl { pub preserve_fds: i32, } -impl ContainerBuilderImpl { +impl<'a> ContainerBuilderImpl<'a> { pub(super) fn create(&mut self) -> Result<()> { self.run_container()?; @@ -58,7 +58,7 @@ impl ContainerBuilderImpl { let namespaces: Namespaces = linux.namespaces.clone().into(); // create the parent and child process structure so the parent and child process can sync with each other - let (mut parent, parent_channel) = parent::ParentProcess::new(self.rootless.clone())?; + let (mut parent, parent_channel) = parent::ParentProcess::new(&self.rootless)?; let child = child::ChildProcess::new(parent_channel)?; // This init_args will be passed to the container init process, diff --git a/src/container/init_builder.rs b/src/container/init_builder.rs index 94416b5a9..f002e8673 100644 --- a/src/container/init_builder.rs +++ b/src/container/init_builder.rs @@ -72,7 +72,7 @@ impl InitContainerBuilder { pid_file: self.base.pid_file, console_socket: csocketfd, use_systemd: self.use_systemd, - spec, + spec: &spec, rootfs, rootless, notify_path, diff --git a/src/container/tenant_builder.rs b/src/container/tenant_builder.rs index 1dd3243f2..173b83eff 100644 --- a/src/container/tenant_builder.rs +++ b/src/container/tenant_builder.rs @@ -111,7 +111,7 @@ impl TenantContainerBuilder { pid_file: self.base.pid_file, console_socket: csocketfd, use_systemd, - spec, + spec: &spec, rootfs, rootless, notify_path: notify_path.clone(), diff --git a/src/process/mod.rs b/src/process/mod.rs index c49c28d5f..8c88e11a1 100644 --- a/src/process/mod.rs +++ b/src/process/mod.rs @@ -12,8 +12,8 @@ pub mod parent; /// Used to describe type of process after fork. /// Parent and child processes mean the same thing as in a normal fork call /// InitProcess is specifically used to indicate the process which will run the command of container -pub enum Process { - Parent(parent::ParentProcess), +pub enum Process<'a> { + Parent(parent::ParentProcess<'a>), Child(child::ChildProcess), } /// Maximum event capacity of polling diff --git a/src/process/parent.rs b/src/process/parent.rs index 0a4fc7004..8e7c9500e 100644 --- a/src/process/parent.rs +++ b/src/process/parent.rs @@ -21,22 +21,22 @@ use oci_spec::LinuxIdMapping; const PARENT: Token = Token(0); /// Contains receiving end of pipe to child process and a poller for that. -pub struct ParentProcess { - child_channel: ChildChannel, +pub struct ParentProcess<'a> { + child_channel: ChildChannel<'a>, } // Poll is used to register and listen for various events // by registering it with an event source such as receiving end of a pipe -impl ParentProcess { +impl<'a> ParentProcess<'a> { /// Create new Parent process structure - pub fn new(rootless: Option) -> Result<(Self, ParentChannel)> { + pub fn new(rootless: &'a Option) -> Result<(Self, ParentChannel)> { let (parent_channel, child_channel) = Self::setup_pipes(rootless)?; let parent = Self { child_channel }; Ok((parent, parent_channel)) } - fn setup_pipes(rootless: Option) -> Result<(ParentChannel, ChildChannel)> { + fn setup_pipes(rootless: &'a Option) -> Result<(ParentChannel, ChildChannel<'a>)> { let (send_to_parent, receive_from_child) = pipe::new()?; let (send_to_child, receive_from_parent) = pipe::new()?; @@ -122,15 +122,15 @@ impl ParentChannel { } } -struct ChildChannel { +struct ChildChannel<'a> { sender: Sender, receiver: Receiver, poll: Poll, - rootless: Option, + rootless: &'a Option>, } -impl ChildChannel { - fn new(sender: Sender, mut receiver: Receiver, rootless: Option) -> Result { +impl<'a> ChildChannel<'a> { + fn new(sender: Sender, mut receiver: Receiver, rootless: &'a Option) -> Result { let poll = Poll::new()?; poll.registry() .register(&mut receiver, PARENT, Interest::READABLE)?; @@ -201,7 +201,7 @@ impl ChildChannel { let rootless = self.rootless.as_ref().unwrap(); write_id_mapping( &format!("/proc/{}/uid_map", target_pid), - &rootless.uid_mappings, + rootless.uid_mappings, rootless.newuidmap.as_deref(), ) } @@ -210,7 +210,7 @@ impl ChildChannel { let rootless = self.rootless.as_ref().unwrap(); write_id_mapping( &format!("/proc/{}/gid_map", target_pid), - &rootless.gid_mappings, + rootless.gid_mappings, rootless.newgidmap.as_deref(), ) } diff --git a/src/rootless.rs b/src/rootless.rs index 84e4c4ce0..ae5861099 100644 --- a/src/rootless.rs +++ b/src/rootless.rs @@ -7,24 +7,24 @@ use oci_spec::{Linux, LinuxIdMapping, Mount, Spec}; use crate::namespaces::Namespaces; #[derive(Debug, Clone)] -pub struct Rootless { +pub struct Rootless<'a> { /// Location of the newuidmap binary pub newuidmap: Option, /// Location of the newgidmap binary pub newgidmap: Option, /// Mappings for user ids - pub uid_mappings: Vec, + pub uid_mappings: &'a Vec, /// Mappings for group ids - pub gid_mappings: Vec, + pub gid_mappings: &'a Vec, } -impl From<&Linux> for Rootless { - fn from(linux: &Linux) -> Self { +impl<'a> From<&'a Linux> for Rootless<'a> { + fn from(linux: &'a Linux) -> Self { Self { newuidmap: None, newgidmap: None, - uid_mappings: linux.uid_mappings.clone(), - gid_mappings: linux.gid_mappings.clone(), + uid_mappings: linux.uid_mappings.as_ref(), + gid_mappings: linux.gid_mappings.as_ref(), } } }