[pve-devel] [PATCH-SERIES guest-common/container/qemu-server/ha-manager] make HA manager not depend on internal configuration details

[pve-devel] [PATCH-SERIES guest-common/container/qemu-server/ha-manager] make HA manager not depend on internal configuration details

[Fiona Ebner]

by introducing a get_derived_property() method for the configuration
plugins. The derived properties are calculated by the plugins and will
stay the same regardless of changes to the configuration structure.
For example, this will allow turning QemuConfig's 'memory' into a
property string (still requires a versioned Breaks for older HA
manager).

Dependency bumps ha-manager -> container,qemu-server are needed.


guest-common:

Fiona Ebner (1):
  abstract config: add method to calculate derived properties from a
    config

 src/PVE/AbstractConfig.pm | 9 +++++++++
 1 file changed, 9 insertions(+)


container:

Fiona Ebner (1):
  config: implement method to calculate derived properties from a config

 src/PVE/LXC/Config.pm | 12 ++++++++++++
 1 file changed, 12 insertions(+)


qemu-server:

Fiona Ebner (1):
  config: implement method to calculate derived properties from a config

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


ha-manager:

Fiona Ebner (1):
  resources: pve: avoid relying on internal configuration details

 src/PVE/HA/Resources/PVECT.pm | 4 ++--
 src/PVE/HA/Resources/PVEVM.pm | 7 ++-----
 2 files changed, 4 insertions(+), 7 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH guest-common 1/1] abstract config: add method to calculate derived properties from a config

[Fiona Ebner]

HA manager currently needs to know about internal details about the
configs and how the properties are calculated. With this method, those
details are abstracted away, allowing to change the configuration
structure. In particular, QemuConfig's 'memory' can be turned into
a property string without HA manager needing to know about it (once HA
manager switched to using this mehtod).

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

diff --git a/src/PVE/AbstractConfig.pm b/src/PVE/AbstractConfig.pm
index a0c0bc6..3d2c255 100644
--- a/src/PVE/AbstractConfig.pm
+++ b/src/PVE/AbstractConfig.pm
@@ -1158,6 +1158,15 @@ sub snapshot_rollback {
     $class->lock_config($vmid, $updatefn);
 }
 
