Skip to content

systemd: Do not check systemd version #154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 8, 2025
+16 −92
Merged

systemd: Do not check systemd version #154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 2 additions & 13 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,22 +72,11 @@ pub mod tests {
child
}

pub fn systemd_version() -> Option<usize> {
pub fn systemd_version() -> Option<String> {
let output = Command::new("systemd").arg("--version").output().ok()?; // Return None if command execution fails

if !output.status.success() {
return None;
}

let stdout = String::from_utf8_lossy(&output.stdout);

// The first line is typically like "systemd 254 (254.5-1-arch)"
let first_line = stdout.lines().next()?;
let mut words = first_line.split_whitespace();

words.next()?; // Skip the "systemd" word
let version_str = words.next()?; // The version number as string

version_str.parse::<usize>().ok()
Some(String::from_utf8_lossy(&output.stdout).to_string())
}
}
26 changes: 7 additions & 19 deletions src/manager/systemd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,31 +91,21 @@ impl SystemdManager<'_> {
&self.unit
}

fn set_cpuset(
&self,
props: &mut Vec<Property>,
linux_cpu: &LinuxCpu,
systemd_version: usize,
) -> Result<()> {
fn set_cpuset(&self, props: &mut Vec<Property>, linux_cpu: &LinuxCpu) -> Result<()> {
if let Some(cpus) = linux_cpu.cpus().as_ref() {
let (id, value) = cpuset::cpus(cpus, systemd_version)?;
let (id, value) = cpuset::cpus(cpus)?;
props.push((id, value.into()));
}

if let Some(mems) = linux_cpu.mems().as_ref() {
let (id, value) = cpuset::mems(mems, systemd_version)?;
let (id, value) = cpuset::mems(mems)?;
props.push((id, value.into()));
}

Ok(())
}

fn set_cpu(
&self,
props: &mut Vec<Property>,
linux_cpu: &LinuxCpu,
systemd_version: usize,
) -> Result<()> {
fn set_cpu(&self, props: &mut Vec<Property>, linux_cpu: &LinuxCpu) -> Result<()> {
if let Some(shares) = linux_cpu.shares() {
let shares = if self.v2() {
conv::cpu_shares_to_cgroup_v2(shares)
Expand All @@ -130,7 +120,7 @@ impl SystemdManager<'_> {
let quota = linux_cpu.quota().unwrap_or(0);

if period != 0 {
let (id, value) = cpu::period(period, systemd_version)?;
let (id, value) = cpu::period(period)?;
props.push((id, value.into()));
}

Expand Down Expand Up @@ -272,11 +262,9 @@ impl Manager for SystemdManager<'_> {
fn set(&mut self, resources: &LinuxResources) -> Result<()> {
let mut props = vec![];

let systemd_version = self.systemd_client.systemd_version()?;

if let Some(linux_cpu) = resources.cpu() {
self.set_cpuset(&mut props, linux_cpu, systemd_version)?;
self.set_cpu(&mut props, linux_cpu, systemd_version)?;
self.set_cpuset(&mut props, linux_cpu)?;
self.set_cpu(&mut props, linux_cpu)?;
}

if let Some(linux_memory) = resources.memory() {
Expand Down
12 changes: 3 additions & 9 deletions src/systemd/cpu.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@
// SPDX-License-Identifier: Apache-2.0 or MIT
//

use crate::systemd::error::{Error, Result};
use crate::systemd::{
CPU_QUOTA_PERIOD_US, CPU_QUOTA_PER_SEC_US, CPU_SHARES, CPU_SYSTEMD_VERSION, CPU_WEIGHT,
};
use crate::systemd::error::Result;
use crate::systemd::{CPU_QUOTA_PERIOD_US, CPU_QUOTA_PER_SEC_US, CPU_SHARES, CPU_WEIGHT};

/// Returns the property for CPU shares.
///
Expand All @@ -21,11 +19,7 @@ pub fn shares(shares: u64, v2: bool) -> Result<(&'static str, u64)> {
}

/// Returns the property for CPU period.
pub fn period(period: u64, systemd_version: usize) -> Result<(&'static str, u64)> {
if systemd_version < CPU_SYSTEMD_VERSION {
return Err(Error::ObsoleteSystemd);
}

pub fn period(period: u64) -> Result<(&'static str, u64)> {
Ok((CPU_QUOTA_PERIOD_US, period))
}

Expand Down
16 changes: 3 additions & 13 deletions src/systemd/cpuset.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,29 +6,19 @@
use bit_vec::BitVec;

use crate::systemd::error::{Error, Result};
use crate::systemd::{ALLOWED_CPUS, ALLOWED_MEMORY_NODES, CPUSET_SYSTEMD_VERSION};
use crate::systemd::{ALLOWED_CPUS, ALLOWED_MEMORY_NODES};

const BYTE_IN_BITS: usize = 8;

/// Returns the property for cpuset CPUs.
pub fn cpus(cpus: &str, systemd_version: usize) -> Result<(&'static str, Vec<u8>)> {
if systemd_version < CPUSET_SYSTEMD_VERSION {
return Err(Error::ObsoleteSystemd);
}

pub fn cpus(cpus: &str) -> Result<(&'static str, Vec<u8>)> {
let mask = convert_list_to_mask(cpus)?;

Ok((ALLOWED_CPUS, mask))
}

/// Returns the property for cpuset memory nodes.
pub fn mems(mems: &str, systemd_version: usize) -> Result<(&'static str, Vec<u8>)> {
if systemd_version < CPUSET_SYSTEMD_VERSION {
return Err(Error::ObsoleteSystemd);
}

pub fn mems(mems: &str) -> Result<(&'static str, Vec<u8>)> {
let mask = convert_list_to_mask(mems)?;

Ok((ALLOWED_MEMORY_NODES, mask))
}

Expand Down
30 changes: 1 addition & 29 deletions src/systemd/dbus/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,21 +143,6 @@ impl SystemdClient<'_> {
Ok(())
}

/// Get the systemd version.
pub fn systemd_version(&self) -> Result<usize> {
let sys_proxy = systemd_manager_proxy()?;

// Parse 249 from "249.11-0ubuntu3.16"
let version = sys_proxy.version()?;
let version = version
.split('.')
.next()
.and_then(|v| v.parse::<usize>().ok())
.ok_or(Error::CorruptedSystemdVersion(version))?;

Ok(version)
}

/// Check if the unit exists.
pub fn exists(&self) -> bool {
let sys_proxy = match systemd_manager_proxy() {
Expand Down Expand Up @@ -216,7 +201,7 @@ pub mod tests {
use crate::systemd::props::PropertiesBuilder;
use crate::systemd::utils::expand_slice;
use crate::systemd::{DEFAULT_DESCRIPTION, DESCRIPTION, PIDS};
use crate::tests::{spawn_sleep_inf, spawn_yes, systemd_version};
use crate::tests::{spawn_sleep_inf, spawn_yes};

const TEST_SLICE: &str = "cgroupsrs-test.slice";

Expand Down Expand Up @@ -504,19 +489,6 @@ pub mod tests {
child.wait().unwrap();
}

#[test]
fn test_systemd_version() {
skip_if_no_systemd!();

let unit = test_unit();
let props = PropertiesBuilder::default_cgroup(TEST_SLICE, &unit).build();
let cgroup = SystemdClient::new(&unit, props).unwrap();
let version = cgroup.systemd_version().unwrap();

let expected_version = systemd_version().unwrap();
assert_eq!(version, expected_version, "Systemd version mismatch");
}

#[test]
fn test_exists() {
skip_if_no_systemd!();
Expand Down
3 changes: 0 additions & 3 deletions src/systemd/dbus/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,4 @@ pub enum Error {

#[error("dbus error: {0}")]
Dbus(#[from] zbus::Error),

#[error("corrupted systemd version: {0}")]
CorruptedSystemdVersion(String),
}
3 changes: 0 additions & 3 deletions src/systemd/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@ pub enum Error {
#[error("invalid argument")]
InvalidArgument,

#[error("obsolete systemd, please upgrade your systemd")]
ObsoleteSystemd,
Copy link

@pmores pmores Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question - I like the resolution and level of detail of these error messages. The message I initially got was from the CorruptedSystemdVersion error which was specific enough to direct me right at the core problem although I had no previous experience with cgroups-rs at all.

Is there a way to preserve at least some of this? I tried to check and I think (not sure though) that users will now get one of zbus's Error variants. It doesn't seem too bad but if there was a way to turn it into something more specific in the context of cgroups-rs that would be awesome.

Copy link
Member Author

@justxuewei justxuewei Aug 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I prefer not. Most of errors from dbus are MethodError. The first item is OwnedErrorName which is a variant of string IMHO. For example, calling a non-existent method will result in an error with the value org.freedesktop.systemd1.NoSuchUnit. For the common errors, we can parse them, but it is hard to cover them all.

BTW, I think the errors are clear enough even if we don't parse it. I have not tried to trigger an error, but it should be expected as follows.

Caused by:
    0: setup device after start vm
    1: setup cgroups after start vm
    2: update sandbox cgroups after start vm
    3: set
    4: systemd dbus error: dbus: No such unit
    5: dbus: No such unit

Copy link

@pmores pmores Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well dbus: no such unit is pretty much exactly the opaque message that I'm afraid says nothing to anybody who doesn't happen to be very familiar with dbus. But fair enough, it's true that what I'd like to see would require non-trivial effort. Maybe later.

Copy link
Member Author

@justxuewei justxuewei Aug 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the high-prio thing is to use cgroups-rs to manage cgroups for kata-agent. I am fixing the issue of that, but it is expected to take a while. I'll keep track of issues related to cgroups. If I find it slows down the debugging process, I'll take steps to make it clearer.


#[error("resource not supported by cgroups v1")]
CgroupsV1NotSupported,
}
3 changes: 0 additions & 3 deletions src/systemd/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,3 @@ pub const DEFAULT_SLICE: &str = "system.slice";

pub const SLICE_SUFFIX: &str = ".slice";
pub const SCOPE_SUFFIX: &str = ".scope";

pub const CPU_SYSTEMD_VERSION: usize = 242;
pub const CPUSET_SYSTEMD_VERSION: usize = 244;
Loading