]> git.proxmox.com Git - pve-manager.git/commitdiff
vzdump: use per-property fallback for performance settings
authorFiona Ebner <f.ebner@proxmox.com>
Tue, 16 Apr 2024 12:09:52 +0000 (14:09 +0200)
committerThomas Lamprecht <t.lamprecht@proxmox.com>
Wed, 17 Apr 2024 14:05:09 +0000 (16:05 +0200)
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>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
PVE/VZDump.pm

index b084fb5df75a56ac4b525f8b2cfb127b30e2cd86..02244cd795c6e6d12dc54d90dad59bcc8dfa10ad 100644 (file)
@@ -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});
        }
     }