Repository: opencontainers/runc
Pull request #3783 (2023-03-24 12:37:53)

Add I/O priority
https://github.com/opencontainers/runtime-spec/pull/1191

AkihiroSuda: Could you squash the commits?
utam0k: @AkihiroSuda Sure. Done.
AkihiroSuda: Needs rebase
kolyshkin: @utam0k can you please rebase?
utam0k: @AkihiroSuda @kolyshkin 👍 Done
lifubang: @utam0k can you please rebase?
lifubang: Another question, shall we need to add a support to update the IO priority value for a started container in `runc update`?
utam0k: > Another question, shall we need to add a support to update the IO priority value for a started container in `runc update`?

hmm... At least, OCI Spec Runtime doesn't specify about it. WDYT?
lifubang: Please squash your commits to one, thanks.
lifubang: Sorry, please rebase and squash your commits to one, the CI will be green.
utam0k: Sure
utam0k: @lifubang Done
lifubang: @kolyshkin @AkihiroSuda PTAL, Merging is blocked. I think this one can be merged now.
AkihiroSuda: Needs rebase
utam0k: @AkihiroSuda Done ✅ 
kolyshkin: > libcontainer/configs/validate/validator_test.go:620:2: expected declaration, found testCases
@utam0k seems there's an error in this file
kolyshkin: @utam0k also you have to do `pull --rebase` and avoid merge commits.
utam0k: @kolyshkin Thanks, done
AkihiroSuda: ` Error: File is not `gofumpt`-ed (gofumpt)`
https://github.com/opencontainers/runc/actions/runs/6533097503/job/17737584357?pr=3783
lifubang: This PR can be merged once you do a rebase and resolve the conflicts, and I can take over this PR if you are in busy status.
kolyshkin: @utam0k please avoid merge commits -- do a rebase instead
utam0k: > @utam0k please avoid merge commits -- do a rebase instead

Done ✅ 
AkihiroSuda: @kolyshkin PTAL
lifubang: @utam0k Please help to do a rebase to main and solve the conflicts. Thanks.

