Merge pull request #89 from Tim-Zhang/release-0.2.11

release: v0.2.11

Cargo.toml

@@ -5,7 +5,7 @@ repository = "https://github.com/kata-containers/cgroups-rs"
 keywords = ["linux", "cgroup", "containers", "isolation"]
 categories = ["os", "api-bindings", "os::unix-apis"]
 license = "MIT OR Apache-2.0"
-version = "0.2.8"
+version = "0.2.11"
 authors = ["The Kata Containers community <kata-dev@lists.katacontainers.io>", "Levente Kurusa <lkurusa@acm.org>", "Sam Wilson <tecywiz121@hotmail.com>"]
 edition = "2018"
 homepage = "https://github.com/kata-containers/cgroups-rs"
@@ -14,7 +14,7 @@ readme = "README.md"
 [dependencies]
 log = "0.4"
 regex = "1.1"
-nix = "0.23.0"
+nix = { version = "0.24", default-features = false, features = ["event", "fs", "process"] }
 libc = "0.2"
 serde = { version = "1.0", features = ["derive"], optional = true }
 

src/blkio.rs

@@ -16,7 +16,8 @@ use crate::error::*;
 
 use crate::{read_string_from, read_u64_from};
 use crate::{
-    BlkIoResources, ControllIdentifier, ControllerInternal, Controllers, Resources, Subsystem,
+    BlkIoResources, ControllIdentifier, ControllerInternal, Controllers, CustomizedAttribute,
+    Resources, Subsystem,
 };
 
 /// A controller that allows controlling the `blkio` subsystem of a Cgroup.
@@ -378,6 +379,10 @@ impl ControllerInternal for BlkIoController {
             let _ = self.throttle_write_iops_for_device(dev.major, dev.minor, dev.rate);
         }
 
+        res.attrs.iter().for_each(|(k, v)| {
+            let _ = self.set(k, v);
+        });
+
         Ok(())
     }
 }
@@ -771,6 +776,7 @@ impl BlkIoController {
     }
 }
 
