From fe82973b9f3e2a53af2183816c39ed225c12a022 Mon Sep 17 00:00:00 2001 From: modeco80 Date: Sat, 2 Nov 2024 18:01:07 -0400 Subject: [PATCH] Only try enabling cpuset controller if required Additionally, "handle" (in many airquotes) errors trying to set controller values. --- config.example.toml | 19 ++++++++++------- cvmts/src/util/cgroup.ts | 27 +++++++++++++++++------- cvmts/src/vm/qemu_launcher.ts | 39 ++++++++++++++++++++++++----------- 3 files changed, 58 insertions(+), 27 deletions(-) diff --git a/config.example.toml b/config.example.toml index 48098d1..b7d8d48 100644 --- a/config.example.toml +++ b/config.example.toml @@ -57,17 +57,20 @@ qemuArgs = "qemu-system-x86_64" vncPort = 5900 snapshots = true -# Resource limits. Only works on Linux, with `Delegate=yes` set in your .service file. +# Resource limits. Only works on Linux, with `Delegate=yes` set in your .service file. No-op on other platforms. +# +# cpuUsageMax optionally specifies the max CPU usage as percentage in the common top notation, so 200% means 2 CPUs, 400% is 4 CPUs, +# so on. +# +# runOnCpus is an optional array that specifies what CPUs the VM is allowed to run on. +# Systemd user slices can not delegate the cpuset controller, meaning this option will *not* work if you do not use a system service. +# (effectively, it will be a loud no-op that logs an error on startup) # -# cpuUsageMax specifies CPU usage limits in the common top notation, so 200% means 2 CPUs, so on so forth. -# runOnCpus specifies what CPUs the VM is allowed to run on. # limitProcess is optional (default false) and determines if only qemu vCPU threads are put into the cgroup, -# or the entire QEMU process (incl. all its threads). This is rarely what you want, so the example does not -# provide it. +# or the entire QEMU process (incl. all its threads). The default behaviour of only limiting vCPU threads +# is more than likely what you want, so the example configuration omits specifying this key. # -# Either can be omitted or specified; however, if you want to disable it entirely, -# it's a better idea to just comment this inline table out, -# since the inline table existing is used to enable cgroup support. +# Commenting this inline table from the configuration disables resource limiting entirely. resourceLimits = { cpuUsageMax = 100, runOnCpus = [ 2, 4 ] } diff --git a/cvmts/src/util/cgroup.ts b/cvmts/src/util/cgroup.ts index 6f0bb00..44783ce 100644 --- a/cvmts/src/util/cgroup.ts +++ b/cvmts/src/util/cgroup.ts @@ -3,6 +3,9 @@ import { appendFileSync, existsSync, mkdirSync, readFileSync, rmdirSync, writeFileSync } from 'node:fs'; import path from 'node:path'; +import pino from 'pino'; + +let logger = pino({ name: 'CVMTS/CGroup' }); export class CGroupController { private controller; @@ -14,7 +17,11 @@ export class CGroupController { } WriteValue(key: string, value: string) { - writeFileSync(path.join(this.cg.Path(), `${this.controller}.${key}`), value); + try { + writeFileSync(path.join(this.cg.Path(), `${this.controller}.${key}`), value); + } catch(e) { + logger.error({error: e, controller_key: `${this.controller}.${key}`, value: value }, 'Failed to set CGroup controller value') + } } } @@ -25,11 +32,20 @@ export class CGroup { this.path = path; } - private InitControllers() { + InitControllers(wants_cpuset: boolean) { // Configure this "root" cgroup to provide cpu and cpuset controllers to the leaf // QEMU cgroups. A bit iffy but whatever. - writeFileSync(path.join(this.path, 'cgroup.subtree_control'), '+cpu +cpuset'); - //writeFileSync(path.join(this.path, 'cgroup.subtree_control'), '+cpu'); + if(wants_cpuset) { + try { + writeFileSync(path.join(this.path, 'cgroup.subtree_control'), '+cpu +cpuset'); + } catch(err) { + logger.error({error: err}, 'Could not provide cpuset controller to subtree'); + // just give up if this fails + writeFileSync(path.join(this.path, 'cgroup.subtree_control'), '+cpu'); + } + } else { + writeFileSync(path.join(this.path, 'cgroup.subtree_control'), '+cpu'); + } } GetController(controller: string) { @@ -95,9 +111,6 @@ export class CGroup { let cg = new CGroup(path.join('/sys/fs/cgroup', cg_path)); - // Do root level group initalization - cg.InitControllers(); - return cg; } } diff --git a/cvmts/src/vm/qemu_launcher.ts b/cvmts/src/vm/qemu_launcher.ts index d1c86d6..a0ff2d8 100644 --- a/cvmts/src/vm/qemu_launcher.ts +++ b/cvmts/src/vm/qemu_launcher.ts @@ -47,14 +47,14 @@ class CGroupLimitedProcess extends EventEmitter implements IProcess { stdin: Writable | null = null; stdout: Readable | null = null; stderr: Readable | null = null; - private cgroup_root: CGroup; + private root_cgroup: CGroup; private cgroup: CGroup; private id; private limits; constructor(cgroup_root: CGroup, id: string, limits: CgroupLimits, command: string, opts?: ProcessLaunchOptions) { super(); - this.cgroup_root = cgroup_root; + this.root_cgroup = cgroup_root; this.cgroup = cgroup_root.GetSubgroup(id); this.id = id; this.limits = limits; @@ -70,6 +70,8 @@ class CGroupLimitedProcess extends EventEmitter implements IProcess { let self = this; this.process.on('spawn', () => { + self.initCgroup(); + if(self.limits.limitProcess) { // it should have one! self.cgroup.AttachProcess(self.process.pid!); @@ -82,6 +84,14 @@ class CGroupLimitedProcess extends EventEmitter implements IProcess { }); } + initCgroup() { + // Set cgroup keys. + for(const val of MakeValuesFromLimits(this.limits)) { + let controller = this.cgroup.GetController(val.controller); + controller.WriteValue(val.key, val.value); + } + } + kill(signal?: number | NodeJS.Signals): boolean { return this.process.kill(signal); } @@ -91,31 +101,36 @@ class CGroupLimitedProcess extends EventEmitter implements IProcess { this.stdout = null; this.stderr = null; - this.cgroup_root.DeleteSubgroup(this.id); + this.root_cgroup.DeleteSubgroup(this.id); this.process.removeAllListeners(); this.removeAllListeners(); } } export class QemuResourceLimitedLauncher implements IProcessLauncher { - public group; private limits; private name; + private root; + public group; constructor(name: string, limits: CgroupLimits) { - let root = CGroup.Self(); + this.root = CGroup.Self(); + + // Make sure + if(limits.runOnCpus) { + this.root.InitControllers(true); + } else { + this.root.InitControllers(false); + } + this.name = name; - this.group = root.GetSubgroup(name); this.limits = limits; - // Set cgroup keys. - for(const val of MakeValuesFromLimits(limits)) { - let controller = this.group.GetController(val.controller); - controller.WriteValue(val.key, val.value); - } + // XXX figure something better out + this.group = this.root.GetSubgroup(this.name); } launch(command: string, opts?: ProcessLaunchOptions | undefined): IProcess { - return new CGroupLimitedProcess(CGroup.Self(), this.name, this.limits, command, opts); + return new CGroupLimitedProcess(this.root, this.name, this.limits, command, opts); } }