From ecadb514eafd561d178597d59ba3b12a56620b27 Mon Sep 17 00:00:00 2001 From: Christophe de Dinechin Date: Thu, 30 Mar 2023 16:09:13 +0200 Subject: [PATCH 1/4] rustjail: Do not unwrap potential error with cgroup manager There can be an error while connecting to the cgroups managager, for example a `ENOENT` if a file is not found. Make sure that this is reported through the proper channels instead of causing a `panic()` that does not provide much information. Fixes: #6561 Signed-off-by: Christophe de Dinechin Reported-by: Greg Kurz (cherry picked from commit 41fdda1d84ea72b3ae2dbadc3152346d418fa73f) Signed-off-by: Greg Kurz --- src/agent/rustjail/src/cgroups/systemd/manager.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/agent/rustjail/src/cgroups/systemd/manager.rs b/src/agent/rustjail/src/cgroups/systemd/manager.rs index c52e727e4c..c34aaf9102 100644 --- a/src/agent/rustjail/src/cgroups/systemd/manager.rs +++ b/src/agent/rustjail/src/cgroups/systemd/manager.rs @@ -41,7 +41,7 @@ pub struct Manager { impl CgroupManager for Manager { fn apply(&self, pid: pid_t) -> Result<()> { let unit_name = self.unit_name.as_str(); - if self.dbus_client.unit_exist(unit_name).unwrap() { + if self.dbus_client.unit_exist(unit_name)? { self.dbus_client.add_process(pid, self.unit_name.as_str())?; } else { self.dbus_client.start_unit( From e0e6f94819c6e4c43ce4c73c625287d67684acbd Mon Sep 17 00:00:00 2001 From: Christophe de Dinechin Date: Thu, 30 Mar 2023 16:13:37 +0200 Subject: [PATCH 2/4] rustjail: Fix minor grammatical error in function name Rename `unit_exist` function to `unit_exists` to match English grammar rule. Fixes: #6561 Signed-off-by: Christophe de Dinechin (cherry picked from commit 7796e6ccc64246576b441824a8617cd1607125b4) Signed-off-by: Greg Kurz --- src/agent/rustjail/src/cgroups/systemd/dbus_client.rs | 4 ++-- src/agent/rustjail/src/cgroups/systemd/manager.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/agent/rustjail/src/cgroups/systemd/dbus_client.rs b/src/agent/rustjail/src/cgroups/systemd/dbus_client.rs index 09c46f24d1..fd3b9bf8fc 100644 --- a/src/agent/rustjail/src/cgroups/systemd/dbus_client.rs +++ b/src/agent/rustjail/src/cgroups/systemd/dbus_client.rs @@ -26,7 +26,7 @@ pub trait SystemdInterface { fn get_version(&self) -> Result; - fn unit_exist(&self, unit_name: &str) -> Result; + fn unit_exists(&self, unit_name: &str) -> Result; fn add_process(&self, pid: i32, unit_name: &str) -> Result<()>; } @@ -108,7 +108,7 @@ impl SystemdInterface for DBusClient { Ok(systemd_version) } - fn unit_exist(&self, unit_name: &str) -> Result { + fn unit_exists(&self, unit_name: &str) -> Result { let proxy = self.build_proxy()?; Ok(proxy.get_unit(unit_name).is_ok()) diff --git a/src/agent/rustjail/src/cgroups/systemd/manager.rs b/src/agent/rustjail/src/cgroups/systemd/manager.rs index c34aaf9102..dcbc65a2cf 100644 --- a/src/agent/rustjail/src/cgroups/systemd/manager.rs +++ b/src/agent/rustjail/src/cgroups/systemd/manager.rs @@ -41,7 +41,7 @@ pub struct Manager { impl CgroupManager for Manager { fn apply(&self, pid: pid_t) -> Result<()> { let unit_name = self.unit_name.as_str(); - if self.dbus_client.unit_exist(unit_name)? { + if self.dbus_client.unit_exists(unit_name)? { self.dbus_client.add_process(pid, self.unit_name.as_str())?; } else { self.dbus_client.start_unit( From f83adbe83d317d26c9b6d4857f54730325da329c Mon Sep 17 00:00:00 2001 From: Christophe de Dinechin Date: Fri, 31 Mar 2023 10:49:24 +0200 Subject: [PATCH 3/4] rustjail: Add anyhow context for D-Bus connections In cases where the D-Bus connection fails, add a little additional context about the origin of the error. Fixes: 6561 Signed-off-by: Christophe de Dinechin Suggested-by: Archana Shinde Spell-checked-by: Greg Kurz (cherry picked from commit b661e0cf3f27f59ec91670e1e45aaf5aa2191e9b) Signed-off-by: Greg Kurz --- src/agent/rustjail/src/cgroups/systemd/dbus_client.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/agent/rustjail/src/cgroups/systemd/dbus_client.rs b/src/agent/rustjail/src/cgroups/systemd/dbus_client.rs index fd3b9bf8fc..0ff606930a 100644 --- a/src/agent/rustjail/src/cgroups/systemd/dbus_client.rs +++ b/src/agent/rustjail/src/cgroups/systemd/dbus_client.rs @@ -36,8 +36,9 @@ pub struct DBusClient {} impl DBusClient { fn build_proxy(&self) -> Result> { - let connection = zbus::blocking::Connection::system()?; - let proxy = SystemManager::new(&connection)?; + let connection = + zbus::blocking::Connection::system().context("Establishing a D-Bus connection")?; + let proxy = SystemManager::new(&connection).context("Building a D-Bus proxy manager")?; Ok(proxy) } } @@ -109,7 +110,9 @@ impl SystemdInterface for DBusClient { } fn unit_exists(&self, unit_name: &str) -> Result { - let proxy = self.build_proxy()?; + let proxy = self + .build_proxy() + .with_context(|| format!("Checking if systemd unit {} exists", unit_name))?; Ok(proxy.get_unit(unit_name).is_ok()) } From 8b597195aba827d8af5e345e12e6ee5ebbb7e3c5 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 7 Apr 2023 17:44:41 +0200 Subject: [PATCH 4/4] rustjail: Use CPUWeight with systemd and CgroupsV2 The CPU shares property belongs to CgroupsV1. CgroupsV2 uses CPU weight instead. The correct value is computed in the latter case but it is passed to systemd using the legacy property. Systemd rejects the request and the agent exists with the following error : Value specified in CPUShares is out of range: unknown Replace the "shares" wording with "weight" in the CgroupsV2 code to avoid confusions. Use the "CPUWeight" property since this is what systemd expects in this case. Fixes #6636 References: https://www.freedesktop.org/software/systemd/man/systemd.resource-control.html#CPUWeight=weight https://www.freedesktop.org/software/systemd/man/systemd.resource-control.html#systemd%20252 https://github.com/containers/crun/blob/main/crun.1.md#cpu-controller Signed-off-by: Greg Kurz (cherry picked from commit c1fbaae8d6c72cb64e8a4681012cac9c3f5ccdf1) Signed-off-by: Greg Kurz --- src/agent/rustjail/src/cgroups/systemd/subsystem/cpu.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/agent/rustjail/src/cgroups/systemd/subsystem/cpu.rs b/src/agent/rustjail/src/cgroups/systemd/subsystem/cpu.rs index 6735b4d3ca..7f7667fcd1 100644 --- a/src/agent/rustjail/src/cgroups/systemd/subsystem/cpu.rs +++ b/src/agent/rustjail/src/cgroups/systemd/subsystem/cpu.rs @@ -71,7 +71,7 @@ impl Cpu { } // v2: - // cpu.shares <-> CPUShares + // cpu.shares <-> CPUWeight // cpu.period <-> CPUQuotaPeriodUSec // cpu.period & cpu.quota <-> CPUQuotaPerSecUSec fn unified_apply( @@ -80,8 +80,8 @@ impl Cpu { systemd_version: &str, ) -> Result<()> { if let Some(shares) = cpu_resources.shares { - let unified_shares = get_unified_cpushares(shares); - properties.push(("CPUShares", Value::U64(unified_shares))); + let weight = shares_to_weight(shares); + properties.push(("CPUWeight", Value::U64(weight))); } if let Some(period) = cpu_resources.period { @@ -104,7 +104,7 @@ impl Cpu { // ref: https://github.com/containers/crun/blob/main/crun.1.md#cgroup-v2 // [2-262144] to [1-10000] -fn get_unified_cpushares(shares: u64) -> u64 { +fn shares_to_weight(shares: u64) -> u64 { if shares == 0 { return 100; }