Patch:
diff --git a/docs/spec-conformance.md b/docs/spec-conformance.md
index b4f3b9df..21c85382 100644
--- a/docs/spec-conformance.md
+++ b/docs/spec-conformance.md
@@ -8,7 +8,6 @@ The following features are not implemented yet:
 Spec version | Feature                                        | PR
 -------------|------------------------------------------------|----------------------------------------------------------
 v1.1.0       | `SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV`       | [#3862](https://github.com/opencontainers/runc/pull/3862)
-v1.1.0       | `.process.ioPriority`                          | [#3783](https://github.com/opencontainers/runc/pull/3783)
 
 ## Architectures
 
diff --git a/libcontainer/configs/config.go b/libcontainer/configs/config.go
index a0a79d19..22fe0f9b 100644
--- a/libcontainer/configs/config.go
+++ b/libcontainer/configs/config.go
@@ -222,6 +222,9 @@ type Config struct {
 
 	// Personality contains configuration for the Linux personality syscall.
 	Personality *LinuxPersonality `json:"personality,omitempty"`
+
+	// IOPriority is the container's I/O priority.
+	IOPriority *IOPriority `json:"io_priority,omitempty"`
 }
 
 // Scheduler is based on the Linux sched_setattr(2) syscall.
@@ -283,6 +286,14 @@ func ToSchedAttr(scheduler *Scheduler) (*unix.SchedAttr, error) {
 	}, nil
 }
 
+var IOPrioClassMapping = map[specs.IOPriorityClass]int{
+	specs.IOPRIO_CLASS_RT:   1,
+	specs.IOPRIO_CLASS_BE:   2,
+	specs.IOPRIO_CLASS_IDLE: 3,
+}
+
+type IOPriority = specs.LinuxIOPriority
+
 type (
 	HookName string
 	HookList []Hook
diff --git a/libcontainer/configs/validate/validator.go b/libcontainer/configs/validate/validator.go
index ed63e950..37ece0ae 100644
--- a/libcontainer/configs/validate/validator.go
+++ b/libcontainer/configs/validate/validator.go
@@ -32,6 +32,7 @@ func Validate(config *configs.Config) error {
 		rootlessEUIDCheck,
 		mountsStrict,
 		scheduler,
+		ioPriority,
 	}
 	for _, c := range checks {
 		if err := c(config); err != nil {
@@ -396,3 +397,14 @@ func scheduler(config *configs.Config) error {
 	}
 	return nil
 }
+
+func ioPriority(config *configs.Config) error {
+	if config.IOPriority == nil {
+		return nil
+	}
+	priority := config.IOPriority.Priority
+	if priority < 0 || priority > 7 {
+		return fmt.Errorf("invalid ioPriority.Priority: %d", priority)
+	}
+	return nil
+}
diff --git a/libcontainer/process.go b/libcontainer/process.go
index 8181062a..3663c7e0 100644
--- a/libcontainer/process.go
+++ b/libcontainer/process.go
@@ -100,6 +100,8 @@ type Process struct {
 	SubCgroupPaths map[string]string
 
 	Scheduler *configs.Scheduler
+
+	IOPriority *configs.IOPriority
 }
 
 // Wait waits for the process to exit.
diff --git a/libcontainer/process_linux.go b/libcontainer/process_linux.go
index ba916571..6d51eada 100644
--- a/libcontainer/process_linux.go
+++ b/libcontainer/process_linux.go
@@ -124,6 +124,13 @@ func (p *setnsProcess) signal(sig os.Signal) error {
 
 func (p *setnsProcess) start() (retErr error) {
 	defer p.comm.closeParent()
+
+	if p.process.IOPriority != nil {
+		if err := setIOPriority(p.process.IOPriority); err != nil {
+			return err
+		}
+	}
+
 	// get the "before" value of oom kill count
 	oom, _ := p.manager.OOMKillCount()
 	err := p.cmd.Start()
@@ -972,3 +979,21 @@ func initWaiter(r io.Reader) chan error {
 
 	return ch
 }
+
+func setIOPriority(ioprio *configs.IOPriority) error {
+	const ioprioWhoPgrp = 1
+
+	class, ok := configs.IOPrioClassMapping[ioprio.Class]
+	if !ok {
+		return fmt.Errorf("invalid io priority class: %s", ioprio.Class)
+	}
+
+	// Combine class and priority into a single value
+	// https://github.com/torvalds/linux/blob/v5.18/include/uapi/linux/ioprio.h#L5-L17
+	iop := (class << 13) | ioprio.Priority
+	_, _, errno := unix.RawSyscall(unix.SYS_IOPRIO_SET, ioprioWhoPgrp, 0, uintptr(iop))
+	if errno != 0 {
+		return fmt.Errorf("failed to set io priority: %w", errno)
+	}
+	return nil
+}
diff --git a/libcontainer/specconv/spec_linux.go b/libcontainer/specconv/spec_linux.go
index 25ef7227..5a09f74b 100644
--- a/libcontainer/specconv/spec_linux.go
+++ b/libcontainer/specconv/spec_linux.go
@@ -534,6 +534,11 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) {
 			s := *spec.Process.Scheduler
 			config.Scheduler = &s
 		}
+
+		if spec.Process.IOPriority != nil {
+			ioPriority := *spec.Process.IOPriority
+			config.IOPriority = &ioPriority
+		}
 	}
 	createHooks(spec, config)
 	config.Version = specs.Version
diff --git a/libcontainer/standard_init_linux.go b/libcontainer/standard_init_linux.go
index c69e4e31..4447032c 100644
--- a/libcontainer/standard_init_linux.go
+++ b/libcontainer/standard_init_linux.go
@@ -161,6 +161,11 @@ func (l *linuxStandardInit) Init() error {
 			return err
 		}
 	}
+	if l.config.Config.IOPriority != nil {
+		if err := setIOPriority(l.config.Config.IOPriority); err != nil {
+			return err
+		}
+	}
 
 	// Tell our parent that we're ready to Execv. This must be done before the
 	// Seccomp rules have been applied, because we need to be able to read and
diff --git a/utils_linux.go b/utils_linux.go
index e7b362cd..a59301c1 100644
--- a/utils_linux.go
+++ b/utils_linux.go
@@ -67,6 +67,11 @@ func newProcess(p specs.Process) (*libcontainer.Process, error) {
 		lp.Scheduler = &s
 	}
 
+	if p.IOPriority != nil {
+		ioPriority := *p.IOPriority
+		lp.IOPriority = &ioPriority
+	}
+
 	if p.Capabilities != nil {
 		lp.Capabilities = &configs.Capabilities{}
 		lp.Capabilities.Bounding = p.Capabilities.Bounding


Test patch:
diff --git a/libcontainer/configs/validate/validator_test.go b/libcontainer/configs/validate/validator_test.go
index 85b5e5ee..b0b740a1 100644
--- a/libcontainer/configs/validate/validator_test.go
+++ b/libcontainer/configs/validate/validator_test.go
@@ -842,3 +842,32 @@ func TestValidateScheduler(t *testing.T) {
 		}
 	}
 }
+
+func TestValidateIOPriority(t *testing.T) {
+	testCases := []struct {
+		isErr    bool
+		priority int
+	}{
+		{isErr: false, priority: 0},
+		{isErr: false, priority: 7},
+		{isErr: true, priority: -1},
+	}
+
+	for _, tc := range testCases {
+		ioPriroty := configs.IOPriority{
+			Priority: tc.priority,
+		}
+		config := &configs.Config{
+			Rootfs:     "/var",
+			IOPriority: &ioPriroty,
+		}
+
+		err := Validate(config)
+		if tc.isErr && err == nil {
+			t.Errorf("iopriority: %d, expected error, got nil", tc.priority)
+		}
+		if !tc.isErr && err != nil {
+			t.Errorf("iopriority: %d, expected nil, got error %v", tc.priority, err)
+		}
+	}
+}
diff --git a/tests/integration/ioprio.bats b/tests/integration/ioprio.bats
new file mode 100644
index 00000000..a907d782
--- /dev/null
+++ b/tests/integration/ioprio.bats
@@ -0,0 +1,30 @@
+#!/usr/bin/env bats
+
+load helpers
+
+function setup() {
+	setup_debian
+}
+
+function teardown() {
+	teardown_bundle
+}
+
+@test "ioprio_set is applied to process group" {
+	# Create a container with a specific I/O priority.
+	update_config '.process.ioPriority = {"class": "IOPRIO_CLASS_BE", "priority": 4}'
+
+	runc run -d --console-socket "$CONSOLE_SOCKET" test_ioprio
+	[ "$status" -eq 0 ]
+
+	# Check the init process.
+	runc exec test_ioprio ionice -p 1
+	[ "$status" -eq 0 ]
+	[[ "$output" = *'best-effort: prio 4'* ]]
+
+	# Check the process made from the exec command.
+	runc exec test_ioprio ionice
+	[ "$status" -eq 0 ]
+
+	[[ "$output" = *'best-effort: prio 4'* ]]
+}