Skip to content

Commit 2cd9af9

Browse files
committed
fix(jailer): honor solitary --parent-cgroup option
If we specify `--parent-cgroup` without any `--cgroup` option, the jailer do anything. We could just error in that situation, and expect something else to launch the jailer process in a cgroup, but we risk breaking anybody that is already using it that way. Instead, we move the process to the `--parent-cgroup` since it's the most intuitive, although the specified `--parent-cgroup` is not a parent in that case. Link: firecracker-microvm#4287 Signed-off-by: Pablo Barbáchano <[email protected]>
1 parent f8d1a9b commit 2cd9af9

File tree

3 files changed

+27
-2
lines changed

3 files changed

+27
-2
lines changed

CHANGELOG.md

+3
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4848

4949
### Changed
5050

51+
- [#XXXX](https://github.com/firecracker-microvm/firecracker/pull/XXXX): The
52+
jailer's option `--parent-cgroup` will move the process to that cgroup if no
53+
`cgroup` options are provided.
5154
- Simplified and clarified the removal policy of deprecated API elements
5255
to follow semantic versioning 2.0.0. For more information, please refer to
5356
[this GitHub discussion](https://github.com/firecracker-microvm/firecracker/discussions/4135).

src/jailer/src/cgroup.rs

+10
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,16 @@ impl CgroupBuilder {
161161
}
162162
}
163163
}
164+
165+
// Returns the path to the root of the hierarchy
166+
pub fn get_v2_hierarchy_path(&mut self) -> Result<&PathBuf, JailerError> {
167+
match self.hierarchies.entry("unified".to_string()) {
168+
Occupied(entry) => Ok(entry.into_mut()),
169+
Vacant(_entry) => Err(JailerError::CgroupHierarchyMissing(
170+
"cgroupsv2 hierarchy missing".to_string(),
171+
)),
172+
}
173+
}
164174
}
165175

166176
#[derive(Debug)]

src/jailer/src/env.rs

+14-2
Original file line numberDiff line numberDiff line change
@@ -233,11 +233,23 @@ impl Env {
233233
.parse::<u8>()
234234
.map_err(|_| JailerError::CgroupInvalidVersion(cgroup_ver.to_string()))?;
235235

236-
let mut cgroup_builder = None;
236+
let cgroups_args: &[String] = arguments.multiple_values("cgroup").unwrap_or_default();
237+
238+
// If the --parent-cgroup exists, and we have no other cgroups,
239+
// then the intent is to move the process to that cgroup.
240+
// Only applies to cgroupsv2 since it's a unified hierarchy
241+
if cgroups_args.len() == 0 && cgroup_ver == 2 {
242+
let mut builder = CgroupBuilder::new(cgroup_ver)?;
243+
let cg_parent = builder.get_v2_hierarchy_path()?.join(parent_cgroup);
244+
let cg_parent_procs = cg_parent.join("cgroup.procs");
245+
if cg_parent.exists() {
246+
fs::write(cg_parent_procs, std::process::id().to_string());
247+
}
248+
}
237249

238250
// cgroup format: <cgroup_controller>.<cgroup_property>=<value>,...
239251
if let Some(cgroups_args) = arguments.multiple_values("cgroup") {
240-
let builder = cgroup_builder.get_or_insert(CgroupBuilder::new(cgroup_ver)?);
252+
let mut builder = CgroupBuilder::new(cgroup_ver)?;
241253
for cg in cgroups_args {
242254
let aux: Vec<&str> = cg.split('=').collect();
243255
if aux.len() != 2 || aux[1].is_empty() {

0 commit comments

Comments
 (0)