From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 1351498688 for ; Wed, 15 Nov 2023 14:51:43 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A3ACB6F8E for ; Wed, 15 Nov 2023 14:51:42 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 15 Nov 2023 14:51:40 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id F28B74316C for ; Wed, 15 Nov 2023 14:51:39 +0100 (CET) From: Dominik Csapak To: pve-devel@lists.proxmox.com Date: Wed, 15 Nov 2023 14:51:35 +0100 Message-Id: <20231115135137.2265822-4-d.csapak@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20231115135137.2265822-1-d.csapak@proxmox.com> References: <20231115135137.2265822-1-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 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 SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: [pve-devel] [PATCH common v2 3/4] section config: allow separated property lists for plugins 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: , X-List-Received-Date: Wed, 15 Nov 2023 13:51:43 -0000 when using 'init(1)'. This saves the property lists per type instead of a big one, and using create/updateSchema creates a new schema with the options as 'oneOf' and/or 'instance-types' (depending if the schemas match). with that, we change how we work with the options hash: it's not needed anymore to specify options that are specified in the type specific propertyList, except if it's 'fixed => 1' (since that does not exist in the schema) Signed-off-by: Dominik Csapak --- changes from v1: * don't use the __global hack anymore, we now construct the plugin-specific options hash in `init` * all 'global' properties are added in the api as they are * improved the compare function and moved to PVE::Tools as a generic recursive variable compare * removed copy_property * optimized the genrated schema: when a property is used by all types, the 'type-property' and 'instance-types' is removed, this makes the docs a bit shorter when many of such options exist * added an is_separated call src/PVE/SectionConfig.pm | 259 ++++++++++++++++++++++++++++++--------- src/PVE/Tools.pm | 33 +++++ 2 files changed, 235 insertions(+), 57 deletions(-) diff --git a/src/PVE/SectionConfig.pm b/src/PVE/SectionConfig.pm index 5690476..eb39b41 100644 --- a/src/PVE/SectionConfig.pm +++ b/src/PVE/SectionConfig.pm @@ -8,6 +8,7 @@ use Digest::SHA; use PVE::Exception qw(raise_param_exc); use PVE::JSONSchema qw(get_standard_option); +use PVE::Tools qw(compare_deeply); my $defaultData = { options => {}, @@ -51,6 +52,61 @@ sub plugindata { return {}; } +sub is_separated { + my ($class) = @_; + + return $class->private()->{propertyListSeparated}->%* > 0; +} + +my sub cmp_property { + my ($a, $b, $skip_opts) = @_; + + my $merged = {$a->%*, $b->%*}; + for my $opt ($skip_opts->@*) { + delete $merged->{$opt}; + } + for my $opt (keys $merged->%*) { + return 0 if !compare_deeply($a->{$opt}, $b->{$opt}); + } + + return 1; +}; + +my sub add_property { + my ($props, $key, $prop, $type) = @_; + + if (!defined($props->{$key})) { + $props->{$key} = $prop; + return; + } + + if (!defined($props->{$key}->{oneOf})) { + if (cmp_property($props->{$key}, $prop, ['instance-types'])) { + push $props->{$key}->{'instance-types'}->@*, $type; + } else { + my $new_prop = delete $props->{$key}; + delete $new_prop->{'type-property'}; + delete $prop->{'type-property'}; + $props->{$key} = { + 'type-property' => 'type', + oneOf => [ + $new_prop, + $prop, + ], + }; + } + } else { + for my $existing_prop ($props->{$key}->{oneOf}->@*) { + if (cmp_property($existing_prop, $prop, ['instance-types', 'type-property'])) { + push $existing_prop->{'instance-types'}->@*, $type; + return; + } + } + + push $props->{$key}->{oneOf}->@*, $prop; + } +}; + sub createSchema { my ($class, $skip_type, $base) = @_; @@ -60,42 +116,61 @@ sub createSchema { my $props = $base || {}; - my $copy_property = sub { - my ($src) = @_; - - my $res = {}; - foreach my $k (keys %$src) { - $res->{$k} = $src->{$k}; - } + if (!$class->is_separated()) { + foreach my $p (keys %$propertyList) { + next if $skip_type && $p eq 'type'; - return $res; - }; + if (!$propertyList->{$p}->{optional}) { + $props->{$p} = $propertyList->{$p}; + next; + } - foreach my $p (keys %$propertyList) { - next if $skip_type && $p eq 'type'; + my $required = 1; - if (!$propertyList->{$p}->{optional}) { - $props->{$p} = $propertyList->{$p}; - next; - } + my $copts = $class->options(); + $required = 0 if defined($copts->{$p}) && $copts->{$p}->{optional}; - my $required = 1; - - my $copts = $class->options(); - $required = 0 if defined($copts->{$p}) && $copts->{$p}->{optional}; + foreach my $t (keys %$plugins) { + my $opts = $pdata->{options}->{$t} || {}; + $required = 0 if !defined($opts->{$p}) || $opts->{$p}->{optional}; + } - foreach my $t (keys %$plugins) { - my $opts = $pdata->{options}->{$t} || {}; - $required = 0 if !defined($opts->{$p}) || $opts->{$p}->{optional}; + if ($required) { + # make a copy, because we modify the optional property + my $res = {$propertyList->{$p}->%*}; # shallow copy + $res->{optional} = 0; + $props->{$p} = $res; + } else { + $props->{$p} = $propertyList->{$p}; + } } - - if ($required) { - # make a copy, because we modify the optional property - my $res = &$copy_property($propertyList->{$p}); - $res->{optional} = 0; - $props->{$p} = $res; - } else { - $props->{$p} = $propertyList->{$p}; + } else { + for my $type (sort keys %$plugins) { + my $opts = $pdata->{options}->{$type} || {}; + for my $key (sort keys $opts->%*) { + my $schema = $class->get_property_schema($type, $key); + my $prop = {$schema->%*}; + $prop->{'instance-types'} = [$type]; + $prop->{'type-property'} = 'type'; + $prop->{optional} = 1 if $opts->{$key}->{optional}; + + add_property($props, $key, $prop, $type); + } + } + # add remaining global properties + for my $opt (keys $propertyList->%*) { + next if $props->{$opt}; + $props->{$opt} = {$propertyList->{$opt}->%*}; + } + for my $opt (keys $props->%*) { + if (my $necessaryTypes = $props->{$opt}->{'instance-types'}) { + if ($necessaryTypes->@* == scalar(keys $plugins->%*)) { + delete $props->{$opt}->{'instance-types'}; + delete $props->{$opt}->{'type-property'}; + } else { + $props->{$opt}->{optional} = 1; + } + } } } @@ -117,30 +192,61 @@ sub updateSchema { my $filter_type = $single_class ? $class->type() : undef; - foreach my $p (keys %$propertyList) { - next if $p eq 'type'; + if (!$class->is_separated()) { + foreach my $p (keys %$propertyList) { + next if $p eq 'type'; - my $copts = $class->options(); + my $copts = $class->options(); - next if defined($filter_type) && !defined($copts->{$p}); + next if defined($filter_type) && !defined($copts->{$p}); - if (!$propertyList->{$p}->{optional}) { - $props->{$p} = $propertyList->{$p}; - next; - } + if (!$propertyList->{$p}->{optional}) { + $props->{$p} = $propertyList->{$p}; + next; + } - my $modifyable = 0; + my $modifyable = 0; - $modifyable = 1 if defined($copts->{$p}) && !$copts->{$p}->{fixed}; + $modifyable = 1 if defined($copts->{$p}) && !$copts->{$p}->{fixed}; - foreach my $t (keys %$plugins) { - my $opts = $pdata->{options}->{$t} || {}; - next if !defined($opts->{$p}); - $modifyable = 1 if !$opts->{$p}->{fixed}; + foreach my $t (keys %$plugins) { + my $opts = $pdata->{options}->{$t} || {}; + next if !defined($opts->{$p}); + $modifyable = 1 if !$opts->{$p}->{fixed}; + } + next if !$modifyable; + + $props->{$p} = $propertyList->{$p}; + } + } else { + for my $type (sort keys %$plugins) { + my $opts = $pdata->{options}->{$type} || {}; + for my $key (sort keys $opts->%*) { + next if $opts->{$key}->{fixed}; + + my $schema = $class->get_property_schema($type, $key); + my $prop = {$schema->%*}; + $prop->{'instance-types'} = [$type]; + $prop->{'type-property'} = 'type'; + $prop->{optional} = 1; + + add_property($props, $key, $prop, $type); + } } - next if !$modifyable; - $props->{$p} = $propertyList->{$p}; + for my $opt (keys $propertyList->%*) { + next if $props->{$opt}; + $props->{$opt} = {$propertyList->{$opt}->%*}; + } + + for my $opt (keys $props->%*) { + if (my $necessaryTypes = $props->{$opt}->{'instance-types'}) { + if ($necessaryTypes->@* == scalar(keys $plugins->%*)) { + delete $props->{$opt}->{'instance-types'}; + delete $props->{$opt}->{'type-property'}; + } + } + } } $props->{digest} = get_standard_option('pve-config-digest'); @@ -160,22 +266,28 @@ sub updateSchema { } sub init { - my ($class) = @_; + my ($class, $separate_properties) = @_; my $pdata = $class->private(); - foreach my $k (qw(options plugins plugindata propertyList)) { + foreach my $k (qw(options plugins plugindata propertyList propertyListSeparated)) { $pdata->{$k} = {} if !$pdata->{$k}; } my $plugins = $pdata->{plugins}; my $propertyList = $pdata->{propertyList}; + my $propertyListSeparated = $pdata->{propertyListSeparated}; foreach my $type (keys %$plugins) { my $props = $plugins->{$type}->properties(); foreach my $p (keys %$props) { - die "duplicate property '$p'" if defined($propertyList->{$p}); - my $res = $propertyList->{$p} = {}; + my $res; + if ($separate_properties) { + $res = $propertyListSeparated->{$type}->{$p} = {}; + } else { + die "duplicate property '$p'" if defined($propertyList->{$p}); + $res = $propertyList->{$p} = {}; + } my $data = $props->{$p}; for my $a (keys %$data) { $res->{$a} = $data->{$a}; @@ -187,8 +299,23 @@ sub init { foreach my $type (keys %$plugins) { my $opts = $plugins->{$type}->options(); foreach my $p (keys %$opts) { - die "undefined property '$p'" if !$propertyList->{$p}; + my $prop; + if ($separate_properties) { + $prop = $propertyListSeparated->{$type}->{$p}; + } + $prop //= $propertyList->{$p}; + die "undefined property '$p'" if !$prop; } + + # automatically the properties to options (if not specified explicitely) + if ($separate_properties) { + foreach my $p (keys $propertyListSeparated->{$type}->%*) { + next if $opts->{$p}; + $opts->{$p} = {}; + $opts->{$p}->{optional} = 1 if $propertyListSeparated->{$type}->{$p}->{optional}; + } + } + $pdata->{options}->{$type} = $opts; } @@ -237,11 +364,12 @@ sub check_value { return $value if $key eq 'type' && $type eq $value; my $opts = $pdata->{options}->{$type}; + die "unknown section type '$type'\n" if !$opts; die "unexpected property '$key'\n" if !defined($opts->{$key}); - my $schema = $pdata->{propertyList}->{$key}; + my $schema = $class->get_property_schema($type, $key); die "unknown property type\n" if !$schema; my $ct = $schema->{type}; @@ -295,6 +423,20 @@ sub format_section_header { return "$type: $sectionId\n"; } +sub get_property_schema { + my ($class, $type, $key) = @_; + + my $pdata = $class->private(); + my $opts = $pdata->{options}->{$type}; + + my $schema; + if ($class->is_separated()) { + $schema = $pdata->{propertyListSeparated}->{$type}->{$key}; + } + $schema //= $pdata->{propertyList}->{$key}; + + return $schema; +} sub parse_config { my ($class, $filename, $raw, $allow_unknown) = @_; @@ -322,7 +464,7 @@ sub parse_config { my $is_array = sub { my ($type, $key) = @_; - my $schema = $pdata->{propertyList}->{$key}; + my $schema = $class->get_property_schema($type, $key); die "unknown property type\n" if !$schema; return $schema->{type} eq 'array'; @@ -494,7 +636,6 @@ sub write_config { my ($class, $filename, $cfg, $allow_unknown) = @_; my $pdata = $class->private(); - my $propertyList = $pdata->{propertyList}; my $out = ''; @@ -516,6 +657,7 @@ sub write_config { my $scfg = $ids->{$sectionId}; my $type = $scfg->{type}; my $opts = $pdata->{options}->{$type}; + my $global_opts = $pdata->{options}->{__global}; die "unknown section type '$type'\n" if !$opts && !$allow_unknown; @@ -545,7 +687,8 @@ sub write_config { if ($scfg->{comment} && !$done_hash->{comment}) { my $k = 'comment'; my $v = $class->encode_value($type, $k, $scfg->{$k}); - $data .= &$format_config_line($propertyList->{$k}, $k, $v); + my $prop = $class->get_property_schema($type, $k); + $data .= &$format_config_line($prop, $k, $v); } $data .= "\tdisable\n" if $scfg->{disable} && !$done_hash->{disable}; @@ -562,7 +705,8 @@ sub write_config { die "section '$sectionId' - missing value for required option '$k'\n" if !defined ($v); $v = $class->encode_value($type, $k, $v); - $data .= &$format_config_line($propertyList->{$k}, $k, $v); + my $prop = $class->get_property_schema($type, $k); + $data .= &$format_config_line($prop, $k, $v); } foreach my $k (@option_keys) { @@ -570,7 +714,8 @@ sub write_config { my $v = $scfg->{$k}; next if !defined($v); $v = $class->encode_value($type, $k, $v); - $data .= &$format_config_line($propertyList->{$k}, $k, $v); + my $prop = $class->get_property_schema($type, $k); + $data .= &$format_config_line($prop, $k, $v); } $out .= "$data\n"; diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm index b3af2c6..23ed069 100644 --- a/src/PVE/Tools.pm +++ b/src/PVE/Tools.pm @@ -36,6 +36,7 @@ no warnings 'portable'; # Support for 64-bit ints required our @EXPORT_OK = qw( $IPV6RE $IPV4RE +compare_deeply lock_file lock_file_full run_command @@ -2150,4 +2151,36 @@ sub get_file_hash { return lc($digest); } +# compare two perl variables recursively, so this works for scalars, nested +# hashes and nested arrays +sub compare_deeply { + my ($a, $b) = @_; + + return 0 if defined($a) != defined($b); + return 1 if !defined($a); # both are undef + + my $ref_a = ref($a); + my $ref_b = ref($b); + + # scalar case + return 0 if !$ref_a && !$ref_b && "$a" ne "$b"; + + # different types + return 0 if $ref_a ne $ref_b; + + if ($ref_a eq 'HASH') { + return 0 if $a->%* != $b->%*; + for my $opt (keys $a->%*) { + return 0 if !compare_deeply->($a->{$opt}, $b->{$opt}); + } + } elsif ($ref_a eq 'ARRAY') { + return 0 if $a->@* != $b->@*; + for (my $i = 0; $i < $a->@*; $i++) { + return 0 if !compare_deeply->($a->[$i], $b->[$i]); + } + } + + return 1; +} + 1; -- 2.30.2