+impl CustomizedAttribute for BlkIoController {}
 #[cfg(test)]
 mod test {
     use crate::blkio::{parse_blkio_data, BlkIoData};

src/cgroup.rs

@@ -359,9 +359,18 @@ fn create_v2_cgroup(root: PathBuf, path: &str) -> Result<()> {
 }
 
 pub fn get_cgroups_relative_paths() -> Result<HashMap<String, String>> {
+    let path = "/proc/self/cgroup".to_string();
+    get_cgroups_relative_paths_by_path(path)
+}
+
+pub fn get_cgroups_relative_paths_by_pid(pid: u32) -> Result<HashMap<String, String>> {
+    let path = format!("/proc/{}/cgroup", pid);
+    get_cgroups_relative_paths_by_path(path)
+}
+
+fn get_cgroups_relative_paths_by_path(path: String) -> Result<HashMap<String, String>> {
     let mut m = HashMap::new();
-    let content =
-        fs::read_to_string("/proc/self/cgroup").map_err(|e| Error::with_cause(ReadFailed, e))?;
+    let content = fs::read_to_string(path).map_err(|e| Error::with_cause(ReadFailed, e))?;
     for l in content.lines() {
         let fl: Vec<&str> = l.split(':').collect();
         if fl.len() != 3 {

src/cpu.rs

@@ -295,7 +295,7 @@ fn parse_cfs_quota_and_period(mut file: File) -> Result<CfsQuotaAndPeriod> {
         return Err(Error::from_string(format!("invaild format: {}", content)));
     }
 
-    let quota = parse_max_value(&fields[0].to_string())?;
+    let quota = parse_max_value(fields[0])?;
     let period = fields[1]
         .parse::<u64>()
         .map_err(|e| Error::with_cause(ParseError, e))?;

src/events.rs

@@ -52,12 +52,11 @@ fn register_memory_event(
         eventfd(0, EfdFlags::EFD_CLOEXEC).map_err(|e| Error::with_cause(ReadFailed, e))?;
 
     let event_control_path = cg_dir.join("cgroup.event_control");
-    let data;
-    if arg.is_empty() {
-        data = format!("{} {}", eventfd, event_file.as_raw_fd());
+    let data = if arg.is_empty() {
+        format!("{} {}", eventfd, event_file.as_raw_fd())
     } else {
-        data = format!("{} {} {}", eventfd, event_file.as_raw_fd(), arg);
-    }
+        format!("{} {} {}", eventfd, event_file.as_raw_fd(), arg)
+    };
 
     // write to file and set mode to 0700(FIXME)
     fs::write(&event_control_path, data).map_err(|e| Error::with_cause(WriteFailed, e))?;

src/hierarchies.rs

@@ -170,7 +170,7 @@ impl Hierarchy for V1 {
     }
 
     fn root_control_group(&self) -> Cgroup {
-        Cgroup::load(auto(), "".to_string())
+        Cgroup::load(auto(), "")
     }
 
     fn root(&self) -> PathBuf {
@@ -246,7 +246,7 @@ impl Hierarchy for V2 {
     }
 
     fn root_control_group(&self) -> Cgroup {
-        Cgroup::load(auto(), "".to_string())
+        Cgroup::load(auto(), "")
     }
 
     fn root(&self) -> PathBuf {
@@ -297,7 +297,10 @@ impl Default for V2 {
 
 pub const UNIFIED_MOUNTPOINT: &str = "/sys/fs/cgroup";
 
-#[cfg(all(target_os = "linux", not(target_env = "musl")))]
+#[cfg(any(
+    all(target_os = "linux", not(target_env = "musl")),
+    target_os = "android"
+))]
 pub fn is_cgroup2_unified_mode() -> bool {
     use nix::sys::statfs;
 

src/hugetlb.rs

@@ -8,6 +8,7 @@
 //!
 //! See the Kernel's documentation for more information about this subsystem, found at:
 //!  [Documentation/cgroup-v1/hugetlb.txt](https://www.kernel.org/doc/Documentation/cgroup-v1/hugetlb.txt)
+use log::warn;
 use std::io::Write;
 use std::path::PathBuf;
 
@@ -88,7 +89,7 @@ impl<'a> From<&'a Subsystem> for &'a HugeTlbController {
 impl HugeTlbController {
     /// Constructs a new `HugeTlbController` with `root` serving as the root of the control group.
     pub fn new(root: PathBuf, v2: bool) -> Self {
-        let sizes = get_hugepage_sizes().unwrap();
+        let sizes = get_hugepage_sizes();
         Self {
             base: root.clone(),
             path: root,
@@ -180,27 +181,29 @@ use regex::Regex;
 use std::collections::HashMap;
 use std::fs;
 
-fn get_hugepage_sizes() -> Result<Vec<String>> {
-    let mut m = Vec::new();
+fn get_hugepage_sizes() -> Vec<String> {
     let dirs = fs::read_dir(HUGEPAGESIZE_DIR);
     if dirs.is_err() {
-        return Ok(m);
+        return Vec::new();
     }
 
-    for e in dirs.unwrap() {
-        let entry = e.unwrap();
-        let name = entry.file_name().into_string().unwrap();
-        let parts: Vec<&str> = name.split('-').collect();
-        if parts.len() != 2 {
-            continue;
-        }
-        let bmap = get_binary_size_map();
-        let size = parse_size(parts[1], &bmap)?;
-        let dabbrs = get_decimal_abbrs();
-        m.push(custom_size(size as f64, 1024.0, &dabbrs));
-    }
+    dirs.unwrap()
+        .filter_map(|e| {
+            let entry = e.map_err(|e| warn!("readdir error: {:?}", e)).ok()?;
+            let name = entry.file_name().into_string().unwrap();
+            let parts: Vec<&str> = name.split('-').collect();
+            if parts.len() != 2 {
+                return None;
+            }
+            let bmap = get_binary_size_map();
+            let size = parse_size(parts[1], &bmap)
+                .map_err(|e| warn!("parse_size error: {:?}", e))
+                .ok()?;
+            let dabbrs = get_decimal_abbrs();
 
-    Ok(m)
+            Some(custom_size(size as f64, 1024.0, &dabbrs))
+        })
+        .collect()
 }
 
 pub const KB: u128 = 1000;

src/lib.rs

@@ -332,7 +332,23 @@ where
             return Ok(());
         }
 
-        remove_dir(self.get_path())
+        // Compatible with runC for remove dir operation
+        // https://github.com/opencontainers/runc/blob/main/libcontainer/cgroups/utils.go#L272
+        //
+        // We trying to remove all paths five times with increasing delay between tries.
+        // If after all there are not removed cgroups - appropriate error will be
+        // returned.
+        let mut delay = std::time::Duration::from_millis(10);
+        let cgroup_path = self.get_path();
+        for _i in 0..4 {
+            if let Ok(()) = remove_dir(cgroup_path) {
+                return Ok(());
+            }
+            std::thread::sleep(delay);
+            delay *= 2;
+        }
+
+        remove_dir(cgroup_path)
     }
 
     /// Attach a task to this controller.
@@ -613,6 +629,15 @@ pub struct BlkIoResources {
     pub throttle_write_bps_device: Vec<BlkIoDeviceThrottleResource>,
     /// Throttled write IO operations per second can be provided for each device.
     pub throttle_write_iops_device: Vec<BlkIoDeviceThrottleResource>,
+
+    /// Customized key-value attributes
+    /// # Usage:
+    /// ```
+    /// let resource = &mut cgroups_rs::Resources::default();
+    /// resource.blkio.attrs.insert("io.cost.weight".to_string(), "10".to_string());
+    /// // apply here
+    /// ```
+    pub attrs: HashMap<String, String>,
 }
 
 /// The resource limits and constraints that will be set on the control group.
@@ -771,7 +796,7 @@ impl fmt::Display for MaxValue {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
             MaxValue::Max => write!(f, "max"),
-            MaxValue::Value(num) => write!(f, "{}", num.to_string()),
+            MaxValue::Value(num) => write!(f, "{}", num),
         }
     }
 }

src/memory.rs

@@ -8,6 +8,7 @@
 //!
 //! See the Kernel's documentation for more information about this subsystem, found at:
 //!  [Documentation/cgroup-v1/memory.txt](https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt)
+use log::warn;
 use std::collections::HashMap;
 use std::io::Write;
 use std::path::PathBuf;
@@ -840,8 +841,15 @@ impl MemController {
 
         self.open_path("memory.kmem.limit_in_bytes", true)
             .and_then(|mut file| {
-                file.write_all(limit.to_string().as_ref())
-                    .map_err(|e| Error::with_cause(WriteFailed, e))
+                let r = file.write_all(limit.to_string().as_ref());
+                match r {
+                    Ok(()) => Ok(()),
+                    Err(ref e) if e.raw_os_error() == Some(libc::EOPNOTSUPP) => {
+                        warn!("memory.kmem.limit_in_bytes is unsupported by the kernel");
+                        Ok(())
+                    }
+                    Err(e) => Err(Error::with_cause(WriteFailed, e)),
+                }
             })
     }
 

src/systemd.rs

@@ -17,7 +17,7 @@ use crate::{ControllIdentifier, ControllerInternal, Controllers, Resources, Subs
 pub struct SystemdController {
     base: PathBuf,
     path: PathBuf,
-    v2: bool,
+    _v2: bool,
 }
 
 impl ControllerInternal for SystemdController {
@@ -66,7 +66,7 @@ impl SystemdController {
         Self {
             base: root.clone(),
             path: root,
-            v2,
+            _v2: v2,
         }
     }
 }

tests/builder.rs

@@ -47,7 +47,8 @@ pub fn test_memory_res_build() {
     {
         let c: &MemController = cg.controller_of().unwrap();
         if !c.v2() {
-            assert_eq!(c.kmem_stat().limit_in_bytes, 128 * 1024 * 1024);
+            // Note: we don't tests the value of c.kmem_stat().limit_in_bytes because on Linux
+            // kernel >= 5.16 setting this value is unsupported.
             assert_eq!(c.memory_stat().swappiness, 70);
         }
         assert_eq!(c.memory_stat().limit_in_bytes, 1024 * 1024 * 1024);
@@ -137,11 +138,8 @@ pub fn test_hugepages_res_build() {
 
     {
         let c: &HugeTlbController = cg.controller_of().unwrap();
-        assert!(c.limit_in_bytes(&"2MB".to_string()).is_ok());
-        assert_eq!(
-            c.limit_in_bytes(&"2MB".to_string()).unwrap(),
-            4 * 2 * 1024 * 1024
-        );
+        assert!(c.limit_in_bytes("2MB").is_ok());
+        assert_eq!(c.limit_in_bytes("2MB").unwrap(), 4 * 2 * 1024 * 1024);
     }
     cg.delete().unwrap();
 }

tests/hugetlb.rs

@@ -40,5 +40,5 @@ fn test_hugetlb_sizes() {
 }
 
 fn assert_no_error(r: Result<u64>) {
-    assert!(!r.is_err())
+    assert!(r.is_ok())
 }

tests/memory.rs

@@ -23,7 +23,7 @@ fn test_disable_oom_killer() {
         if !mem_controller.v2() {
             // disable oom killer
             let r = mem_controller.disable_oom_killer();
-            assert!(!r.is_err());
+            assert!(r.is_ok());
 
             // after disable
             let m = mem_controller.memory_stat();
@@ -33,6 +33,21 @@ fn test_disable_oom_killer() {
     cg.delete().unwrap();
 }
 
+#[test]
+fn set_kmem_limit_v1() {
+    let h = cgroups_rs::hierarchies::auto();
+    if h.v2() {
+        return;
+    }
+
+    let cg = Cgroup::new(h, String::from("set_kmem_limit_v1"));
+    {
+        let mem_controller: &MemController = cg.controller_of().unwrap();
+        mem_controller.set_kmem_limit(1).unwrap();
+    }
+    cg.delete().unwrap();
+}
+
 #[test]
 fn set_mem_v2() {
     let h = cgroups_rs::hierarchies::auto();