[pve-devel] [PATCH manager] restore default value of 0 for remove/maxfiles

[pve-devel] [PATCH manager] restore default value of 0 for remove/maxfiles

[Stefan Reiter]

If neither the 'remove' option of vzdump nor the 'maxfiles' option in
the storage config are set, assume a value of 0, i.e. do not delete
anything and allow unlimited backups.

Restores previous behaviour that was broken in 7ab7d6f15f.

Also fixes a warning about using '== 0' on a non-number type.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 PVE/VZDump.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 6873ceaf..517becb1 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -474,7 +474,7 @@ sub new {
 
 	    if (!defined($opts->{'prune-backups'}) && !defined($opts->{maxfiles})) {
 		$opts->{'prune-backups'} = $info->{'prune-backups'};
-		$opts->{maxfiles} = $info->{maxfiles};
+		$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;
-- 
2.20.1

Re: [pve-devel] [PATCH manager] restore default value of 0 for remove/maxfiles

[Fabian Ebner]

AFAICT the previous behavior is maxfiles = 1 when it's not set anywhere. 
That's the default value in the VZDump schema.

And that should happen a bit below in the code:
     if (!defined($opts->{'prune-backups'})) {
         my $maxfiles = delete $opts->{maxfiles} // $defaults->{maxfiles};
         $opts->{'prune-backups'} = { 'keep-last' => $maxfiles } if 
$maxfiles;

Or is there some weird perl behavior that makes this not work?

Am 05.11.20 um 16:21 schrieb Stefan Reiter:
> If neither the 'remove' option of vzdump nor the 'maxfiles' option in
> the storage config are set, assume a value of 0, i.e. do not delete
> anything and allow unlimited backups.
> 
> Restores previous behaviour that was broken in 7ab7d6f15f.
> 
> Also fixes a warning about using '== 0' on a non-number type.
> 
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
>   PVE/VZDump.pm | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
> index 6873ceaf..517becb1 100644
> --- a/PVE/VZDump.pm
> +++ b/PVE/VZDump.pm
> @@ -474,7 +474,7 @@ sub new {
>   
>   	    if (!defined($opts->{'prune-backups'}) && !defined($opts->{maxfiles})) {
>   		$opts->{'prune-backups'} = $info->{'prune-backups'};
> -		$opts->{maxfiles} = $info->{maxfiles};
> +		$opts->{maxfiles} = $info->{maxfiles} // 0;
>   		if ($opts->{maxfiles} == 0) {
Wouldn't adding a definedness check here be sufficient?

>   		    # zero means keep all, so avoid triggering any remove code path to be safe
>   		    $opts->{remove} = 0;
> 

[pve-devel] applied: [PATCH manager] restore default value of 0 for remove/maxfiles

[Thomas Lamprecht]

On 05.11.20 16:21, Stefan Reiter wrote:
> If neither the 'remove' option of vzdump nor the 'maxfiles' option in
> the storage config are set, assume a value of 0, i.e. do not delete
> anything and allow unlimited backups.
> 
> Restores previous behaviour that was broken in 7ab7d6f15f.
> 
> Also fixes a warning about using '== 0' on a non-number type.
> 
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
>  PVE/VZDump.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, thanks!