+# Calculate a derived property from a configuration. Derived properties are:
+# max-cpu - maximum amount of CPUs the guest can currently use (can be a fraction)
+# max-memory - maximum amount of memory the guest can currently use
+sub get_derived_property {
+    my ($class, $conf, $name) = @_;
+
+    die "implement me - abstract method\n";
+}
+
 # bash completion helper
 
 sub snapshot_list {
-- 
2.30.2

[pve-devel] applied: [PATCH guest-common 1/1] abstract config: add method to calculate derived properties from a config

[Thomas Lamprecht]

Am 28/02/2023 um 11:54 schrieb Fiona Ebner:
> HA manager currently needs to know about internal details about the
> configs and how the properties are calculated. With this method, those
> details are abstracted away, allowing to change the configuration
> structure. In particular, QemuConfig's 'memory' can be turned into
> a property string without HA manager needing to know about it (once HA
> manager switched to using this mehtod).
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  src/PVE/AbstractConfig.pm | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
>

applied, thanks!

[pve-devel] [PATCH container 1/1] config: implement method to calculate derived properties from a config

[Fiona Ebner]

See the corresponding commit in guest-common for more information.

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

diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index af25a96..b511c2b 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -1732,4 +1732,16 @@ sub get_backup_volumes {
     return $return_volumes;
 }
 
+sub get_derived_property {
+    my ($class, $conf, $name) = @_;
+
+    if ($name eq 'max-cpu') {
+	return $conf->{cpulimit} || $conf->{cores} || 0;
+    } elsif ($name eq 'max-memory') {
+	return ($conf->{memory} || 512) * 1024 * 1024;
+    } else {
+	die "unknown derived property - $name\n";
+    }
+}
+
 1;
-- 
2.30.2

[pve-devel] applied: Re: [PATCH container 1/1] config: implement method to calculate derived properties from a config

[Thomas Lamprecht]

On 28/02/2023 11:54, Fiona Ebner wrote:
> See the corresponding commit in guest-common for more information.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  src/PVE/LXC/Config.pm | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
>

applied, thanks!

[pve-devel] [PATCH qemu-server 1/1] config: implement method to calculate derived properties from a config

[Fiona Ebner]

See the corresponding commit in guest-common for more information.

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

diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
index 051382cd..10e69299 100644
--- a/PVE/QemuConfig.pm
+++ b/PVE/QemuConfig.pm
@@ -540,6 +540,22 @@ sub load_current_config {
     return $conf;
 }
 
+sub get_derived_property {
+    my ($class, $conf, $name) = @_;
+
+    my $defaults = PVE::QemuServer::load_defaults();
+
+    if ($name eq 'max-cpu') {
+	my $cpus =
+	    ($conf->{sockets} || $defaults->{sockets}) * ($conf->{cores} || $defaults->{cores});
+	return $conf->{vcpus} || $cpus;
+    } elsif ($name eq 'max-memory') {
+	return ($conf->{memory} || $defaults->{memory}) * 1024 * 1024;
+    } else {
+	die "unknown derived property - $name\n";
+    }
+}
+
 # END implemented abstract methods from PVE::AbstractConfig
 
 sub has_cloudinit {
-- 
2.30.2

[pve-devel] applied: Re: [PATCH qemu-server 1/1] config: implement method to calculate derived properties from a config

[Thomas Lamprecht]

On 28/02/2023 11:54, Fiona Ebner wrote:
> See the corresponding commit in guest-common for more information.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  PVE/QemuConfig.pm | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
>

applied, thanks!

[pve-devel] [PATCH ha-manager 1/1] resources: pve: avoid relying on internal configuration details

[Fiona Ebner]

Instead, use the new get_derived_property() method to get the same
information in a way that is robust regarding changes in the
configuration structure.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/HA/Resources/PVECT.pm | 4 ++--
 src/PVE/HA/Resources/PVEVM.pm | 7 ++-----
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/src/PVE/HA/Resources/PVECT.pm b/src/PVE/HA/Resources/PVECT.pm
index e77d98c..6e14692 100644
--- a/src/PVE/HA/Resources/PVECT.pm
+++ b/src/PVE/HA/Resources/PVECT.pm
@@ -158,8 +158,8 @@ sub get_static_stats {
     my $conf = PVE::LXC::Config->load_config($id, $service_node);
 
     return {
-	maxcpu => $conf->{cpulimit} || $conf->{cores} || 0,
-	maxmem => ($conf->{memory} || 512) * 1024 * 1024,
+	maxcpu => PVE::LXC::Config->get_derived_property($conf, 'max-cpu'),
+	maxmem => PVE::LXC::Config->get_derived_property($conf, 'max-memory'),
     };
 }
 
diff --git a/src/PVE/HA/Resources/PVEVM.pm b/src/PVE/HA/Resources/PVEVM.pm
index f405d86..1543a2d 100644
--- a/src/PVE/HA/Resources/PVEVM.pm
+++ b/src/PVE/HA/Resources/PVEVM.pm
@@ -179,13 +179,10 @@ sub get_static_stats {
     my ($class, $haenv, $id, $service_node) = @_;
 
     my $conf = PVE::QemuConfig->load_config($id, $service_node);
-    my $defaults = PVE::QemuServer::load_defaults();
-
-    my $cpus = ($conf->{sockets} || $defaults->{sockets}) * ($conf->{cores} || $defaults->{cores});
 
     return {
-	maxcpu => $conf->{vcpus} || $cpus,
-	maxmem => ($conf->{memory} || $defaults->{memory}) * 1024 * 1024,
+	maxcpu => PVE::QemuConfig->get_derived_property($conf, 'max-cpu'),
+	maxmem => PVE::QemuConfig->get_derived_property($conf, 'max-memory'),
     };
 }
 
-- 
2.30.2

Re: [pve-devel] [PATCH-SERIES guest-common/container/qemu-server/ha-manager] make HA manager not depend on internal configuration details

[Fiona Ebner]

Am 28.02.23 um 11:54 schrieb Fiona Ebner:
> by introducing a get_derived_property() method for the configuration
> plugins. The derived properties are calculated by the plugins and will
> stay the same regardless of changes to the configuration structure.
> For example, this will allow turning QemuConfig's 'memory' into a
> property string (still requires a versioned Breaks for older HA
> manager).
> 
> Dependency bumps ha-manager -> container,qemu-server are needed.
> 

Should this be applied together with the versioned Breaks? That Breaks
is needed in preparation for Alexandre's memory hotplug rework
series[0]. Now would be a good opportunity for the versioned Breaks.

[0]: https://lists.proxmox.com/pipermail/pve-devel/2023-February/055920.html