From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH v3 manager 2/4] vzdump: use per-property fallback for performance settings
Date: Tue, 7 Nov 2023 14:49:25 +0100 [thread overview]
Message-ID: <20231107134928.81303-3-f.ebner@proxmox.com> (raw)
In-Reply-To: <20231107134928.81303-1-f.ebner@proxmox.com>
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 <f.ebner@proxmox.com>
---
New in v3.
PVE/VZDump.pm | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 182fb80b..9a82260f 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})) {
@@ -575,8 +590,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
next prev parent reply other threads:[~2023-11-07 13:50 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-07 13:49 [pve-devel] [PATCH-SERIES v3 manager/docs] close #4513: add performance tab for backup jobs Fiona Ebner
2023-11-07 13:49 ` [pve-devel] [PATCH v3 manager 1/4] vzdump: actually honor schema defaults for performance Fiona Ebner
2023-11-07 13:49 ` Fiona Ebner [this message]
2023-11-07 13:49 ` [pve-devel] [PATCH v3 manager 3/4] close #4513: ui: backup job: add performance tab Fiona Ebner
2023-11-07 13:49 ` [pve-devel] [PATCH v3 manager 4/4] ui: backup job: disable zstd thread count field when zstd isn't used Fiona Ebner
2023-11-07 13:49 ` [pve-devel] [PATCH v3 docs 5/5] backup: update information about performance settings Fiona Ebner
2024-03-25 14:09 ` [pve-devel] [PATCH-SERIES v3 manager/docs] close #4513: add performance tab for backup jobs Fiona Ebner
2024-04-16 10:16 ` Thomas Lamprecht
2024-04-16 12:11 ` Fiona Ebner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231107134928.81303-3-f.ebner@proxmox.com \
--to=f.ebner@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox