Skip to content
New issue

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

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

Already on GitHub? # to your account

add io priority #2164

Merged
merged 1 commit into from
Jul 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions crates/libcgroups/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ cgroupsv2_devices = ["rbpf", "libbpf-sys", "errno", "libc"]
[dependencies]
nix = "0.26.2"
procfs = "0.15.1"
oci-spec = { version = "~0.6.1", features = ["runtime"] }
oci-spec = { version = "~0.6.2", features = ["runtime"] }
dbus = { version = "0.9.7", optional = true }
fixedbitset = "0.4.2"
serde = { version = "1.0", features = ["derive"] }
Expand All @@ -35,7 +35,7 @@ tracing = { version = "0.1.37", features = ["attributes"]}

[dev-dependencies]
anyhow = "1.0"
oci-spec = { version = "~0.6.1", features = ["proptests", "runtime"] }
oci-spec = { version = "~0.6.2", features = ["proptests", "runtime"] }
quickcheck = "1"
mockall = { version = "0.11.4", features = [] }
clap = "4.1.6"
Expand Down
4 changes: 2 additions & 2 deletions crates/libcontainer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ fastrand = "^2.0.0"
futures = { version = "0.3", features = ["thread-pool"] }
libc = "0.2.147"
nix = "0.26.2"
oci-spec = { version = "~0.6.1", features = ["runtime"] }
oci-spec = { version = "~0.6.2", features = ["runtime"] }
once_cell = "1.18.0"
procfs = "0.15.1"
prctl = "1.0.0"
Expand All @@ -43,7 +43,7 @@ tracing = { version = "0.1.37", features = ["attributes"]}
safe-path = "0.1.0"

[dev-dependencies]
oci-spec = { version = "~0.6.1", features = ["proptests", "runtime"] }
oci-spec = { version = "~0.6.2", features = ["proptests", "runtime"] }
quickcheck = "1"
serial_test = "2.0.0"
tempfile = "3"
Expand Down
17 changes: 17 additions & 0 deletions crates/libcontainer/src/container/init_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,23 @@ impl InitContainerBuilder {
Err(ErrInvalidSpec::AppArmorNotEnabled)?;
}
}

if let Some(io_priority) = process.io_priority() {
let priority = io_priority.priority();
let iop_class_res = serde_json::to_string(&io_priority.class());
match iop_class_res {
Ok(iop_class) => {
if !(0..=7).contains(&priority) {
tracing::error!(?priority, "io priority '{}' not between 0 and 7 (inclusive), class '{}' not in (IO_PRIO_CLASS_RT,IO_PRIO_CLASS_BE,IO_PRIO_CLASS_IDLE)",priority, iop_class);
Err(ErrInvalidSpec::IoPriority)?;
}
}
Err(e) => {
tracing::error!(?priority, ?e, "failed to parse io priority class");
Err(ErrInvalidSpec::IoPriority)?;
}
}
}
}

Ok(())
Expand Down
36 changes: 35 additions & 1 deletion crates/libcontainer/src/container/tenant_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use std::{
str::FromStr,
};

use crate::error::{LibcontainerError, MissingSpecError};
use crate::error::{ErrInvalidSpec, LibcontainerError, MissingSpecError};
use crate::process::args::ContainerType;
use crate::{capabilities::CapabilityExt, container::builder_impl::ContainerBuilderImpl};
use crate::{notify_socket::NotifySocket, rootless::Rootless, tty, utils};
Expand Down Expand Up @@ -188,10 +188,44 @@ impl TenantContainerBuilder {
err
})?;

Self::validate_spec(&spec)?;

spec.canonicalize_rootfs(container.bundle())?;
Ok(spec)
}

