[pve-devel] [PATCH cluster 1/2] refactor backup_cfs_database

[Thomas Lamprecht]

The logic for finding out which backups to delete was a bit weird

Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
---
 data/PVE/Cluster.pm | 37 +++++++++++++++----------------------
 1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/data/PVE/Cluster.pm b/data/PVE/Cluster.pm
index 2920ff6..27b1c97 100644
--- a/data/PVE/Cluster.pm
+++ b/data/PVE/Cluster.pm
@@ -1743,36 +1743,29 @@ sub assert_joinable {
     }
 }
 
+# NOTE: filesystem must be offline here, no DB changes allowed
 my $backup_cfs_database = sub {
     my ($dbfile) = @_;
 
     mkdir $dbbackupdir;
 
-    print "backup old database\n";
     my $ctime = time();
-    my $cmd = [
-	['echo', '.dump'],
-	['sqlite3', $dbfile],
-	['gzip', '-', \ ">${dbbackupdir}/config-${ctime}.sql.gz"],
-    ];
-
-    PVE::Tools::run_command($cmd, 'errmsg' => "cannot backup old database\n");
-
-    # purge older backup
-    my $maxfiles = 10;
-    my @bklist = ();
-    foreach my $fn (<$dbbackupdir/config-*.sql.gz>) {
-	if ($fn =~ m!/config-(\d+)\.sql.gz$!) {
-	    push @bklist, [$fn, $1];
+    my $backup_fn = "$dbbackupdir/config-$ctime.sql.gz";
+
+    print "backup old database to '$backup_fn'\n";
+
+    my $cmd = [ ['sqlite3', $dbfile, '.dump'], ['gzip', '-', \ ">${backup_fn}"] ];
+    run_command($cmd, 'errmsg' => "cannot backup old database\n");
+
+    my $maxfiles = 10; # purge older backup
+    my $backups = [ sort { $b cmp $a } <$dbbackupdir/config-*.sql.gz> ];
+
+    if ((my $count = scalar(@$backups)) > $maxfiles) {
+	foreach my $f (@$backups[$maxfiles..$count-1]) {
+	    print "delete old backup '$1'\n";
+	    unlink $1;
 	}
     }
-
-    @bklist = sort { $b->[1] <=> $a->[1] } @bklist;
-    while (scalar (@bklist) >= $maxfiles) {
-	my $d = pop @bklist;
-	print "delete old backup '$d->[0]'\n";
-	unlink $d->[0];
-    }
 };
 
 sub join {
-- 
2.14.2