From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id ED3B51FF143 for ; Mon, 19 Jan 2026 12:49:08 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 96F1D1C876; Mon, 19 Jan 2026 12:49:15 +0100 (CET) Message-ID: <1aa14390-ebc4-44e8-874e-292354ad2468@proxmox.com> Date: Mon, 19 Jan 2026 12:48:41 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Proxmox VE development discussion , Daniel Kral References: <20251215102409.142350-1-d.kral@proxmox.com> <20251215102409.142350-4-d.kral@proxmox.com> Content-Language: en-US From: Fiona Ebner In-Reply-To: <20251215102409.142350-4-d.kral@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1768823268273 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.017 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH ha-manager 3/4] config, env: allow bulk updates with update_resources_config X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" Am 15.12.25 um 11:24 AM schrieb Daniel Kral: > 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}->{$ytype}; > + 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) = @_; Nit: I feel like the $delete belongs inside the $changes for a clean interface. So something like $changes = { $sid => { param => $param, delete => $delete, } } I know the single caller using bulk won't need it, but the current interface does feel slightly off to me. What do you think? > > 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) { Nit: I'd prefer to iterate after 'sort' > + my $param = $changes->{$sid}; > + update_single_resource_config_inplace($cfg, $sid, $param, $delete); > } > > write_resources_config($cfg); ---snip--- > 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) { Nit: I'd prefer to iterate after 'sort' > + my $param = $changes->{$sid}; > + update_single_service_config_inplace($conf, $sid, $param, $delete); > + } > > $self->write_service_config($conf); > } _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel