[pve-devel] [PATCH ha-manager v2 3/4] config, env: allow bulk updates with update_service_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 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