[pve-devel] [PATCH-SERIES docs/(guest-)common/qemu-server/container/manager] cpuunits improvements

[pve-devel] [PATCH-SERIES docs/(guest-)common/qemu-server/container/manager] cpuunits improvements

[Fiona Ebner]

This series does multiple things, all related to cpuunits:

* Updates the documentation in pve-docs and pve-container for the
cgroup v2 cpuunits defaults and limits, while still mentioning the old
values that apply to legacy cgroup v1 hosts. qemu-server is already
updated.

* Move the get_cpuunits() helper from qemu-server to guest-common and
re-use it for containers. Makes behavior consistent with VMs and
allows starting existing containers with an out-of-range value. This
requires dependency bumps qemu-server,pve-container -> guest-common.

* Drop an unused parameter for change_cpu_shares(). Here technically,
a Breaks from common -> pve-container,qemu-server is needed, before
we can introduce a new parameter.

* Clamp cpuunits value already when setting via API.

* Broadcast the node's cgroup version as a pmxcfs kv entry. This is
the v2 of that patch, changes are noted there and use the value in
the UI to dynamically show the correct defaults and limits.


docs:

Fiona Ebner (2):
  qm: resource limits: update systemd setting name for cpuunits
  pct/qm: update cpuunits default

 pct.adoc |  5 +++--
 qm.adoc  | 14 +++++++-------
 2 files changed, 10 insertions(+), 9 deletions(-)


guest-common

Fiona Ebner (1):
  helpers: move get_cpuunits helper from qemu-server

 src/PVE/GuestHelpers.pm | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)


container:

Fiona Ebner (7):
  config: cpuunits: drop description for outdated special value
  change cpu shares: hard-code cgroupv1 default parameter
  config: cpuunits: update default description
  use helper from guest-common for cpuunits
  config: replace mentions of "VM" with "container"
  change cpu shares: drop superfluous parameter
  api: create/update vm: clamp cpuunit value

 src/PVE/API2/LXC.pm        |  3 +++
 src/PVE/API2/LXC/Config.pm |  4 ++++
 src/PVE/LXC.pm             |  5 ++---
 src/PVE/LXC/Config.pm      | 19 +++++++++++--------
 4 files changed, 20 insertions(+), 11 deletions(-)


qemu-server:

Fiona Ebner (3):
  move get_cpuunits helper to guest-common
  change cpu shares: drop superfluous parameter
  api: create/update vm: clamp cpuunit value

 PVE/API2/Qemu.pm  |  6 ++++++
 PVE/QemuServer.pm | 23 ++++-------------------
 2 files changed, 10 insertions(+), 19 deletions(-)


common:

Fiona Ebner (1):
  change cpu shares: drop unused $cgroupv1_default parameter

 src/PVE/CGroup.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


manager:

Fiona Ebner (5):
  ui: lxc: cpu edit: drop superfluous delete_if_default
  ui: qemu/lxc: cpu edit: fix 'create' parameter for delete_if_default()
  ui: lxc/qemu: cpu edit: use emptyText for cpuunits
  cluster resources: add cgroup-mode to node properties
  ui: lxc/qemu: cpu edit: make cpuunits depend on node's cgroup version

 PVE/API2/Cluster.pm                | 12 +++++++++
 PVE/Service/pvestatd.pm            | 16 ++++++++++++
 www/manager6/lxc/CreateWizard.js   |  8 ++++++
 www/manager6/lxc/ResourceEdit.js   | 39 +++++++++++++++++++++++-------
 www/manager6/lxc/Resources.js      |  8 +++++-
 www/manager6/qemu/CreateWizard.js  |  8 ++++++
 www/manager6/qemu/HardwareView.js  |  8 +++++-
 www/manager6/qemu/ProcessorEdit.js | 34 ++++++++++++++++++++------
 8 files changed, 114 insertions(+), 19 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH docs 1/2] qm: resource limits: update systemd setting name for cpuunits

[Fiona Ebner]

As mentioned in man 5 systemd.resource-control, the CPUShares setting
was replaced by CPUWeight and is deprecated.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 qm.adoc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qm.adoc b/qm.adoc
index 4d0c7c4..19ac15d 100644
--- a/qm.adoc
+++ b/qm.adoc
@@ -325,7 +325,7 @@ changed to `2048`, the latter VM 200 would receive twice the CPU bandwidth than
 the first VM 100.
 
 For more information see `man systemd.resource-control`, here `CPUQuota`
-corresponds to `cpulimit` and `CPUShares` corresponds to our `cpuunits`
+corresponds to `cpulimit` and `CPUWeight` corresponds to our `cpuunits`
 setting, visit its Notes section for references and implementation details.
 
 CPU Type
-- 
2.30.2

[pve-devel] [PATCH docs 2/2] pct/qm: update cpuunits default

[Fiona Ebner]

which is 100 on hosts running cgroup v2. Still mention the old default
relevant for hosts with legacy cgroups.

Also reword the sentence with "gets in regards to other VMs running"
which sounded a bit off.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 pct.adoc |  5 +++--
 qm.adoc  | 12 ++++++------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/pct.adoc b/pct.adoc
index ebae3e9..90a6d04 100644
--- a/pct.adoc
+++ b/pct.adoc
@@ -427,8 +427,9 @@ cpulimit: 0.5
 
 `cpuunits`: :: This is a relative weight passed to the kernel scheduler. The
 larger the number is, the more CPU time this container gets. Number is relative
-to the weights of all the other running containers. The default is 1024. You
-can use this setting to prioritize some containers.
+to the weights of all the other running containers. The default is `100` (or
+`1024` if the host uses legacy cgroup v1). You can use this setting to
+prioritize some containers.
 
 
 [[pct_memory]]