fn validate_spec(spec: &Spec) -> Result<(), LibcontainerError> {
let version = spec.version();
if !version.starts_with("1.") {
tracing::error!(
"runtime spec has incompatible version '{}'. Only 1.X.Y is supported",
spec.version()
);
Err(ErrInvalidSpec::UnsupportedVersion)?;
}

if let Some(process) = spec.process() {
if let Some(io_priority) = process.io_priority() {
let priority = io_priority.priority();
let iop_class_res = serde_json::to_string(&io_priority.class());
match iop_class_res {
Ok(iop_class) => {
if !(0..=7).contains(&priority) {
tracing::error!(?priority, "io priority '{}' not between 0 and 7 (inclusive), class '{}' not in (IO_PRIO_CLASS_RT,IO_PRIO_CLASS_BE,IO_PRIO_CLASS_IDLE)",priority, iop_class);
Err(ErrInvalidSpec::IoPriority)?;
}
}
Err(e) => {
tracing::error!(?priority, ?e, "failed to parse io priority class");
Err(ErrInvalidSpec::IoPriority)?;
}
}
}
}

Ok(())
}

fn load_container_state(&self, container_dir: PathBuf) -> Result<Container, LibcontainerError> {
let container = Container::load(container_dir)?;
if !container.can_exec() {
Expand Down
2 changes: 2 additions & 0 deletions crates/libcontainer/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,6 @@ pub enum ErrInvalidSpec {
UnsupportedVersion,
#[error("apparmor is specified but not enabled on this system")]
AppArmorNotEnabled,
#[error("invalid io priority or class.")]
IoPriority,
}
69 changes: 67 additions & 2 deletions crates/libcontainer/src/process/container_init_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use nix::sched::CloneFlags;
use nix::sys::stat::Mode;
use nix::unistd::setsid;
use nix::unistd::{self, Gid, Uid};
use oci_spec::runtime::{LinuxNamespaceType, Spec, User};
use oci_spec::runtime::{IOPriorityClass, LinuxIOPriority, LinuxNamespaceType, Spec, User};
use std::collections::HashMap;
use std::os::unix::io::AsRawFd;
use std::{
Expand Down Expand Up @@ -70,6 +70,8 @@ pub enum InitProcessError {
NotifyListener(#[from] notify_socket::NotifyListenerError),
#[error(transparent)]
Workload(#[from] workload::ExecutorManagerError),
#[error("invalid io priority class: {0}")]
IoPriorityClass(String),
}

type Result<T> = std::result::Result<T, InitProcessError>;
Expand Down Expand Up @@ -346,6 +348,9 @@ pub fn container_init_process(
tracing::error!(?err, "failed to setsid to create a session");
InitProcessError::NixOther(err)
})?;

set_io_priority(syscall.as_ref(), proc.io_priority())?;

// set up tty if specified
if let Some(csocketfd) = args.console_socket {
tty::setup_console(&csocketfd).map_err(|err| {
Expand Down Expand Up @@ -757,6 +762,47 @@ fn set_supplementary_gids(
Ok(())
}

/// set_io_priority set io priority
fn set_io_priority(syscall: &dyn Syscall, io_priority_op: &Option<LinuxIOPriority>) -> Result<()> {
match io_priority_op {
Some(io_priority) => {
let io_prio_class_mapping: HashMap<_, _> = [
(IOPriorityClass::IoprioClassRt, 1i64),
(IOPriorityClass::IoprioClassBe, 2i64),
(IOPriorityClass::IoprioClassIdle, 3i64),
]
.iter()
.filter_map(|(class, num)| match serde_json::to_string(&class) {
Ok(class_str) => Some((class_str, *num)),
Err(err) => {
tracing::error!(?err, "failed to parse io priority class");
None
}
})
.collect();

let iop_class = serde_json::to_string(&io_priority.class())
.map_err(|err| InitProcessError::IoPriorityClass(err.to_string()))?;

match io_prio_class_mapping.get(&iop_class) {
Some(value) => {
syscall
.set_io_priority(*value, io_priority.priority())
.map_err(|err| {
tracing::error!(?err, ?io_priority, "failed to set io_priority");
InitProcessError::SyscallOther(err)
})?;
}
None => {
return Err(InitProcessError::IoPriorityClass(iop_class));
}
}
}
None => {}
}
Ok(())
}

#[cfg(feature = "libseccomp")]
fn sync_seccomp(
fd: Option<i32>,
Expand Down Expand Up @@ -789,7 +835,7 @@ mod tests {
use super::*;
use crate::syscall::{
syscall::create_syscall,
test::{ArgName, MountArgs, TestHelperSyscall},
test::{ArgName, IoPriorityArgs, MountArgs, TestHelperSyscall},
};
use anyhow::Result;
#[cfg(feature = "libseccomp")]
Expand Down Expand Up @@ -1079,4 +1125,23 @@ mod tests {
assert!(!is_executable(&non_executable_path).unwrap());
assert!(!is_executable(directory_path).unwrap());
}

#[test]
fn test_set_io_priority() {
let test_command = TestHelperSyscall::default();
let io_priority_op = None;
assert!(set_io_priority(&test_command, &io_priority_op).is_ok());

let data = "{\"class\":\"IOPRIO_CLASS_RT\",\"priority\":1}";
let iop: LinuxIOPriority = serde_json::from_str(data).unwrap();
let io_priority_op = Some(iop);
assert!(set_io_priority(&test_command, &io_priority_op).is_ok());

let want_io_priority = IoPriorityArgs {
class: 1,
priority: 1,
};
let set_io_prioritys = test_command.get_io_priority_args();
assert_eq!(set_io_prioritys[0], want_io_priority);
}
}
19 changes: 19 additions & 0 deletions crates/libcontainer/src/syscall/linux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,25 @@ impl Syscall for LinuxSyscall {
}?;
Ok(())
}

fn set_io_priority(&self, class: i64, priority: i64) -> Result<()> {
let ioprio_who_progress: libc::c_int = 1;
let ioprio_who_pid = 0;
let iop = (class << 13) | priority;
match unsafe {
libc::syscall(
libc::SYS_ioprio_set,
ioprio_who_progress,
ioprio_who_pid,
iop as libc::c_ulong,
)
} {
0 => Ok(()),
-1 => Err(nix::Error::last()),
_ => Err(nix::Error::UnknownErrno),
}?;
Ok(())
}
}

#[cfg(test)]
Expand Down
1 change: 1 addition & 0 deletions crates/libcontainer/src/syscall/syscall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ pub trait Syscall {
mount_attr: &MountAttr,
size: libc::size_t,
) -> Result<()>;
fn set_io_priority(&self, class: i64, priority: i64) -> Result<()>;
}

#[derive(Clone, Copy)]
Expand Down
24 changes: 24 additions & 0 deletions crates/libcontainer/src/syscall/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ pub struct ChownArgs {
pub group: Option<Gid>,
}

#[derive(Clone, PartialEq, Eq, Debug)]
pub struct IoPriorityArgs {
pub class: i64,
pub priority: i64,
}

#[derive(Default)]
struct Mock {
values: Vec<Box<dyn Any>>,
Expand All @@ -62,6 +68,7 @@ pub enum ArgName {
Domainname,
Groups,
Capability,
IoPriority,
}

impl ArgName {
Expand All @@ -77,6 +84,7 @@ impl ArgName {
ArgName::Domainname,
ArgName::Groups,
ArgName::Capability,
ArgName::IoPriority,
]
.iter()
.copied()
Expand Down Expand Up @@ -249,6 +257,13 @@ impl Syscall for TestHelperSyscall {
) -> Result<()> {
todo!()
}

fn set_io_priority(&self, class: i64, priority: i64) -> Result<()> {
self.mocks.act(
ArgName::IoPriority,
Box::new(IoPriorityArgs { class, priority }),
)
}
}

impl TestHelperSyscall {
Expand Down Expand Up @@ -350,4 +365,13 @@ impl TestHelperSyscall {
.map(|x| x.downcast_ref::<Vec<Gid>>().unwrap().clone())
.collect::<Vec<Vec<Gid>>>()
}

pub fn get_io_priority_args(&self) -> Vec<IoPriorityArgs> {
self.mocks
.fetch(ArgName::IoPriority)
.values
.iter()
.map(|x| x.downcast_ref::<IoPriorityArgs>().unwrap().clone())
.collect::<Vec<IoPriorityArgs>>()
}
}