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 66B75969BA for ; Tue, 16 Apr 2024 14:10:02 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 21E8E1918D for ; Tue, 16 Apr 2024 14:10:02 +0200 (CEST) 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 ; Tue, 16 Apr 2024 14:10:01 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 1558C45029 for ; Tue, 16 Apr 2024 14:10:01 +0200 (CEST) From: Fiona Ebner To: pve-devel@lists.proxmox.com Date: Tue, 16 Apr 2024 14:09:52 +0200 Message-Id: <20240416120957.75269-3-f.ebner@proxmox.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240416120957.75269-1-f.ebner@proxmox.com> References: <20240416120957.75269-1-f.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.070 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 Subject: [pve-devel] [PATCH v4 manager 2/5] vzdump: use per-property fallback for performance settings 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: Tue, 16 Apr 2024 12:10:04 -0000 Currently, fallback for the 'performance' option is done as a whole, taking away flexibility from the user. It also means that when only one of the two sub-properties is specified, the other one will default to the backend (i.e. QEMU or proxmox-backup-client) default rather than the schema default. For the latter point in particular, it can be argued to be incorrect. These limitations will only get worse in the future with more sub-properties. Switch to a per-property fallback mechanism to improve the situation, having each go through the usual preference order (CLI/job > node-wide default > schema default). Technically, this is a breaking change, but pbs-entries-max is rather new and potential for breakage seems rather low. Requirements for breakage: * job (or CLI) that defines only one of the performance options * job also covers a guest where the other performance option applies * the other performance option is defined in the node-wide configuration * the node-wide setting is worse for the job than the implicit backend default (because this change will have the node-wide default win over the implicit backend default). Signed-off-by: Fiona Ebner --- No changes in v4. PVE/VZDump.pm | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm index b084fb5d..02244cd7 100644 --- a/PVE/VZDump.pm +++ b/PVE/VZDump.pm @@ -139,6 +139,17 @@ my sub parse_performance { } } +my sub merge_performance { + my ($prefer, $fallback) = @_; + + my $res = {}; + for my $opt (keys PVE::JSONSchema::get_format('backup-performance')->%*) { + $res->{$opt} = $prefer->{$opt} // $fallback->{$opt} + if defined($prefer->{$opt}) || defined($fallback->{$opt}); + } + return $res; +} + my $parse_prune_backups_maxfiles = sub { my ($param, $kind) = @_; @@ -312,8 +323,12 @@ sub read_vzdump_defaults { $parse_prune_backups_maxfiles->($res, "options in '$fn'"); parse_performance($res); - foreach my $key (keys %$defaults) { - $res->{$key} = $defaults->{$key} if !defined($res->{$key}); + for my $key (keys $defaults->%*) { + if (!defined($res->{$key})) { + $res->{$key} = $defaults->{$key}; + } elsif ($key eq 'performance') { + $res->{$key} = merge_performance($res->{$key}, $defaults->{$key}); + } } if (defined($res->{storage}) && defined($res->{dumpdir})) { @@ -598,8 +613,10 @@ sub new { if ($k eq 'dumpdir' || $k eq 'storage') { $opts->{$k} = $defaults->{$k} if !defined ($opts->{dumpdir}) && !defined ($opts->{storage}); - } else { - $opts->{$k} = $defaults->{$k} if !defined ($opts->{$k}); + } elsif (!defined($opts->{$k})) { + $opts->{$k} = $defaults->{$k}; + } elsif ($k eq 'performance') { + $opts->{$k} = merge_performance($opts->{$k}, $defaults->{$k}); } } -- 2.39.2