feat(jailer): honor solitary --parent-cgroup option on cgroupsv2

If we specify `--parent-cgroup` without any `--cgroup` option, the
jailer doesn't do anything, though a user specifying it could reasonably
expect it to move the process under that cgroup.

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: #4287

Signed-off-by: Pablo Barbáchano <[email protected]>

Author: pb8o

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).

docs/jailer.md

@@ -46,6 +46,8 @@ jailer --id <id> \
   the jailer will write all cgroup parameters specified through `--cgroup` in
   `/sys/fs/cgroup/<controller_name>/all_uvms/external_uvms/<id>`. 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.

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)]

src/jailer/src/env.rs

@@ -233,11 +233,24 @@ impl Env {
             .parse::<u8>()
             .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: <cgroup_controller>.<cgroup_property>=<value>,...
         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() {

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}")]