fix(cgroup): fix parse zero or multi optional fields in mountinfo (#113)

seanmonstar · Dec 20, 2021 · a1c3827 · a1c3827
1 parent f9b4136
commit a1c3827
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 50 deletions.
diff --git a/fixtures/cgroups/proc/cgroups/mountinfo_multi_opt b/fixtures/cgroups/proc/cgroups/mountinfo_multi_opt
@@ -0,0 +1,8 @@
+1 0 8:1 / / rw,noatime shared:1 - ext4 /dev/sda1 rw,errors=remount-ro,data=reordered
+2 1 0:1 / /dev rw,relatime shared:2 - devtmpfs udev rw,size=10240k,nr_inodes=16487629,mode=755
+3 1 0:2 / /proc rw,nosuid,nodev,noexec,relatime shared:3 - proc proc rw
+4 1 0:3 / /sys rw,nosuid,nodev,noexec,relatime shared:4 - sysfs sysfs rw
+5 4 0:4 / /sys/fs/cgroup ro,nosuid,nodev,noexec shared:5 - tmpfs tmpfs ro,mode=755
+6 5 0:5 / /sys/fs/cgroup/cpuset rw,nosuid,nodev,noexec,relatime shared:6 - cgroup cgroup rw,cpuset
+7 5 0:6 / /sys/fs/cgroup/cpu,cpuacct rw,nosuid,nodev,noexec,relatime shared:7 shared:8 shared:9 - cgroup cgroup rw,cpu,cpuacct
+8 5 0:7 / /sys/fs/cgroup/memory rw,nosuid,nodev,noexec,relatime shared:8 - cgroup cgroup rw,memory
diff --git a/fixtures/cgroups/proc/cgroups/mountinfo_zero_opt b/fixtures/cgroups/proc/cgroups/mountinfo_zero_opt
@@ -0,0 +1,8 @@
+1 0 8:1 / / rw,noatime shared:1 - ext4 /dev/sda1 rw,errors=remount-ro,data=reordered
+2 1 0:1 / /dev rw,relatime shared:2 - devtmpfs udev rw,size=10240k,nr_inodes=16487629,mode=755
+3 1 0:2 / /proc rw,nosuid,nodev,noexec,relatime shared:3 - proc proc rw
+4 1 0:3 / /sys rw,nosuid,nodev,noexec,relatime shared:4 - sysfs sysfs rw
+5 4 0:4 / /sys/fs/cgroup ro,nosuid,nodev,noexec shared:5 - tmpfs tmpfs ro,mode=755
+6 5 0:5 / /sys/fs/cgroup/cpuset rw,nosuid,nodev,noexec,relatime shared:6 - cgroup cgroup rw,cpuset
+7 5 0:6 / /sys/fs/cgroup/cpu,cpuacct rw,nosuid,nodev,noexec,relatime - cgroup cgroup rw,cpu,cpuacct
+8 5 0:7 / /sys/fs/cgroup/memory rw,nosuid,nodev,noexec,relatime shared:8 - cgroup cgroup rw,memory
diff --git a/src/linux.rs b/src/linux.rs
@@ -18,15 +18,15 @@ macro_rules! debug {
 }
 
 macro_rules! some {
-    ($e:expr) => ({
+    ($e:expr) => {{
         match $e {
             Some(v) => v,
             None => {
                 debug!("NONE: {:?}", stringify!($e));
                 return None;
             }
         }
-    })
+    }};
 }
 
 pub fn get_num_cpus() -> usize {
@@ -126,18 +126,15 @@ fn init_cgroups() {
     // Should only be called once
     debug_assert!(CGROUPS_CPUS.load(Ordering::SeqCst) == 0);
 
-    match load_cgroups("/proc/self/cgroup", "/proc/self/mountinfo") {
-        Some(quota) => {
-            if quota == 0 {
-                return;
-            }
+    if let Some(quota) = load_cgroups("/proc/self/cgroup", "/proc/self/mountinfo") {
+        if quota == 0 {
+            return;
+        }
 
-            let logical = logical_cpus();
-            let count = ::std::cmp::min(quota, logical);
+        let logical = logical_cpus();
+        let count = ::std::cmp::min(quota, logical);
 
-            CGROUPS_CPUS.store(count, Ordering::SeqCst);
-        }
-        None => return,
+        CGROUPS_CPUS.store(count, Ordering::SeqCst);
     }
 }
 
@@ -167,18 +164,14 @@ struct Subsys {
 
 impl Cgroup {
     fn new(dir: PathBuf) -> Cgroup {
-        Cgroup {
-            base: dir,
-        }
+        Cgroup { base: dir }
     }
 
     fn translate(mntinfo: MountInfo, subsys: Subsys) -> Option<Cgroup> {
         // Translate the subsystem directory via the host paths.
         debug!(
             "subsys = {:?}; root = {:?}; mount_point = {:?}",
-            subsys.base,
-            mntinfo.root,
-            mntinfo.mount_point
+            subsys.base, mntinfo.root, mntinfo.mount_point
         );
 
         let rel_from_root = some!(Path::new(&subsys.base).strip_prefix(&mntinfo.root).ok());
@@ -238,13 +231,27 @@ impl MountInfo {
     fn parse_line(line: String) -> Option<MountInfo> {
         let mut fields = line.split(' ');
 
+        // 7 5 0:6 </> /sys/fs/cgroup/cpu,cpuacct rw,nosuid,nodev,noexec,relatime shared:7 - cgroup cgroup rw,cpu,cpuacct
         let mnt_root = some!(fields.nth(3));
-        let mnt_point = some!(fields.nth(0));
+        // 7 5 0:6 / </sys/fs/cgroup/cpu,cpuacct> rw,nosuid,nodev,noexec,relatime shared:7 - cgroup cgroup rw,cpu,cpuacct
+        let mnt_point = some!(fields.next());
+
+        // Ignore all fields until the separator(-).
+        // Note: there could be zero or more optional fields before hyphen.
+        // See: https://man7.org/linux/man-pages/man5/proc.5.html
+        // 7 5 0:6 / /sys/fs/cgroup/cpu,cpuacct rw,nosuid,nodev,noexec,relatime shared:7 <-> cgroup cgroup rw,cpu,cpuacct
+        // Note: we cannot use `?` here because we need to support Rust 1.13.
+        match fields.find(|&s| s == "-") {
+            Some(_) => {}
+            None => return None,
+        };
 
-        if fields.nth(3) != Some("cgroup") {
+        // 7 5 0:6 / /sys/fs/cgroup/cpu,cpuacct rw,nosuid,nodev,noexec,relatime shared:7 - <cgroup> cgroup rw,cpu,cpuacct
+        if fields.next() != Some("cgroup") {
             return None;
         }
 
+        // 7 5 0:6 / /sys/fs/cgroup/cpu,cpuacct rw,nosuid,nodev,noexec,relatime shared:7 - cgroup cgroup <rw,cpu,cpuacct>
         let super_opts = some!(fields.nth(1));
 
         // We only care about the 'cpu' option
@@ -281,16 +288,18 @@ impl Subsys {
             return None;
         }
 
-        fields.next().map(|path| Subsys { base: path.to_owned() })
+        fields.next().map(|path| Subsys {
+            base: path.to_owned(),
+        })
     }
 }
 
 #[cfg(test)]
 mod tests {
-    use std::path::{Path, PathBuf};
     use super::{Cgroup, MountInfo, Subsys};
+    use std::path::{Path, PathBuf};
 
-
+    // `static_in_const` feature is not stable in Rust 1.13.
     static FIXTURES_PROC: &'static str = "fixtures/cgroups/proc/cgroups";
 
     static FIXTURES_CGROUPS: &'static str = "fixtures/cgroups/cgroups";
@@ -304,12 +313,29 @@ mod tests {
 
     #[test]
     fn test_load_mountinfo() {
+        // test only one optional fields
         let path = join!(FIXTURES_PROC, "mountinfo");
 
         let mnt_info = MountInfo::load_cpu(path).unwrap();
 
         assert_eq!(mnt_info.root, "/");
         assert_eq!(mnt_info.mount_point, "/sys/fs/cgroup/cpu,cpuacct");
+
+        // test zero optional field
+        let path = join!(FIXTURES_PROC, "mountinfo_zero_opt");
+
+        let mnt_info = MountInfo::load_cpu(path).unwrap();
+
+        assert_eq!(mnt_info.root, "/");
+        assert_eq!(mnt_info.mount_point, "/sys/fs/cgroup/cpu,cpuacct");
+
+        // test multi optional fields
+        let path = join!(FIXTURES_PROC, "mountinfo_multi_opt");
+
+        let mnt_info = MountInfo::load_cpu(path).unwrap();
+
+        assert_eq!(mnt_info.root, "/");
+        assert_eq!(mnt_info.mount_point, "/sys/fs/cgroup/cpu,cpuacct");
     }
 
     #[test]
@@ -324,12 +350,7 @@ mod tests {
     #[test]
     fn test_cgroup_mount() {
         let cases = &[
-            (
-                "/",
-                "/sys/fs/cgroup/cpu",
-                "/",
-                Some("/sys/fs/cgroup/cpu"),
-            ),
+            ("/", "/sys/fs/cgroup/cpu", "/", Some("/sys/fs/cgroup/cpu")),
             (
                 "/docker/01abcd",
                 "/sys/fs/cgroup/cpu",
@@ -348,27 +369,10 @@ mod tests {
                 "/docker/01abcd/large",
                 Some("/sys/fs/cgroup/cpu/large"),
             ),
-
             // fails
-
-            (
-                "/docker/01abcd",
-                "/sys/fs/cgroup/cpu",
-                "/",
-                None,
-            ),
-            (
-                "/docker/01abcd",
-                "/sys/fs/cgroup/cpu",
-                "/docker",
-                None,
-            ),
-            (
-                "/docker/01abcd",
-                "/sys/fs/cgroup/cpu",
-                "/elsewhere",
-                None,
-            ),
+            ("/docker/01abcd", "/sys/fs/cgroup/cpu", "/", None),
+            ("/docker/01abcd", "/sys/fs/cgroup/cpu", "/docker", None),
+            ("/docker/01abcd", "/sys/fs/cgroup/cpu", "/elsewhere", None),
             (
                 "/docker/01abcd",
                 "/sys/fs/cgroup/cpu",
@@ -387,7 +391,7 @@ mod tests {
             };
 
             let actual = Cgroup::translate(mnt_info, subsys).map(|c| c.base);
-            let expected = expected.map(|s| PathBuf::from(s));
+            let expected = expected.map(PathBuf::from);
             assert_eq!(actual, expected);
         }
     }