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 211BD6721C for ; Mon, 9 Nov 2020 09:57:09 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 0BDEA1477C for ; Mon, 9 Nov 2020 09:56:39 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 82B9F14765 for ; Mon, 9 Nov 2020 09:56:38 +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 4ECEE45BF9 for ; Mon, 9 Nov 2020 09:56:38 +0100 (CET) From: Fabian Ebner To: pve-devel@lists.proxmox.com Date: Mon, 9 Nov 2020 09:56:33 +0100 Message-Id: <20201109085633.12688-1-f.ebner@proxmox.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.028 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust 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 manager] fix maxfiles behavior 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: Mon, 09 Nov 2020 08:57:09 -0000 Commit 5ba2a605ac14de58572f7b8d6e04b45b34724b0a hard-coded 0 as the default for maxfiles in the --storage case, but the actual default should be the value from read_vzdump_defaults(), which obtains the value from /etc/vzdump.conf or the VZDump schema if the value has not been modified in that file. The initial default from the schema is 1, not 0. Tested on PVE 6.1 to verify that behavior. Move the sanity check for zero-ness to where we have the final value for maxfiles. Like this, we also have an implicit definedness check and more importantly, it is more future-proof in case we ever allow maxfiles 0 in the VZDump schema itself. Also, force conversion to int to be extra safe. Signed-off-by: Fabian Ebner --- @Stefan: I wasn't able to trigger a warning about using '== 0' on a non-number type, the only thing I can get is: Use of uninitialized value in numeric eq (==) Does this patch work with your use case as well or is there something off? PVE/VZDump.pm | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm index 517becb1..40a5a035 100644 --- a/PVE/VZDump.pm +++ b/PVE/VZDump.pm @@ -474,11 +474,7 @@ sub new { if (!defined($opts->{'prune-backups'}) && !defined($opts->{maxfiles})) { $opts->{'prune-backups'} = $info->{'prune-backups'}; - $opts->{maxfiles} = $info->{maxfiles} // 0; - if ($opts->{maxfiles} == 0) { - # zero means keep all, so avoid triggering any remove code path to be safe - $opts->{remove} = 0; - } + $opts->{maxfiles} = $info->{maxfiles}; } } } elsif ($opts->{dumpdir}) { @@ -490,7 +486,13 @@ sub new { if (!defined($opts->{'prune-backups'})) { my $maxfiles = delete $opts->{maxfiles} // $defaults->{maxfiles}; - $opts->{'prune-backups'} = { 'keep-last' => $maxfiles } if $maxfiles; + $maxfiles = int($maxfiles); # shouldn't be necessary, but be safe + if ($maxfiles) { + $opts->{'prune-backups'} = { 'keep-last' => $maxfiles }; + } else { + # maxfiles being zero means keep all, so avoid triggering any remove code path to be safe + $opts->{remove} = 0; + } } if ($opts->{tmpdir} && ! -d $opts->{tmpdir}) { -- 2.20.1