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

[Daniel Kral <d.kral@proxmox.com>]

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

This is 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>
---
I left the delete parameter in the signature instead of putting it into
the $changes items too to remain closer to the original method
signature, but that could be moved there too if it's cleaner.

 src/PVE/API2/HA/Resources.pm |  3 +-
 src/PVE/HA/Config.pm         | 57 +++++++++++++++++++++---------------
 src/PVE/HA/Env.pm            |  4 +--
 src/PVE/HA/Env/PVE2.pm       |  4 +--
 src/PVE/HA/Manager.pm        | 10 ++++---
 src/PVE/HA/Sim/Env.pm        |  4 +--
 src/PVE/HA/Sim/Hardware.pm   | 17 ++++++++---
 7 files changed, 60 insertions(+), 39 deletions(-)

diff --git a/src/PVE/API2/HA/Resources.pm b/src/PVE/API2/HA/Resources.pm
index b95c0e1f..581f4c6a 100644
--- a/src/PVE/API2/HA/Resources.pm
+++ b/src/PVE/API2/HA/Resources.pm
@@ -266,7 +266,8 @@ __PACKAGE__->register_method({
 
         check_service_state($sid, $param->{state});
 
-        PVE::HA::Config::update_resources_config($sid, $param, $delete, $digest);
+        my $changes = { $sid => $param };
+        PVE::HA::Config::update_resources_config($changes, $delete, $digest);
 
         return undef;
     },
diff --git a/src/PVE/HA/Config.pm b/src/PVE/HA/Config.pm
index 04e039e0..159798a7 100644
--- a/src/PVE/HA/Config.pm
+++ b/src/PVE/HA/Config.pm
@@ -136,37 +136,46 @@ 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, $delete, $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";
-
-            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};
-                }
+            for my $sid (keys %$changes) {
+                my $param = $changes->{$sid};
+                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..bfe93f12 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, $delete) = @_;
 
-    return $self->{plug}->update_service_config($sid, $param, $delete);
+    return $self->{plug}->update_service_config($changes, $delete);
 }
 
 sub write_service_config {
diff --git a/src/PVE/HA/Env/PVE2.pm b/src/PVE/HA/Env/PVE2.pm
index 9d0dd22b..7bec601d 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, $delete) = @_;
 
-    return PVE::HA::Config::update_resources_config($sid, $param, $delete);
+    return PVE::HA::Config::update_resources_config($changes, $delete);
 }
 
 sub write_service_config {
diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index c9d1dde5..f55d5870 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} = {};
+            $changes->{$sid}->{failback} = 0 if !$resources->{$sid}->{failback};
 
-            $haenv->update_service_config($sid, $param, 'group');
+            $haenv->update_service_config($changes, 'group');
         }
         $haenv->log('notice', "ha groups migration: migration to resources config successful");
 
@@ -1060,7 +1061,8 @@ sub next_state_started {
                     );
                 }
                 &$change_service_state($self, $sid, 'request_stop', timeout => $timeout);
-                $haenv->update_service_config($sid, { 'state' => 'stopped' });
+                my $changes = { $sid => { 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..0e7e0e5d 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, $delete) = @_;
 
-    return $self->{hardware}->update_service_config($sid, $param, $delete);
+    return $self->{hardware}->update_service_config($changes, $delete);
 }
 
 sub write_service_config {
diff --git a/src/PVE/HA/Sim/Hardware.pm b/src/PVE/HA/Sim/Hardware.pm
index 0848d18a..b19a23b4 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,17 @@ sub update_service_config {
             delete $sconf->{$k};
         }
     }
+}
+
+sub update_service_config {
+    my ($self, $changes, $delete) = @_;
+
+    my $conf = $self->read_service_config();
+
+    for my $sid (keys %$changes) {
+        my $param = $changes->{$sid};
+        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