[pve-devel] [PATCH qemu-server] fix #5528: override cgroup methods to call systemd via dbus

* [pve-devel] [PATCH qemu-server] fix #5528: override cgroup methods to call systemd via dbus
@ 2024-07-09  9:10 Wolfgang Bumiller
  2024-07-24 12:26 ` [pve-devel] applied: " Thomas Lamprecht
  0 siblings, 1 reply; 2+ messages in thread
From: Wolfgang Bumiller @ 2024-07-09  9:10 UTC (permalink / raw)
  To: pve-devel

Systemd reapplies its known values on reload, so we cannot simply call
into PVE::CGroup. Call systemd's SetUnitProperties method via dbus
instead.

The hotplug and startup code also calculated different values, as one
operated within systemd's value framework (documented in
systemd.resource-control(5)) and one worked with cgroup values
(distinguishing between cgroup v1 and v2 manually).

This is now unified by overriding `change_cpu_quota()` and
`change_cpu_shares()` via `PVE::QemuServer::CGroup` which now takes
systemd-based values and sends those directly via dbus.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 PVE/QemuServer.pm        |  4 ++--
 PVE/QemuServer/CGroup.pm | 51 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 88c274d..6e78cff 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5100,8 +5100,8 @@ sub vmconfig_hotplug_pending {
 		my $new_cpuunits = PVE::CGroup::clamp_cpu_shares($conf->{pending}->{$opt}); #clamp
 		$cgroup->change_cpu_shares($new_cpuunits);
 	    } elsif ($opt eq 'cpulimit') {
-		my $cpulimit = $conf->{pending}->{$opt} == 0 ? -1 : int($conf->{pending}->{$opt} * 100000);
-		$cgroup->change_cpu_quota($cpulimit, 100000);
+		my $cpulimit = $conf->{pending}->{$opt} == 0 ? undef : int($conf->{pending}->{$opt} * 100);
+		$cgroup->change_cpu_quota($cpulimit, undef);
 	    } elsif ($opt eq 'agent') {
 		vmconfig_update_agent($conf, $opt, $value);
 	    } else {
diff --git a/PVE/QemuServer/CGroup.pm b/PVE/QemuServer/CGroup.pm
index 479991e..882163f 100644
--- a/PVE/QemuServer/CGroup.pm
+++ b/PVE/QemuServer/CGroup.pm
@@ -2,7 +2,10 @@ package PVE::QemuServer::CGroup;
 
 use strict;
 use warnings;
-use PVE::CGroup;
+
+use Net::DBus qw(dbus_uint64 dbus_boolean);
+
+use PVE::Systemd;
 use base('PVE::CGroup');
 
 sub get_subdir {
@@ -11,4 +14,50 @@ sub get_subdir {
     return "qemu.slice/$vmid.scope/";
 }
 
+sub scope {
+    my ($self) = @_;
+    return $self->{vmid} . '.scope';
+}
+
+my sub set_unit_properties : prototype($$) {
+    my ($self, $properties) = @_;
+
+    PVE::Systemd::systemd_call(sub {
+	my ($if, $reactor, $finish_cb) = @_;
+	$if->SetUnitProperties($self->scope(), dbus_boolean(1), $properties);
+	return 1;
+    });
+}
+
+# Update the 'cpulimit' of VM.
+# Note that this is now the systemd API and we expect a value for `CPUQuota` as
+# set on VM startup, rather than cgroup values.
+sub change_cpu_quota {
+    my ($self, $quota, $period) = @_;
+
+    die "period is not controlled for VMs\n" if defined($period);
+
+    $quota = dbus_uint64(defined($quota) ? ($quota * 10_000) : -1);
+    set_unit_properties($self, [ [ CPUQuotaPerSecUSec => $quota ] ]);
+
+    return;
+}
+
+# Update the 'cpuunits' of a VM.
+# Note that this is now the systemd API and we expect a value for `CPUQuota` as
+# set on VM startup, rather than cgroup values.
+sub change_cpu_shares {
+    my ($self, $shares) = @_;
+
+    $shares //= -1;
+
+    if (PVE::CGroup::cgroup_mode() == 2) {
+	set_unit_properties($self, [ [ CPUWeight => dbus_uint64($shares) ] ]);
+    } else {
+	set_unit_properties($self, [ [ CPUShares => dbus_uint64($shares) ] ]);
+    }
+
+    return;
+}
+
 1;
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 2+ messages in thread