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
[email protected]
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
