Only try enabling cpuset controller if required

Additionally, "handle" (in many airquotes) errors trying to set controller values.

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 ] }
 
 

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;
 	}
 }

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);
 	}
 }