diff --git a/CHANGELOG.md b/CHANGELOG.md index d3dee91387d..7b6d867c7db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- [#4309](https://github.com/firecracker-microvm/firecracker/pull/4309): The + jailer's option `--parent-cgroup` will move the process to that cgroup if no + `cgroup` options are provided. - Simplified and clarified the removal policy of deprecated API elements to follow semantic versioning 2.0.0. For more information, please refer to [this GitHub discussion](https://github.com/firecracker-microvm/firecracker/discussions/4135). diff --git a/docs/jailer.md b/docs/jailer.md index 99ed6465e97..725bbc9707f 100644 --- a/docs/jailer.md +++ b/docs/jailer.md @@ -46,6 +46,8 @@ jailer --id \ the jailer will write all cgroup parameters specified through `--cgroup` in `/sys/fs/cgroup//all_uvms/external_uvms/`. By default, the parent cgroup is `exec-file`. + If there are no `--cgroup` parameters specified and `--group-version=2` was + passed, then the jailer will move the process to the specified cgroup. - `cgroup-version` is used to select which type of cgroup hierarchy to use for the creation of cgroups. The default value is "1" which means that cgroups specified with the `cgroup` argument will be created within a v1 hierarchy. diff --git a/src/jailer/src/cgroup.rs b/src/jailer/src/cgroup.rs index 349bf0183ae..ceefdecea40 100644 --- a/src/jailer/src/cgroup.rs +++ b/src/jailer/src/cgroup.rs @@ -161,6 +161,16 @@ impl CgroupBuilder { } } } + + // Returns the path to the root of the hierarchy + pub fn get_v2_hierarchy_path(&mut self) -> Result<&PathBuf, JailerError> { + match self.hierarchies.entry("unified".to_string()) { + Occupied(entry) => Ok(entry.into_mut()), + Vacant(_entry) => Err(JailerError::CgroupHierarchyMissing( + "cgroupsv2 hierarchy missing".to_string(), + )), + } + } } #[derive(Debug)] diff --git a/src/jailer/src/env.rs b/src/jailer/src/env.rs index d7a6613075d..14cf10e24e7 100644 --- a/src/jailer/src/env.rs +++ b/src/jailer/src/env.rs @@ -233,11 +233,24 @@ impl Env { .parse::() .map_err(|_| JailerError::CgroupInvalidVersion(cgroup_ver.to_string()))?; - let mut cgroup_builder = None; + let cgroups_args: &[String] = arguments.multiple_values("cgroup").unwrap_or_default(); + + // If the --parent-cgroup exists, and we have no other cgroups, + // then the intent is to move the process to that cgroup. + // Only applies to cgroupsv2 since it's a unified hierarchy + if cgroups_args.is_empty() && cgroup_ver == 2 { + let mut builder = CgroupBuilder::new(cgroup_ver)?; + let cg_parent = builder.get_v2_hierarchy_path()?.join(parent_cgroup); + let cg_parent_procs = cg_parent.join("cgroup.procs"); + if cg_parent.exists() { + fs::write(cg_parent_procs, std::process::id().to_string()) + .map_err(|_| JailerError::CgroupWrite(io::Error::last_os_error()))?; + } + } // cgroup format: .=,... if let Some(cgroups_args) = arguments.multiple_values("cgroup") { - let builder = cgroup_builder.get_or_insert(CgroupBuilder::new(cgroup_ver)?); + let mut builder = CgroupBuilder::new(cgroup_ver)?; for cg in cgroups_args { let aux: Vec<&str> = cg.split('=').collect(); if aux.len() != 2 || aux[1].is_empty() { diff --git a/src/jailer/src/main.rs b/src/jailer/src/main.rs index d9c969b2bcc..0a63ba64f4a 100644 --- a/src/jailer/src/main.rs +++ b/src/jailer/src/main.rs @@ -32,8 +32,6 @@ pub enum JailerError { CgroupLineNotFound(String, String), #[error("Cgroup invalid file: {0}")] CgroupInvalidFile(String), - #[error("Expected value {0} for {2}. Current value: {1}")] - CgroupWrite(String, String, String), #[error("Invalid format for cgroups: {0}")] CgroupFormat(String), #[error("Hierarchy not found: {0}")] @@ -44,6 +42,8 @@ pub enum JailerError { CgroupInvalidVersion(String), #[error("Parent cgroup path is invalid. Path should not be absolute or contain '..' or '.'")] CgroupInvalidParentPath(), + #[error("Failed to write to cgroups file: {0}")] + CgroupWrite(io::Error), #[error("Failed to change owner for {0:?}: {1}")] ChangeFileOwner(PathBuf, io::Error), #[error("Failed to chdir into chroot directory: {0}")]