[pve-devel] [PATCH-SERIES ha-manager v2 0/4] fix #7133 and improve HA group migration

[pve-devel] [PATCH-SERIES ha-manager v2 0/4] fix #7133 and improve HA group migration

[Daniel Kral]

Changes from v1 to v2 (Thanks @Fiona!):
    - pull $delete parameter into $changes hash in
      update_{service,resources}_config
    - sort the HA resources' sids when iterating over the $changes hash

Tested the changes for v2 again with:
    - creating a groups.cfg and checking whether the groups have been
      correctly migrated,
    - changing a HA resource to stopped state changes the config too, &
    - changing a HA resource's config through the API reflects the
      changes in the resources.cfg.


This small patch series improves the HA groups to rules migration for HA
setups with a lot of HA resources:

- Patch 1 reduces the amount of unnecessary updates and is enough for
  fixing #7133,
- Patch 2 removes redundant failback flag writes to reduce the resource
  config file size, and patch, and
- Patch 3-4 make the resource config in a single write instead of
  individual writes.

Tested this for a PVE 8.4 to PVE 9.1 3-node cluster upgrade.

Daniel Kral (4):
  fix #7133: manager: group migration: skip update for resources without
    group
  manager: group migration: write only non-default failback values
  config, env: allow bulk updates with update_service_config
  manager: group migration: bulk update changes to resource config

 src/PVE/API2/HA/Resources.pm |  8 ++++-
 src/PVE/HA/Config.pm         | 57 +++++++++++++++++++++---------------
 src/PVE/HA/Env.pm            |  4 +--
 src/PVE/HA/Env/PVE2.pm       |  4 +--
 src/PVE/HA/Manager.pm        | 15 ++++++++--
 src/PVE/HA/Sim/Env.pm        |  4 +--
 src/PVE/HA/Sim/Hardware.pm   | 19 +++++++++---
 7 files changed, 74 insertions(+), 37 deletions(-)

-- 
2.47.3



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

[pve-devel] [PATCH ha-manager v2 1/4] fix #7133: manager: group migration: skip update for resources without group

[Daniel Kral]

The HA group migration is executed if the HA groups config is still
present in the HA config directory and individually updates every HA
resource, even if none of those are part of a HA group.