diff --git a/qm.adoc b/qm.adoc
index 19ac15d..0adf98d 100644
--- a/qm.adoc
+++ b/qm.adoc
@@ -317,12 +317,12 @@ never uses more CPU time than virtual CPUs assigned set the *cpulimit* setting
 to the same value as the total core count.
 
 The second CPU resource limiting setting, *cpuunits* (nowadays often called CPU
-shares or CPU weight), controls how much CPU time a VM gets in regards to other
-VMs running.  It is a relative weight which defaults to `1024`, if you increase
-this for a VM it will be prioritized by the scheduler in comparison to other
-VMs with lower weight. E.g., if VM 100 has set the default 1024 and VM 200 was
-changed to `2048`, the latter VM 200 would receive twice the CPU bandwidth than
-the first VM 100.
+shares or CPU weight), controls how much CPU time a VM gets compared to other
+running VMs. It is a relative weight which defaults to `100` (or `1024` if the
+host uses legacy cgroup v1). If you increase this for a VM it will be
+prioritized by the scheduler in comparison to other VMs with lower weight. E.g.,
+if VM 100 has set the default `100` and VM 200 was changed to `200`, the latter
+VM 200 would receive twice the CPU bandwidth than the first VM 100.
 
 For more information see `man systemd.resource-control`, here `CPUQuota`
 corresponds to `cpulimit` and `CPUWeight` corresponds to our `cpuunits`
-- 
2.30.2

[pve-devel] [PATCH guest-common 1/1] helpers: move get_cpuunits helper from qemu-server

[Fiona Ebner]

while making it take the value directly instead of the config.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/GuestHelpers.pm | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/src/PVE/GuestHelpers.pm b/src/PVE/GuestHelpers.pm
index 0fe3fd6..f3a4202 100644
--- a/src/PVE/GuestHelpers.pm
+++ b/src/PVE/GuestHelpers.pm
@@ -3,6 +3,7 @@ package PVE::GuestHelpers;
 use strict;
 use warnings;
 
+use PVE::CGroup;
 use PVE::Tools;
 use PVE::Storage;
 
@@ -246,4 +247,19 @@ sub config_with_pending_array {
     return $res;
 }
 
+sub get_cpuunits {
+    my ($cpuunits) = @_;
+
+    my $is_cgroupv2 = PVE::CGroup::cgroup_mode() == 2;
+
+    return $is_cgroupv2 ? 100 : 1024 if !defined($cpuunits);
+
+    if ($is_cgroupv2) {
+	$cpuunits = 10000 if $cpuunits >= 10000; # v1 can be higher, so clamp v2 there
+    } else {
+	$cpuunits = 2 if $cpuunits < 2; # v2 can be lower, so clamp v1 there
+    }
+    return $cpuunits;
+}
+
 1;
-- 
2.30.2

[pve-devel] [PATCH container 1/7] config: cpuunits: drop description for outdated special value

[Fiona Ebner]

