vzdump: add new 'remove' flag to disable automatic file removal

The maxfiles flags is now a storage related flag. So we set that on
the storage definition, and removed it from the scheduled backup GUI.
We use '--remove 0' for 'Backup now', so the user need to remove
old backups himself.

PVE/VZDump.pm

@@ -439,6 +439,9 @@ sub new {
 
     my $defaults = read_vzdump_defaults();
 
+    my $maxfiles = $opts->{maxfiles};
+    $opts->{remove} = 1 if !defined($opts->{remove});
+
     foreach my $k (keys %$defaults) {
 	if ($k eq 'dumpdir' || $k eq 'storage') {
 	    $opts->{$k} = $defaults->{$k} if !defined ($opts->{dumpdir}) &&
@@ -497,6 +500,7 @@ sub new {
     if ($opts->{storage}) {
 	my $info = storage_info ($opts->{storage});
 	$opts->{dumpdir} = $info->{dumpdir};
+	$maxfiles = $info->{maxfiles} if !$maxfiles && $info->{maxfiles};
     } elsif ($opts->{dumpdir}) {
 	die "dumpdir '$opts->{dumpdir}' does not exist\n"
 	    if ! -d $opts->{dumpdir};
@@ -508,6 +512,8 @@ sub new {
 	die "tmpdir '$opts->{tmpdir}' does not exist\n";
     }
 
+    $opts->{maxfiles} = $maxfiles if $maxfiles;
+
     return $self;
 
 }
@@ -661,6 +667,22 @@ sub compressor_info {
 	die "internal error - unknown compression option '$opt_compress'";
     }
 }
+
+sub get_backup_file_list {
+    my ($dir, $bkname, $exclude_fn) = @_;
+
+    my $bklist = [];
+    foreach my $fn (<$dir/${bkname}-*>) {
+	next if $exclude_fn && $fn eq $exclude_fn;
+	if ($fn =~ m!/(${bkname}-(\d{4})_(\d{2})_(\d{2})-(\d{2})_(\d{2})_(\d{2})\.(tgz|(tar(\.(gz|lzo))?)))$!) {
+	    $fn = "$dir/$1"; # untaint
+	    my $t = timelocal ($7, $6, $5, $4, $3 - 1, $2 - 1900);
+	    push @$bklist, [$fn, $t];
+	}
+    }
+
+    return $bklist;
+}
  
 sub exec_backup_task {
     my ($self, $task) = @_;
@@ -692,6 +714,14 @@ sub exec_backup_task {
 	$lt->year + 1900, $lt->mon + 1, $lt->mday, 
 	$lt->hour, $lt->min, $lt->sec;
 
+	my $maxfiles = $opts->{maxfiles};
+
+	if ($maxfiles && !$opts->{remove}) {
+	    my $bklist = get_backup_file_list($opts->{dumpdir}, $bkname);
+	    die "only $maxfiles backup(s) allowed - please consider to remove old backup files.\n" 
+		if scalar(@$bklist) >= $maxfiles;
+	}
+
 	my $logfile = $task->{logfile} = "$opts->{dumpdir}/$basename.log";
 
 	my $ext = '.tar';
@@ -877,26 +907,12 @@ sub exec_backup_task {
 
 	# purge older backup
 
-	my $maxfiles = $opts->{maxfiles};
-
-	if ($maxfiles) {
-	    my @bklist = ();
-	    my $dir = $opts->{dumpdir};
-	    foreach my $fn (<$dir/${bkname}-*>) {
-		next if $fn eq $task->{tarfile};
-		if ($fn =~ m!/(${bkname}-(\d{4})_(\d{2})_(\d{2})-(\d{2})_(\d{2})_(\d{2})\.(tgz|(tar(\.(gz|lzo))?)))$!) {
-		    $fn = "$dir/$1"; # untaint
-		    my $t = timelocal ($7, $6, $5, $4, $3 - 1, $2 - 1900);
-		    push @bklist, [$fn, $t];
-		}
-	    }
-	
-	    @bklist = sort { $b->[1] <=> $a->[1] } @bklist;
+	if ($maxfiles && $opts->{remove}) {
+	    my $bklist = get_backup_file_list($opts->{dumpdir}, $bkname, $task->{tarfile});
+	    $bklist = [ sort { $b->[1] <=> $a->[1] } @$bklist ];
 
-	    my $ind = scalar (@bklist);
-
-	    while (scalar (@bklist) >= $maxfiles) {
-		my $d = pop @bklist;
+	    while (scalar (@$bklist) >= $maxfiles) {
+		my $d = pop @$bklist;
 		debugmsg ('info', "delete old backup '$d->[0]'", $logfd);
 		unlink $d->[0];
 		my $logfn = $d->[0];
@@ -1161,6 +1177,12 @@ my $confdesc = {
 	optional => 1,
 	minimum => 1,
     },
+    remove => {
+	type => 'boolean',
+	description => "Remove old backup files if there are more than 'maxfiles' backup files.",
+	optional => 1,
+	default => 1,
+    },
 };
 
 sub option_exists {

debian/changelog.Debian

+pve-manager (2.0-44) unstable; urgency=low
+
+  * Disable automatic file removal on 'Backup now'
+  
+  * Option 'maxfiles' is now a storage option (to specify the maximal
+    number of allowed backups per VM)
+
+ -- Proxmox Support Team <support@proxmox.com>  Fri, 23 Mar 2012 10:03:23 +0100
+
 pve-manager (2.0-43) unstable; urgency=low
 
   * vzdump: do not use tar flag --ignore-failed-read

defines.mk

@@ -2,7 +2,7 @@ RELEASE=2.0
 
 VERSION=2.0
 PACKAGE=pve-manager
-PACKAGERELEASE=43
+PACKAGERELEASE=44
 
 BINDIR=${DESTDIR}/usr/bin
 PERLLIBDIR=${DESTDIR}/usr/share/perl5

www/manager/dc/Backup.js

@@ -164,15 +164,6 @@ Ext.define('PVE.dc.BackupEdit', {
 		name: 'compress',
 		value: me.create ? 'lzo' : ''
 	    },
-	    {
-		xtype: 'numberfield',
-		fieldLabel: gettext('Max files'),
-		name: 'maxfiles',
-		minValue: 1,
-		maxValue: 365,
-		value: '1',
-		allowBlank: false
-	    },
 	    {
 		xtype: 'pveBackupModeSelector',
 		fieldLabel: gettext('Mode'),

www/manager/storage/DirEdit.js

@@ -61,6 +61,15 @@ Ext.define('PVE.storage.DirInputPanel', {
 		name: 'shared',
 		uncheckedValue: 0,
 		fieldLabel: gettext('Shared')
+	    },
+	    {
+		xtype: 'numberfield',
+		fieldLabel: gettext('Max Backups'),
+		name: 'maxfiles',
+		minValue: 0,
+		maxValue: 365,
+		value: me.create ? '1' : undefined,
+		allowBlank: false
 	    }
 	];
 

www/manager/storage/NFSEdit.js

@@ -141,6 +141,15 @@ Ext.define('PVE.storage.NFSInputPanel', {
 		checked: true,
 		uncheckedValue: 0,
 		fieldLabel: gettext('Enable')
+	    },
+	    {
+		xtype: 'numberfield',
+		fieldLabel: gettext('Max Backups'),
+		name: 'maxfiles',
+		minValue: 0,
+		maxValue: 365,
+		value: me.create ? '1' : undefined,
+		allowBlank: false
 	    }
 	];
 

www/manager/window/Backup.js

@@ -59,9 +59,10 @@ Ext.define('PVE.window.Backup', {
 		var storage = storagesel.getValue();
 		var values = form.getValues();
 		var params = {
-			storage: storage,
-			vmid: me.vmid,
-			mode: values.mode
+		    storage: storage,
+		    vmid: me.vmid,
+		    mode: values.mode,
+		    remove: 0
 		};
 		if (values.compress) {
 		    params.compress = values.compress;