On 9/30/19 9:22 AM, Fabian Ebner wrote:
> This makes it easier to update the resource configuration from within the
> CRM/LRM stack,
> which is needed for the new 'stop' command.
>
> Signed-off-by: Fabian Ebner <f.eb...@proxmox.com>
> ---
> src/PVE/API2/HA/Resources.pm | 34 +--------------------------------
> src/PVE/HA/Config.pm | 37 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 38 insertions(+), 33 deletions(-)
>
> diff --git a/src/PVE/API2/HA/Resources.pm b/src/PVE/API2/HA/Resources.pm
> index 22d7f28..2b62ee8 100644
> --- a/src/PVE/API2/HA/Resources.pm
> +++ b/src/PVE/API2/HA/Resources.pm
> @@ -237,39 +237,7 @@ __PACKAGE__->register_method ({
>
> check_service_state($sid, $param->{state});
>
> - PVE::HA::Config::lock_ha_domain(
> - sub {
> -
> - my $cfg = PVE::HA::Config::read_resources_config();
> -
> - 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};
> - }
> - }
> -
> - PVE::HA::Config::write_resources_config($cfg)
> -
> - }, "update resource failed");
> + PVE::HA::Config::update_resources_config($digest, $delete, $sid,
> $param);
>
> return undef;
> }});
> diff --git a/src/PVE/HA/Config.pm b/src/PVE/HA/Config.pm
> index ead1ee2..e800154 100644
> --- a/src/PVE/HA/Config.pm
> +++ b/src/PVE/HA/Config.pm
> @@ -125,6 +125,43 @@ sub read_and_check_resources_config {
> return $conf;
> }
>
> +sub update_resources_config {
> + my ($digest, $delete, $sid, $param) = @_;
Hmm, I do not like the order of arguments, IMO it's better to have
optional and less likely ones at the end, so that they can just
be omitted allowing to write
foo("bar");
vs-
foo(undef, undef, "bar");
I'd propose
my ($sid, $param, $delete, $digest) = @_;
The $digest is only for the API, the CRM does not cares as it just
enforces a state from a locked context and does no "read-change-update"
cycle where other changes from other API users could be overwritten.
$delete is also something the API will rather use, in your in-service
usage of this method you have both set to "0" anyway.
Sorry, this could've been said on v1 already, missed it.
> +
> + 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};
> + }
> + }
> +
> + write_resources_config($cfg);
> + }, "update resources config failed");
> +}
> +
> sub parse_sid {
> my ($sid) = @_;
>
>
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