It won't work on hosts using cgroup v2. And there's one place where
$conf->{cpuunits} || 1024 is used, so zero would be overwritten there.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/LXC/Config.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index 23c1ba7..d66cb75 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -504,7 +504,7 @@ my $confdesc = {
     cpuunits => {
 	optional => 1,
 	type => 'integer',
-	description => "CPU weight for a VM. Argument is used in the kernel fair scheduler. The larger the number is, the more CPU time this VM gets. Number is relative to the weights of all the other running VMs.\n\nNOTE: You can disable fair-scheduler configuration by setting this to 0.",
+	description => "CPU weight for a VM. Argument is used in the kernel fair scheduler. The larger the number is, the more CPU time this VM gets. Number is relative to the weights of all the other running VMs.",
 	minimum => 0,
 	maximum => 500000,
 	default => 1024,
-- 
2.30.2

[pve-devel] [PATCH container 2/7] change cpu shares: hard-code cgroupv1 default parameter

[Fiona Ebner]

so that the description of the default can be changed to reflect that
it depends on cgroup version.

Not strictly necessary, because the function currently will ignore the
value anyways. But certainly more future-proof then starting to pass
something invalid.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/LXC/Config.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index d66cb75..48e0a6e 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -1342,7 +1342,7 @@ sub vmconfig_hotplug_pending {
 	    } elsif ($opt eq 'cpulimit') {
 		$cgroup->change_cpu_quota(undef, undef); # reset, cgroup module can better decide values
 	    } elsif ($opt eq 'cpuunits') {
-		$cgroup->change_cpu_shares(undef, $confdesc->{cpuunits}->{default});
+		$cgroup->change_cpu_shares(undef, 1024);
 	    } elsif ($opt =~ m/^net(\d)$/) {
 		my $netid = $1;
 		PVE::Network::veth_delete("veth${vmid}i$netid");
@@ -1367,7 +1367,7 @@ sub vmconfig_hotplug_pending {
 		my $quota = 100000 * $value;
 		$cgroup->change_cpu_quota(int(100000 * $value), 100000);
 	    } elsif ($opt eq 'cpuunits') {
-		$cgroup->change_cpu_shares($value, $confdesc->{cpuunits}->{default});
+		$cgroup->change_cpu_shares($value, 1024);
 	    } elsif ($opt =~ m/^net(\d+)$/) {
 		my $netid = $1;
 		my $net = $class->parse_lxc_network($value);
-- 
2.30.2

[pve-devel] [PATCH container 3/7] config: cpuunits: update default description

[Fiona Ebner]

to reflect that it depends on the cgroup version.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

In qemu-server, it's already like this.

 src/PVE/LXC/Config.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index 48e0a6e..b32eff6 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -507,7 +507,7 @@ my $confdesc = {
 	description => "CPU weight for a VM. Argument is used in the kernel fair scheduler. The larger the number is, the more CPU time this VM gets. Number is relative to the weights of all the other running VMs.",
 	minimum => 0,
 	maximum => 500000,
-	default => 1024,
+	default => 'cgroup v1: 1024, cgroup v2: 100',
     },
     memory => {
 	optional => 1,
-- 
2.30.2

[pve-devel] [PATCH container 4/7] use helper from guest-common for cpuunits

[Fiona Ebner]

to make behavior more consistent with what we do for VMs. The helper
will clamp the value as needed, rather than dying.

Allows starting existing containers with an out-of-range (for the
relevant cgroup version) value. It's also possible to end up with
out-of-range values via update/create API.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Dependency bump for guest-common needed.

 src/PVE/LXC.pm        | 5 ++---
 src/PVE/LXC/Config.pm | 5 ++++-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index fe63087..e143855 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -699,7 +699,7 @@ sub update_lxc_config {
 	    $raw .= "lxc.cgroup.cpu.cfs_quota_us = $value\n";
 	}
 
-	my $shares = $conf->{cpuunits} || 1024;
+	my $shares = PVE::GuestHelpers::get_cpuunits($conf->{cpuunits});
 	$raw .= "lxc.cgroup.cpu.shares = $shares\n";
     } elsif ($cgv2->{cpu}) {
 	# See PVE::CGroup
@@ -709,8 +709,7 @@ sub update_lxc_config {
 	}
 
 	if (defined(my $shares = $conf->{cpuunits})) {
-	    die "cpu weight (shares) must be in range [1, 10000]\n"
-		if $shares < 1 || $shares > 10000;
+	    $shares = PVE::GuestHelpers::get_cpuunits($shares);
 	    $raw .= "lxc.cgroup2.cpu.weight = $shares\n";
 	}
     }
diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index b32eff6..7be6be1 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -504,7 +504,10 @@ my $confdesc = {
     cpuunits => {
 	optional => 1,
 	type => 'integer',
-	description => "CPU weight for a VM. Argument is used in the kernel fair scheduler. The larger the number is, the more CPU time this VM gets. Number is relative to the weights of all the other running VMs.",
+	description => "CPU weight for a container, will be clamped to [1, 10000] in cgroup v2.",
+	verbose_description => "CPU weight for a container. Argument is used in the kernel fair "
+	    ."scheduler. The larger the number is, the more CPU time this container gets. Number "
+	    ."is relative to the weights of all the other running guests.",
 	minimum => 0,
 	maximum => 500000,
 	default => 'cgroup v1: 1024, cgroup v2: 100',
-- 
2.30.2

[pve-devel] [PATCH container 5/7] config: replace mentions of "VM" with "container"

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/LXC/Config.pm | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index 7be6be1..4b3ccf5 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -443,13 +443,13 @@ my $confdesc = {
     lock => {
 	optional => 1,
 	type => 'string',
-	description => "Lock/unlock the VM.",
+	description => "Lock/unlock the container.",
 	enum => [qw(backup create destroyed disk fstrim migrate mounted rollback snapshot snapshot-delete)],
     },
     onboot => {
 	optional => 1,
 	type => 'boolean',
-	description => "Specifies whether a VM will be started during system bootup.",
+	description => "Specifies whether a container will be started during system bootup.",
 	default => 0,
     },
     startup => get_standard_option('pve-startup-order'),
@@ -515,14 +515,14 @@ my $confdesc = {
     memory => {
 	optional => 1,
 	type => 'integer',
-	description => "Amount of RAM for the VM in MB.",
+	description => "Amount of RAM for the container in MB.",
 	minimum => 16,
 	default => 512,
     },
     swap => {
 	optional => 1,
 	type => 'integer',
-	description => "Amount of SWAP for the VM in MB.",
+	description => "Amount of SWAP for the container in MB.",
 	minimum => 0,
 	default => 512,
     },
-- 
2.30.2

[pve-devel] [RFC container 6/7] change cpu shares: drop superfluous parameter

[Fiona Ebner]

See the related commit in pve-common.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/LXC/Config.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index 4b3ccf5..fc654e6 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -1345,7 +1345,7 @@ sub vmconfig_hotplug_pending {
 	    } elsif ($opt eq 'cpulimit') {
 		$cgroup->change_cpu_quota(undef, undef); # reset, cgroup module can better decide values
 	    } elsif ($opt eq 'cpuunits') {
-		$cgroup->change_cpu_shares(undef, 1024);
+		$cgroup->change_cpu_shares(undef);
 	    } elsif ($opt =~ m/^net(\d)$/) {
 		my $netid = $1;
 		PVE::Network::veth_delete("veth${vmid}i$netid");
@@ -1370,7 +1370,7 @@ sub vmconfig_hotplug_pending {
 		my $quota = 100000 * $value;
 		$cgroup->change_cpu_quota(int(100000 * $value), 100000);
 	    } elsif ($opt eq 'cpuunits') {
-		$cgroup->change_cpu_shares($value, 1024);
+		$cgroup->change_cpu_shares($value);
 	    } elsif ($opt =~ m/^net(\d+)$/) {
 		my $netid = $1;
 		my $net = $class->parse_lxc_network($value);
-- 
2.30.2

[pve-devel] [RFC container 7/7] api: create/update vm: clamp cpuunit value

[Fiona Ebner]

While the clamping already happens before setting the actual
cpu.weight lxc config key, it can be done here too, to avoid writing
new out-of-range values into the config.

Can't use a validator enforcing this, because existing out-of-range
values should not become errors on parsing the config.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/API2/LXC.pm        | 3 +++
 src/PVE/API2/LXC/Config.pm | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 589f96f..8282850 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -221,6 +221,9 @@ __PACKAGE__->register_method({
 	my $restore = extract_param($param, 'restore');
 	my $unique = extract_param($param, 'unique');
 
+	$param->{cpuunits} = PVE::GuestHelpers::get_cpuunits($param->{cpuunits})
+	    if defined($param->{cpuunits}); # clamp value depending on cgroup version
+
 	# used to skip firewall config restore if user lacks permission
 	my $skip_fw_config_restore = 0;
 
diff --git a/src/PVE/API2/LXC/Config.pm b/src/PVE/API2/LXC/Config.pm
index 1fec048..a27be56 100644
--- a/src/PVE/API2/LXC/Config.pm
+++ b/src/PVE/API2/LXC/Config.pm
@@ -10,6 +10,7 @@ use PVE::INotify;
 use PVE::Cluster qw(cfs_read_file);
 use PVE::AccessControl;
 use PVE::Firewall;
+use PVE::GuestHelpers;
 use PVE::Storage;
 use PVE::RESTHandler;
 use PVE::RPCEnvironment;
@@ -144,6 +145,9 @@ __PACKAGE__->register_method({
 	my $revert_str = extract_param($param, 'revert');
 	my @revert = PVE::Tools::split_list($revert_str);
 
+	$param->{cpuunits} = PVE::GuestHelpers::get_cpuunits($param->{cpuunits})
+	    if defined($param->{cpuunits}); # clamp value depending on cgroup version
+
 	my $code = sub {
 
 	    my $conf = PVE::LXC::Config->load_config($vmid);
-- 
2.30.2

[pve-devel] [PATCH qemu-server 1/3] move get_cpuunits helper to guest-common

[Fiona Ebner]

while making it take the value directly instead of the config.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Dependency bump for guest-common needed.

 PVE/QemuServer.pm | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 4e85dd02..73e38292 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3446,21 +3446,6 @@ sub query_understood_cpu_flags {
     return \@flags;
 }
 
-my sub get_cpuunits {
-    my ($conf) = @_;
-    my $is_cgroupv2 = PVE::CGroup::cgroup_mode() == 2;
-
-    my $cpuunits = $conf->{cpuunits};
-    return $is_cgroupv2 ? 100 : 1024 if !defined($cpuunits);
-
-    if ($is_cgroupv2) {
-	$cpuunits = 10000 if $cpuunits >= 10000; # v1 can be higher, so clamp v2 there
-    } else {
-	$cpuunits = 2 if $cpuunits < 2; # v2 can be lower, so clamp v1 there
-    }
-    return $cpuunits;
-}
-
 # Since commit 277d33454f77ec1d1e0bc04e37621e4dd2424b67 in pve-qemu, smm is not off by default
 # anymore. But smm=off seems to be required when using SeaBIOS and serial display.
 my sub should_disable_smm {
@@ -4994,7 +4979,7 @@ sub vmconfig_hotplug_pending {
 		die "skip\n" if !$hotplug_features->{memory};
 		$value = PVE::QemuServer::Memory::qemu_memory_hotplug($vmid, $conf, $defaults, $opt, $value);
 	    } elsif ($opt eq 'cpuunits') {
-		my $new_cpuunits = get_cpuunits({ $opt => $conf->{pending}->{$opt} }); # to clamp
+		my $new_cpuunits = PVE::GuestHelpers::get_cpuunits($conf->{pending}->{$opt}); #clamp
 		$cgroup->change_cpu_shares($new_cpuunits, 1024);
 	    } elsif ($opt eq 'cpulimit') {
 		my $cpulimit = $conf->{pending}->{$opt} == 0 ? -1 : int($conf->{pending}->{$opt} * 100000);
@@ -5624,7 +5609,7 @@ sub vm_start_nolock {
     # timeout should be more than enough here...
     PVE::Systemd::wait_for_unit_removed("$vmid.scope", 20);
 
-    my $cpuunits = get_cpuunits($conf);
+    my $cpuunits = PVE::GuestHelpers::get_cpuunits($conf->{cpuunits});
 
     my %run_params = (
 	timeout => $statefile ? undef : $start_timeout,
-- 
2.30.2

[pve-devel] [RFC qemu-server 2/3] change cpu shares: drop superfluous parameter

[Fiona Ebner]

See the related commit in pve-common.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/QemuServer.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 73e38292..5bcc75f9 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -4885,7 +4885,7 @@ sub vmconfig_hotplug_pending {
 		die "skip\n" if !$hotplug_features->{memory};
 		PVE::QemuServer::Memory::qemu_memory_hotplug($vmid, $conf, $defaults, $opt);
 	    } elsif ($opt eq 'cpuunits') {
-		$cgroup->change_cpu_shares(undef, 1024);
+		$cgroup->change_cpu_shares(undef);
 	    } elsif ($opt eq 'cpulimit') {
 		$cgroup->change_cpu_quota(undef, undef); # reset, cgroup module can better decide values
 	    } else {
@@ -4980,7 +4980,7 @@ sub vmconfig_hotplug_pending {
 		$value = PVE::QemuServer::Memory::qemu_memory_hotplug($vmid, $conf, $defaults, $opt, $value);
 	    } elsif ($opt eq 'cpuunits') {
 		my $new_cpuunits = PVE::GuestHelpers::get_cpuunits($conf->{pending}->{$opt}); #clamp
-		$cgroup->change_cpu_shares($new_cpuunits, 1024);
+		$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);
-- 
2.30.2

[pve-devel] [RFC qemu-server 3/3] api: create/update vm: clamp cpuunit value

[Fiona Ebner]

While the clamping already happens before setting the actual systemd
CPU{Shares, Weight}, it can be done here too, to avoid writing new
out-of-range values into the config.

Can't use a validator enforcing this because existing out-of-range
values should not become errors upon parsing the config.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Qemu.pm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 3ec31c26..625d6ea7 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -814,6 +814,9 @@ __PACKAGE__->register_method({
 		PVE::Tools::validate_ssh_public_keys($ssh_keys);
 	}
 
+	$param->{cpuunits} = PVE::GuestHelpers::get_cpuunits($param->{cpuunits})
+	    if defined($param->{cpuunits}); # clamp value depending on cgroup version
+
 	PVE::Cluster::check_cfs_quorum();
 
 	my $filename = PVE::QemuConfig->config_file($vmid);
@@ -1362,6 +1365,9 @@ my $update_vm_api  = sub {
 	PVE::Tools::validate_ssh_public_keys($ssh_keys);
     }
 
+    $param->{cpuunits} = PVE::GuestHelpers::get_cpuunits($param->{cpuunits})
+	if defined($param->{cpuunits}); # clamp value depending on cgroup version
+
     die "no options specified\n" if !$delete_str && !$revert_str && !scalar(keys %$param);
 
     my $storecfg = PVE::Storage::config();
-- 
2.30.2

[pve-devel] [RFC common 1/1] change cpu shares: drop unused $cgroupv1_default parameter

[Fiona Ebner]

The only expression it appears in short-circuits, because the line
before ensures that $shares is guaranteed to be set.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Before we can add a new parameter to the function again, it would need
a Breaks for qemu-server and pve-container. So maybe we prefer to just
leave it as is? cgroupv1 won't be around forever anyways.

 src/PVE/CGroup.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/PVE/CGroup.pm b/src/PVE/CGroup.pm
index 45c0788..ae929f0 100644
--- a/src/PVE/CGroup.pm
+++ b/src/PVE/CGroup.pm
@@ -500,7 +500,7 @@ sub change_cpu_quota {
 #
 # Dies on error (including a not-running or currently-shutting-down guest).
 sub change_cpu_shares {
-    my ($self, $shares, $cgroupv1_default) = @_;
+    my ($self, $shares) = @_;
 
     my ($path, $ver) = $self->get_path('cpu', 1);
     if (!defined($path)) {
@@ -512,7 +512,7 @@ sub change_cpu_shares {
 	PVE::ProcFSTools::write_proc_entry("$path/cpu.weight", $shares);
     } elsif ($ver == 1) {
 	$shares //= 1024;
-	PVE::ProcFSTools::write_proc_entry("$path/cpu.shares", $shares // $cgroupv1_default);
+	PVE::ProcFSTools::write_proc_entry("$path/cpu.shares", $shares);
     } else {
 	die "bad cgroup version: $ver\n";
     }
-- 
2.30.2

[pve-devel] [PATCH manager 1/5] ui: lxc: cpu edit: drop superfluous delete_if_default

[Fiona Ebner]

Commit 9fcbafbe ("use IntegerField from widget toolkit") switched to
using an Promxox IntegerField with 'deleteEmpty: true'. This means
that a value of '' already is removed from 'values' and added to
'delete' before onGetValues() is called here.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 www/manager6/lxc/ResourceEdit.js | 1 -
 1 file changed, 1 deletion(-)

diff --git a/www/manager6/lxc/ResourceEdit.js b/www/manager6/lxc/ResourceEdit.js
index 6f21571b..f926d784 100644
--- a/www/manager6/lxc/ResourceEdit.js
+++ b/www/manager6/lxc/ResourceEdit.js
@@ -46,7 +46,6 @@ Ext.define('PVE.lxc.CPUInputPanel', {
     onGetValues: function(values) {
 	var me = this;
 
-	PVE.Utils.delete_if_default(values, 'cores', '', me.insideWizard);
 	// cpu{limit,unit} aren't in the wizard so create is always false
 	PVE.Utils.delete_if_default(values, 'cpulimit', '0', 0);
 	PVE.Utils.delete_if_default(values, 'cpuunits', '1024', 0);
-- 
2.30.2

[pve-devel] [PATCH manager 2/5] ui: qemu/lxc: cpu edit: fix 'create' parameter for delete_if_default()

[Fiona Ebner]

For lxc, commit 3f40bc29 ("lxc/ResoureEdit: move some cpu fields to
advanced options") made the fields available in the create wizard, so
the comment is also wrong.

Both wizards do a 'delete kv.delete' in their onSubmit() function, so
it didn't actually matter in practice.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 www/manager6/lxc/ResourceEdit.js   | 5 ++---
 www/manager6/qemu/ProcessorEdit.js | 4 ++--
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/www/manager6/lxc/ResourceEdit.js b/www/manager6/lxc/ResourceEdit.js
index f926d784..7b0d6886 100644
--- a/www/manager6/lxc/ResourceEdit.js
+++ b/www/manager6/lxc/ResourceEdit.js
@@ -46,9 +46,8 @@ Ext.define('PVE.lxc.CPUInputPanel', {
     onGetValues: function(values) {
 	var me = this;
 
-	// cpu{limit,unit} aren't in the wizard so create is always false
-	PVE.Utils.delete_if_default(values, 'cpulimit', '0', 0);
-	PVE.Utils.delete_if_default(values, 'cpuunits', '1024', 0);
+	PVE.Utils.delete_if_default(values, 'cpulimit', '0', me.insideWizard);
+	PVE.Utils.delete_if_default(values, 'cpuunits', '1024', me.insideWizard);
 
 	return values;
     },
diff --git a/www/manager6/qemu/ProcessorEdit.js b/www/manager6/qemu/ProcessorEdit.js
index 1bed2877..7bb4cc60 100644
--- a/www/manager6/qemu/ProcessorEdit.js
+++ b/www/manager6/qemu/ProcessorEdit.js
@@ -27,8 +27,8 @@ Ext.define('PVE.qemu.ProcessorInputPanel', {
 	    values.delete = values.delete.join(',');
 	}
 
-	PVE.Utils.delete_if_default(values, 'cpulimit', '0', 0);
-	PVE.Utils.delete_if_default(values, 'cpuunits', '1024', 0);
+	PVE.Utils.delete_if_default(values, 'cpulimit', '0', me.insideWizard);
+	PVE.Utils.delete_if_default(values, 'cpuunits', '1024', me.insideWizard);
 
 	// build the cpu options:
 	me.cpu.cputype = values.cputype;
-- 
2.30.2

[pve-devel] [PATCH manager 3/5] ui: lxc/qemu: cpu edit: use emptyText for cpuunits

[Fiona Ebner]

This makes it slightly clearer that it's a default value (not being
written to the config explicitly). It's also in preparation to bind
the default based on the cgroup version, where it's a bit nicer to
bind the emptyText instead of the value (or a user set value would be
overwritten when the binding updates).

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 www/manager6/lxc/ResourceEdit.js   | 6 ++++--
 www/manager6/qemu/ProcessorEdit.js | 3 ++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/www/manager6/lxc/ResourceEdit.js b/www/manager6/lxc/ResourceEdit.js
index 7b0d6886..42357d85 100644
--- a/www/manager6/lxc/ResourceEdit.js
+++ b/www/manager6/lxc/ResourceEdit.js
@@ -70,11 +70,13 @@ Ext.define('PVE.lxc.CPUInputPanel', {
 	    xtype: 'proxmoxintegerfield',
 	    name: 'cpuunits',
 	    fieldLabel: gettext('CPU units'),
-	    value: 1024,
+	    value: '',
 	    minValue: 8,
 	    maxValue: 500000,
+	    emptyText: '1024',
 	    labelWidth: labelWidth,
-	    allowBlank: false,
+	    deleteEmpty: true,
+	    allowBlank: true,
 	},
     ],
 
diff --git a/www/manager6/qemu/ProcessorEdit.js b/www/manager6/qemu/ProcessorEdit.js
index 7bb4cc60..2edfc65c 100644
--- a/www/manager6/qemu/ProcessorEdit.js
+++ b/www/manager6/qemu/ProcessorEdit.js
@@ -190,7 +190,8 @@ Ext.define('PVE.qemu.ProcessorInputPanel', {
 	    // FIXME: change to [1, 1000] once cgroup v1 support gets removed (PVE 8 ?)
 	    minValue: 2,
 	    maxValue: 262144,
-	    value: '1024',
+	    value: '',
+	    emptyText: '1024',
 	    deleteEmpty: true,
 	    allowBlank: true,
 	},
-- 
2.30.2

[pve-devel] [PATCH v2 manager 4/5] cluster resources: add cgroup-mode to node properties

[Fiona Ebner]

so the frontend has the information readily available.

It's enough for pvestatd to broadcast the value whenever it's outdated
or not set in the node's local kv store, because pmxcfs will
re-broadcast the local kv store whenever the quorate partition
changes. This is already relied upon for the 'ceph-versions' kv pair.

Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Changes from v1:
    * Only broadcast if necessary rather than periodically.

 PVE/API2/Cluster.pm     | 12 ++++++++++++
 PVE/Service/pvestatd.pm | 16 ++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/PVE/API2/Cluster.pm b/PVE/API2/Cluster.pm
index d6b405e2..cb96abd7 100644
--- a/PVE/API2/Cluster.pm
+++ b/PVE/API2/Cluster.pm
@@ -312,6 +312,11 @@ __PACKAGE__->register_method({
 		    type => 'string',
 		    optional => 1,
 		},
+		'cgroup-mode' => {
+		    description => "The cgroup mode the node operates under (when type == node).",
+		    type => 'integer',
+		    optional => 1,
+		},
 	    },
 	},
     },
@@ -416,10 +421,17 @@ __PACKAGE__->register_method({
 	    }
 	}
 
+	my $cgroup_modes = PVE::Cluster::get_node_kv("cgroup-mode");
+
 	if (!$param->{type} || $param->{type} eq 'node') {
 	    foreach my $node (@$nodelist) {
 		my $can_audit = $rpcenv->check($authuser, "/nodes/$node", [ 'Sys.Audit' ], 1);
 		my $entry = PVE::API2Tools::extract_node_stats($node, $members, $rrd, !$can_audit);
+
+		if (defined(my $mode = $cgroup_modes->{$node})) {
+		    $entry->{'cgroup-mode'} = int($mode);
+		}
+
 		push @$res, $entry;
 	    }
 	}
diff --git a/PVE/Service/pvestatd.pm b/PVE/Service/pvestatd.pm
index eac953df..61de8b36 100755
--- a/PVE/Service/pvestatd.pm
+++ b/PVE/Service/pvestatd.pm
@@ -123,9 +123,25 @@ my $generate_rrd_string = sub {
     return join(':', map { $_ // 'U' } @$data);
 };
 
+my sub broadcast_cgroup_mode {
+    my $cgroup_mode = eval { PVE::CGroup::cgroup_mode(); };
+    if (my $err = $@) {
+	syslog('err', "cgroup mode error: $err");
+	return;
+    }
+
+    my $old = PVE::Cluster::get_node_kv("cgroup-mode", $cgroup_mode, $nodename);
+    $old = eval { decode_json($old->{$nodename}) } if defined($old->{$nodename});
+
+    PVE::Cluster::broadcast_node_kv("cgroup-mode", $cgroup_mode)
+	if !$old || $old != $cgroup_mode;
+}
+
 sub update_node_status {
     my ($status_cfg) = @_;
 
+    broadcast_cgroup_mode();
+
     my ($uptime) = PVE::ProcFSTools::read_proc_uptime();
 
     my ($avg1, $avg5, $avg15) = PVE::ProcFSTools::read_loadavg();
-- 
2.30.2

[pve-devel] [PATCH manager 5/5] ui: lxc/qemu: cpu edit: make cpuunits depend on node's cgroup version

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 www/manager6/lxc/CreateWizard.js   |  8 ++++++++
 www/manager6/lxc/ResourceEdit.js   | 31 +++++++++++++++++++++++++-----
 www/manager6/lxc/Resources.js      |  8 +++++++-
 www/manager6/qemu/CreateWizard.js  |  8 ++++++++
 www/manager6/qemu/HardwareView.js  |  8 +++++++-
 www/manager6/qemu/ProcessorEdit.js | 31 +++++++++++++++++++++++-------
 6 files changed, 80 insertions(+), 14 deletions(-)

diff --git a/www/manager6/lxc/CreateWizard.js b/www/manager6/lxc/CreateWizard.js
index 1f902c2c..0b82cc1c 100644
--- a/www/manager6/lxc/CreateWizard.js
+++ b/www/manager6/lxc/CreateWizard.js
@@ -8,6 +8,14 @@ Ext.define('PVE.lxc.CreateWizard', {
 	    storage: '',
 	    unprivileged: true,
 	},
+	formulas: {
+	    cgroupMode: function(get) {
+		const nodeInfo = PVE.data.ResourceStore.getNodes().find(
+		    node => node.node === get('nodename'),
+		);
+		return nodeInfo ? nodeInfo['cgroup-mode'] : 2;
+	    },
+	},
     },
 
     cbindData: {
diff --git a/www/manager6/lxc/ResourceEdit.js b/www/manager6/lxc/ResourceEdit.js
index 42357d85..9f4f7e08 100644
--- a/www/manager6/lxc/ResourceEdit.js
+++ b/www/manager6/lxc/ResourceEdit.js
@@ -20,9 +20,17 @@ Ext.define('PVE.lxc.MemoryEdit', {
 
 Ext.define('PVE.lxc.CPUEdit', {
     extend: 'Proxmox.window.Edit',
+    alias: 'widget.pveLxcCPUEdit',
+
+    viewModel: {
+	data: {
+	    cgroupMode: 2,
+	},
+    },
 
     initComponent: function() {
-	var me = this;
+	let me = this;
+	me.getViewModel().set('cgroupMode', me.cgroupMode);
 
 	Ext.apply(me, {
 	    subject: gettext('CPU'),
@@ -35,6 +43,7 @@ Ext.define('PVE.lxc.CPUEdit', {
     },
 });
 
+// The view model of the parent shoul contain a 'cgroupMode' variable (or params for v2 are used).
 Ext.define('PVE.lxc.CPUInputPanel', {
     extend: 'Proxmox.panel.InputPanel',
     alias: 'widget.pveLxcCPUInputPanel',
@@ -43,11 +52,19 @@ Ext.define('PVE.lxc.CPUInputPanel', {
 
     insideWizard: false,
 
+    viewModel: {
+	formulas: {
+	    cpuunitsDefault: (get) => get('cgroupMode') === 1 ? 1024 : 100,
+	    cpuunitsMax: (get) => get('cgroupMode') === 1 ? 500000 : 10000,
+	},
+    },
+
     onGetValues: function(values) {
-	var me = this;
+	let me = this;
+	let cpuunitsDefault = me.getViewModel().get('cpuunitsDefault');
 
 	PVE.Utils.delete_if_default(values, 'cpulimit', '0', me.insideWizard);
-	PVE.Utils.delete_if_default(values, 'cpuunits', '1024', me.insideWizard);
+	PVE.Utils.delete_if_default(values, 'cpuunits', `${cpuunitsDefault}`, me.insideWizard);
 
 	return values;
     },
@@ -72,8 +89,12 @@ Ext.define('PVE.lxc.CPUInputPanel', {
 	    fieldLabel: gettext('CPU units'),
 	    value: '',
 	    minValue: 8,
-	    maxValue: 500000,
-	    emptyText: '1024',
+	    maxValue: '10000',
+	    emptyText: '100',
+	    bind: {
+		emptyText: '{cpuunitsDefault}',
+		maxValue: '{cpuunitsMax}',
+	    },
 	    labelWidth: labelWidth,
 	    deleteEmpty: true,
 	    allowBlank: true,
diff --git a/www/manager6/lxc/Resources.js b/www/manager6/lxc/Resources.js
index 4b2ae95e..85112345 100644
--- a/www/manager6/lxc/Resources.js
+++ b/www/manager6/lxc/Resources.js
@@ -45,6 +45,12 @@ Ext.define('PVE.lxc.RessourceView', {
 
 	var mpeditor = caps.vms['VM.Config.Disk'] ? 'PVE.lxc.MountPointEdit' : undefined;
 
+	const nodeInfo = PVE.data.ResourceStore.getNodes().find(node => node.node === nodename);
+	let cpuEditor = {
+	    xtype: 'pveLxcCPUEdit',
+	    cgroupMode: nodeInfo['cgroup-mode'],
+	};
+
 	var rows = {
 	    memory: {
 		header: gettext('Memory'),
@@ -68,7 +74,7 @@ Ext.define('PVE.lxc.RessourceView', {
 	    },
 	    cores: {
 		header: gettext('Cores'),
-		editor: caps.vms['VM.Config.CPU'] ? 'PVE.lxc.CPUEdit' : undefined,
+		editor: caps.vms['VM.Config.CPU'] ? cpuEditor : undefined,
 		defaultValue: '',
 		tdCls: 'pmx-itype-icon-processor',
 		group: 3,
diff --git a/www/manager6/qemu/CreateWizard.js b/www/manager6/qemu/CreateWizard.js
index a785a882..a65067ea 100644
--- a/www/manager6/qemu/CreateWizard.js
+++ b/www/manager6/qemu/CreateWizard.js
@@ -10,6 +10,14 @@ Ext.define('PVE.qemu.CreateWizard', {
 		scsihw: '',
 	    },
 	},
+	formulas: {
+	    cgroupMode: function(get) {
+		const nodeInfo = PVE.data.ResourceStore.getNodes().find(
+		    node => node.node === get('nodename'),
+		);
+		return nodeInfo ? nodeInfo['cgroup-mode'] : 2;
+	    },
+	},
     },
 
     cbindData: {
diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
index 6e9d03b4..eb069b30 100644
--- a/www/manager6/qemu/HardwareView.js
+++ b/www/manager6/qemu/HardwareView.js
@@ -59,6 +59,12 @@ Ext.define('PVE.qemu.HardwareView', {
 
 	let isCloudInitKey = v => v && v.toString().match(/vm-.*-cloudinit/);
 
+	const nodeInfo = PVE.data.ResourceStore.getNodes().find(node => node.node === nodename);
+	let processorEditor = {
+	    xtype: 'pveQemuProcessorEdit',
+	    cgroupMode: nodeInfo['cgroup-mode'],
+	};
+
 	let rows = {
 	    memory: {
 		header: gettext('Memory'),
@@ -93,7 +99,7 @@ Ext.define('PVE.qemu.HardwareView', {
 		header: gettext('Processors'),
 		never_delete: true,
 		editor: caps.vms['VM.Config.CPU'] || caps.vms['VM.Config.HWType']
-		    ? 'PVE.qemu.ProcessorEdit' : undefined,
+		    ? processorEditor : undefined,
 		tdCls: 'pve-itype-icon-cpu',
 		group: 3,
 		defaultValue: '1',
diff --git a/www/manager6/qemu/ProcessorEdit.js b/www/manager6/qemu/ProcessorEdit.js
index 2edfc65c..601f3edf 100644
--- a/www/manager6/qemu/ProcessorEdit.js
+++ b/www/manager6/qemu/ProcessorEdit.js
@@ -1,3 +1,4 @@
+// The view model of the parent shoul contain a 'cgroupMode' variable (or params for v2 are used).
 Ext.define('PVE.qemu.ProcessorInputPanel', {
     extend: 'Proxmox.panel.InputPanel',
     alias: 'widget.pveQemuProcessorPanel',
@@ -13,6 +14,9 @@ Ext.define('PVE.qemu.ProcessorInputPanel', {
 	},
 	formulas: {
 	    totalCoreCount: get => get('socketCount') * get('coreCount'),
+	    cpuunitsDefault: (get) => get('cgroupMode') === 1 ? 1024 : 100,
+	    cpuunitsMin: (get) => get('cgroupMode') === 1 ? 2 : 1,
+	    cpuunitsMax: (get) => get('cgroupMode') === 1 ? 262144 : 10000,
 	},
     },
 
@@ -21,14 +25,15 @@ Ext.define('PVE.qemu.ProcessorInputPanel', {
     },
 
     onGetValues: function(values) {
-	var me = this;
+	let me = this;
+	let cpuunitsDefault = me.getViewModel().get('cpuunitsDefault');
 
 	if (Array.isArray(values.delete)) {
 	    values.delete = values.delete.join(',');
 	}
 
 	PVE.Utils.delete_if_default(values, 'cpulimit', '0', me.insideWizard);
-	PVE.Utils.delete_if_default(values, 'cpuunits', '1024', me.insideWizard);
+	PVE.Utils.delete_if_default(values, 'cpuunits', `${cpuunitsDefault}`, me.insideWizard);
 
 	// build the cpu options:
 	me.cpu.cputype = values.cputype;
@@ -187,11 +192,15 @@ Ext.define('PVE.qemu.ProcessorInputPanel', {
 	    xtype: 'proxmoxintegerfield',
 	    name: 'cpuunits',
 	    fieldLabel: gettext('CPU units'),
-	    // FIXME: change to [1, 1000] once cgroup v1 support gets removed (PVE 8 ?)
-	    minValue: 2,
-	    maxValue: 262144,
+	    minValue: '1',
+	    maxValue: '10000',
 	    value: '',
-	    emptyText: '1024',
+	    emptyText: '100',
+	    bind: {
+		minValue: '{cpuunitsMin}',
+		maxValue: '{cpuunitsMax}',
+		emptyText: '{cpuunitsDefault}',
+	    },
 	    deleteEmpty: true,
 	    allowBlank: true,
 	},
@@ -216,11 +225,19 @@ Ext.define('PVE.qemu.ProcessorInputPanel', {
 
 Ext.define('PVE.qemu.ProcessorEdit', {
     extend: 'Proxmox.window.Edit',
+    alias: 'widget.pveQemuProcessorEdit',
 
     width: 700,
 
+    viewModel: {
+	data: {
+	    cgroupMode: 2,
+	},
+    },
+
     initComponent: function() {
-	var me = this;
+	let me = this;
+	me.getViewModel().set('cgroupMode', me.cgroupMode);
 
 	var ipanel = Ext.create('PVE.qemu.ProcessorInputPanel');
 
-- 
2.30.2

[pve-devel] applied: [PATCH docs 1/2] qm: resource limits: update systemd setting name for cpuunits

[Thomas Lamprecht]

Am 07/10/2022 um 14:41 schrieb Fiona Ebner:
> As mentioned in man 5 systemd.resource-control, the CPUShares setting
> was replaced by CPUWeight and is deprecated.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  qm.adoc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied both docs patches, thanks!

Re: [pve-devel] [PATCH guest-common 1/1] helpers: move get_cpuunits helper from qemu-server

[Thomas Lamprecht]

Am 07/10/2022 um 14:41 schrieb Fiona Ebner:
> while making it take the value directly instead of the config.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  src/PVE/GuestHelpers.pm | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/src/PVE/GuestHelpers.pm b/src/PVE/GuestHelpers.pm
> index 0fe3fd6..f3a4202 100644
> --- a/src/PVE/GuestHelpers.pm
> +++ b/src/PVE/GuestHelpers.pm
> @@ -3,6 +3,7 @@ package PVE::GuestHelpers;
>  use strict;
>  use warnings;
>  
> +use PVE::CGroup;
>  use PVE::Tools;
>  use PVE::Storage;
>  
> @@ -246,4 +247,19 @@ sub config_with_pending_array {
>      return $res;
>  }
>  
> +sub get_cpuunits {
> +    my ($cpuunits) = @_;
> +
> +    my $is_cgroupv2 = PVE::CGroup::cgroup_mode() == 2;
> +
> +    return $is_cgroupv2 ? 100 : 1024 if !defined($cpuunits);
> +
> +    if ($is_cgroupv2) {
> +	$cpuunits = 10000 if $cpuunits >= 10000; # v1 can be higher, so clamp v2 there
> +    } else {
> +	$cpuunits = 2 if $cpuunits < 2; # v2 can be lower, so clamp v1 there
> +    }
> +    return $cpuunits;
> +}
> +
>  1;

replaced with adding a similar clamp_cpu_shares method to pve-common's PVE::CGroup module

Re: [pve-devel] [PATCH v2 manager 4/5] cluster resources: add cgroup-mode to node properties

[Fiona Ebner]

> diff --git a/PVE/Service/pvestatd.pm b/PVE/Service/pvestatd.pm
> index eac953df..61de8b36 100755
> --- a/PVE/Service/pvestatd.pm
> +++ b/PVE/Service/pvestatd.pm
> @@ -123,9 +123,25 @@ my $generate_rrd_string = sub {
>      return join(':', map { $_ // 'U' } @$data);
>  };
>  
> +my sub broadcast_cgroup_mode {
> +    my $cgroup_mode = eval { PVE::CGroup::cgroup_mode(); };
> +    if (my $err = $@) {
> +	syslog('err', "cgroup mode error: $err");
> +	return;
> +    }
> +
> +    my $old = PVE::Cluster::get_node_kv("cgroup-mode", $cgroup_mode, $nodename);

The second parameter here is wrong, sorry!

Since the static node information like CPU core count and total memory
is intended to be similarly broadcast, it might even be better to
generalize the kv entry to "static-info", which includes cgroup mode as
well as CPU+memory info.

[pve-devel] applied: [RFC common 1/1] change cpu shares: drop unused $cgroupv1_default parameter

[Thomas Lamprecht]

Am 07/10/2022 um 14:41 schrieb Fiona Ebner:
> The only expression it appears in short-circuits, because the line
> before ensures that $shares is guaranteed to be set.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
> 
> Before we can add a new parameter to the function again, it would need
> a Breaks for qemu-server and pve-container. So maybe we prefer to just
> leave it as is? cgroupv1 won't be around forever anyways.
> 
>  src/PVE/CGroup.pm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
>

applied, with a note added to the method to record the hint about breakage if
we add new params to it, at least during this major release series, thanks!

[pve-devel] partially-applied-series: [PATCH-SERIES docs/(guest-)common/qemu-server/container/manager] cpuunits improvements

[Thomas Lamprecht]

Am 07/10/2022 um 14:41 schrieb Fiona Ebner:
> docs:
> 
> Fiona Ebner (2):
>   qm: resource limits: update systemd setting name for cpuunits
>   pct/qm: update cpuunits default
> 

> guest-common
> 
> Fiona Ebner (1):
>   helpers: move get_cpuunits helper from qemu-server
> 

> container:
> 
> Fiona Ebner (7):
>   config: cpuunits: drop description for outdated special value
>   change cpu shares: hard-code cgroupv1 default parameter
>   config: cpuunits: update default description
>   use helper from guest-common for cpuunits
>   config: replace mentions of "VM" with "container"
>   change cpu shares: drop superfluous parameter
>   api: create/update vm: clamp cpuunit value

> qemu-server:
> 
> Fiona Ebner (3):
>   move get_cpuunits helper to guest-common
>   change cpu shares: drop superfluous parameter
>   api: create/update vm: clamp cpuunit value

> common:
> 
> Fiona Ebner (1):
>   change cpu shares: drop unused $cgroupv1_default parameter

> manager:
> 
> Fiona Ebner (5):
>   ui: lxc: cpu edit: drop superfluous delete_if_default
>   ui: qemu/lxc: cpu edit: fix 'create' parameter for delete_if_default()
>   ui: lxc/qemu: cpu edit: use emptyText for cpuunits
>   cluster resources: add cgroup-mode to node properties
>   ui: lxc/qemu: cpu edit: make cpuunits depend on node's cgroup version
> 


applied all but the last two manager patches, and the guest-common one got
replaced with adding such a helper to pve-common's PVE::CGroup with call sites
adapted (squashed into your patches), thanks!