The call to update_service_config(...) for the HA resources without
group assignments cause unnecessary updates to the config and can become
costly with higher HA resource counts, which might prevent the CRM to
update its watchdog in time, so skip these updates.

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/HA/Manager.pm | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index f5843dd4..80b29966 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -580,6 +580,9 @@ my $migrate_group_persistently = sub {
 
         PVE::HA::Groups::migrate_groups_to_resources($groups, $resources);
         for my $sid (keys %$resources) {
+            # prevent unnecessary updates for HA resources that do not change
+            next if !defined($resources->{$sid}->{group});
+
             my $param = { failback => $resources->{$sid}->{failback} };
 
             $haenv->update_service_config($sid, $param, 'group');
-- 
2.47.3



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

[pve-devel] [PATCH ha-manager v2 2/4] manager: group migration: write only non-default failback values

[Daniel Kral]

The failback flag for HA resources is set by default, so it is wasteful
to write the default value in the resources config.

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/HA/Manager.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index 80b29966..c9d1dde5 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -583,7 +583,8 @@ my $migrate_group_persistently = sub {
             # prevent unnecessary updates for HA resources that do not change
             next if !defined($resources->{$sid}->{group});
 
-            my $param = { failback => $resources->{$sid}->{failback} };
+            my $param = {};
+            $param->{failback} = 0 if !$resources->{$sid}->{failback};
 
             $haenv->update_service_config($sid, $param, 'group');
         }
-- 
2.47.3



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

[pve-devel] [PATCH ha-manager v2 3/4] config, env: allow bulk updates with update_service_config

[Daniel Kral]

This is used by the next patch to improve the migration of HA groups to
HA node affinity rules.

This was the more straightforward approach instead of duplicating the
logic into another new virtual subroutine and can be reverted back to
the original form if the bulk capability is not needed after the HA
group migration code is removed in PVE 10.

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes v1 -> v2:
 - pull $delete into update_{service,resources}_config's $changes
   parameter
 - sort the keys when iterating over $changes

 src/PVE/API2/HA/Resources.pm |  8 ++++-
 src/PVE/HA/Config.pm         | 57 +++++++++++++++++++++---------------
 src/PVE/HA/Env.pm            |  4 +--
 src/PVE/HA/Env/PVE2.pm       |  4 +--
 src/PVE/HA/Manager.pm        | 14 ++++++---
 src/PVE/HA/Sim/Env.pm        |  4 +--
 src/PVE/HA/Sim/Hardware.pm   | 19 +++++++++---
 7 files changed, 72 insertions(+), 38 deletions(-)

diff --git a/src/PVE/API2/HA/Resources.pm b/src/PVE/API2/HA/Resources.pm
index 791cd94d..29181931 100644
--- a/src/PVE/API2/HA/Resources.pm
+++ b/src/PVE/API2/HA/Resources.pm
@@ -266,7 +266,13 @@ __PACKAGE__->register_method({
 
         check_service_state($sid, $param->{state});
 
-        PVE::HA::Config::update_resources_config($sid, $param, $delete, $digest);
+        my $changes = {
+            $sid => {
+                param => $param,
+                delete => $delete,
+            },
+        };
+        PVE::HA::Config::update_resources_config($changes, $digest);
 
         return undef;
     },
diff --git a/src/PVE/HA/Config.pm b/src/PVE/HA/Config.pm
index 04e039e0..844ab1b1 100644
--- a/src/PVE/HA/Config.pm
+++ b/src/PVE/HA/Config.pm
@@ -136,37 +136,48 @@ sub read_and_check_resources_config {
     return wantarray ? ($conf, $res->{digest}) : $conf;
 }
 
+my sub update_single_resource_config_inplace {
+    my ($cfg, $sid, $param, $delete) = @_;
+
+    ($sid, my $type, my $name) = parse_sid($sid);
+
+    my $scfg = $cfg->{ids}->{$sid}
+        || die "no such resource '$sid'\n";
+
+    my $plugin = PVE::HA::Resources->lookup($scfg->{type});
+    my $opts = $plugin->check_config($sid, $param, 0, 1);
+
+    foreach my $k (%$opts) {
+        $scfg->{$k} = $opts->{$k};
+    }
+
+    if ($delete) {
+        my $options = $plugin->private()->{options}->{$type};
+        foreach my $k (PVE::Tools::split_list($delete)) {
+            my $d = $options->{$k}
+                || die "no such option '$k'\n";
+            die "unable to delete required option '$k'\n"
+                if !$d->{optional};
+            die "unable to delete fixed option '$k'\n"
+                if $d->{fixed};
+            delete $scfg->{$k};
+        }
+    }
+}
+
 sub update_resources_config {
-    my ($sid, $param, $delete, $digest) = @_;
+    my ($changes, $digest) = @_;
 
     lock_ha_domain(
         sub {
             my $cfg = read_resources_config();
-            ($sid, my $type, my $name) = parse_sid($sid);
-
             PVE::SectionConfig::assert_if_modified($cfg, $digest);
 
-            my $scfg = $cfg->{ids}->{$sid}
-                || die "no such resource '$sid'\n";
+            for my $sid (sort keys %$changes) {
+                my $param = $changes->{$sid}->{param} // {};
+                my $delete = $changes->{$sid}->{delete};
 
-            my $plugin = PVE::HA::Resources->lookup($scfg->{type});
-            my $opts = $plugin->check_config($sid, $param, 0, 1);
-
-            foreach my $k (%$opts) {
-                $scfg->{$k} = $opts->{$k};
-            }
-
-            if ($delete) {
-                my $options = $plugin->private()->{options}->{$type};
-                foreach my $k (PVE::Tools::split_list($delete)) {
-                    my $d = $options->{$k}
-                        || die "no such option '$k'\n";
-                    die "unable to delete required option '$k'\n"
-                        if !$d->{optional};
-                    die "unable to delete fixed option '$k'\n"
-                        if $d->{fixed};
-                    delete $scfg->{$k};
-                }
+                update_single_resource_config_inplace($cfg, $sid, $param, $delete);
             }
 
             write_resources_config($cfg);
diff --git a/src/PVE/HA/Env.pm b/src/PVE/HA/Env.pm
index 4282d33f..64cf3ea5 100644
--- a/src/PVE/HA/Env.pm
+++ b/src/PVE/HA/Env.pm
@@ -95,9 +95,9 @@ sub read_service_config {
 }
 
 sub update_service_config {
-    my ($self, $sid, $param, $delete) = @_;
+    my ($self, $changes) = @_;
 
-    return $self->{plug}->update_service_config($sid, $param, $delete);
+    return $self->{plug}->update_service_config($changes);
 }
 
 sub write_service_config {
diff --git a/src/PVE/HA/Env/PVE2.pm b/src/PVE/HA/Env/PVE2.pm
index 9d0dd22b..37720f72 100644
--- a/src/PVE/HA/Env/PVE2.pm
+++ b/src/PVE/HA/Env/PVE2.pm
@@ -140,9 +140,9 @@ sub read_service_config {
 }
 
 sub update_service_config {
-    my ($self, $sid, $param, $delete) = @_;
+    my ($self, $changes) = @_;
 
-    return PVE::HA::Config::update_resources_config($sid, $param, $delete);
+    return PVE::HA::Config::update_resources_config($changes);
 }
 
 sub write_service_config {
diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index c9d1dde5..36a50a4e 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -583,10 +583,11 @@ my $migrate_group_persistently = sub {
             # prevent unnecessary updates for HA resources that do not change
             next if !defined($resources->{$sid}->{group});
 
-            my $param = {};
-            $param->{failback} = 0 if !$resources->{$sid}->{failback};
+            my $changes = {};
+            $changes->{$sid} = { param => {}, delete => 'group' };
+            $changes->{$sid}->{param}->{failback} = 0 if !$resources->{$sid}->{failback};
 
-            $haenv->update_service_config($sid, $param, 'group');
+            $haenv->update_service_config($changes);
         }
         $haenv->log('notice', "ha groups migration: migration to resources config successful");
 
@@ -1060,7 +1061,12 @@ sub next_state_started {
                     );
                 }
                 &$change_service_state($self, $sid, 'request_stop', timeout => $timeout);
-                $haenv->update_service_config($sid, { 'state' => 'stopped' });
+                my $changes = {
+                    $sid => {
+                        param => { state => 'stopped' },
+                    },
+                };
+                $haenv->update_service_config($changes);
             } else {
                 $haenv->log('err', "unknown command '$cmd' for service '$sid'");
             }
diff --git a/src/PVE/HA/Sim/Env.pm b/src/PVE/HA/Sim/Env.pm
index 1d70026e..32b5224c 100644
--- a/src/PVE/HA/Sim/Env.pm
+++ b/src/PVE/HA/Sim/Env.pm
@@ -212,9 +212,9 @@ sub read_service_config {
 }
 
 sub update_service_config {
-    my ($self, $sid, $param, $delete) = @_;
+    my ($self, $changes) = @_;
 
-    return $self->{hardware}->update_service_config($sid, $param, $delete);
+    return $self->{hardware}->update_service_config($changes);
 }
 
 sub write_service_config {
diff --git a/src/PVE/HA/Sim/Hardware.pm b/src/PVE/HA/Sim/Hardware.pm
index 0848d18a..97ada580 100644
--- a/src/PVE/HA/Sim/Hardware.pm
+++ b/src/PVE/HA/Sim/Hardware.pm
@@ -114,10 +114,8 @@ sub read_service_config {
     return $conf;
 }
 
-sub update_service_config {
-    my ($self, $sid, $param, $delete) = @_;
-
-    my $conf = $self->read_service_config();
+my sub update_single_service_config_inplace {
+    my ($conf, $sid, $param, $delete) = @_;
 
     my $sconf = $conf->{$sid} || die "no such resource '$sid'\n";
 
@@ -130,6 +128,19 @@ sub update_service_config {
             delete $sconf->{$k};
         }
     }
+}
+
+sub update_service_config {
+    my ($self, $changes) = @_;
+
+    my $conf = $self->read_service_config();
+
+    for my $sid (sort keys %$changes) {
+        my $param = $changes->{$sid}->{param} // {};
+        my $delete = $changes->{$sid}->{delete};
+
+        update_single_service_config_inplace($conf, $sid, $param, $delete);
+    }
 
     $self->write_service_config($conf);
 }
-- 
2.47.3



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

[pve-devel] [PATCH ha-manager v2 4/4] manager: group migration: bulk update changes to resource config

[Daniel Kral]

The migration process from HA groups to HA rules might require a lot of
small updates to individual HA resource configs. These updates have been
done per-HA resource, which is quite inefficient and can cause the CRM
to fail to update its watchdog in time.

Even though this is a one-off procedure, it is part of the HA Manager
state machine and must be done in a single action to prevent reading
from an partially updated resource config, i.e., no forking or splitting
the task over multiple HA Manager rounds.

Therefore use the bulk capability of update_service_config(...) to
remove the group fields and migrate the failback flag for all HA
resources in a single read-update-write operation.

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes v1 -> v2:
 - adapted to changes in previous patch

 src/PVE/HA/Manager.pm | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index 36a50a4e..2e31296f 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -579,16 +579,15 @@ my $migrate_group_persistently = sub {
         $haenv->log('notice', "ha groups migration: migration to rules config successful");
 
         PVE::HA::Groups::migrate_groups_to_resources($groups, $resources);
+        my $changes = {};
         for my $sid (keys %$resources) {
             # prevent unnecessary updates for HA resources that do not change
             next if !defined($resources->{$sid}->{group});
 
-            my $changes = {};
             $changes->{$sid} = { param => {}, delete => 'group' };
             $changes->{$sid}->{param}->{failback} = 0 if !$resources->{$sid}->{failback};
-
-            $haenv->update_service_config($changes);
         }
+        $haenv->update_service_config($changes);
         $haenv->log('notice', "ha groups migration: migration to resources config successful");
 
         $haenv->delete_group_config();
-- 
2.47.3



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

[pve-devel] applied: [PATCH-SERIES ha-manager v2 0/4] fix #7133 and improve HA group migration

[Fiona Ebner]

On Tue, 20 Jan 2026 09:47:52 +0100, Daniel Kral wrote:
> Changes from v1 to v2 (Thanks @Fiona!):
>     - pull $delete parameter into $changes hash in
>       update_{service,resources}_config
>     - sort the HA resources' sids when iterating over the $changes hash
> 
> Tested the changes for v2 again with:
>     - creating a groups.cfg and checking whether the groups have been
>       correctly migrated,
>     - changing a HA resource to stopped state changes the config too, &
>     - changing a HA resource's config through the API reflects the
>       changes in the resources.cfg.
> 
> [...]

Applied, thanks!

[1/4] fix #7133: manager: group migration: skip update for resources without group
      commit: 2e22c2a25794d5115d99952d7240a7b4ce987bd2
[2/4] manager: group migration: write only non-default failback values
      commit: 63c705e52dc97cde7a7e330cc8b3234b3b15965e
[3/4] config, env: allow bulk updates with update_service_config
      commit: e59a7741a97a93242886f347c251773962f36250
[4/4] manager: group migration: bulk update changes to resource config
      commit: 0bb26b676490d8af5ccf01843658acc4b441934a


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