[pve-devel] [PATCH manager v6 37/37] backup: implement backup for external providers

[Fiona Ebner <f.ebner@proxmox.com>]

Call job_{init,cleanup}() and backup_{init,cleanup}() methods so that
backup providers can prepare and clean up for the whole backup job and
for individual guest backups.

It is necessary to adapt some log messages and special case some
things like is already done for PBS, e.g. log file handling.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Changes in v6:
* Replace job/backup hooks with dedicated methods.
* Remove backup_get_archive_name() method. The archive name is now
  returned as part of the backup_init() method. That was also a bit of
  a misnomer, since it was necessary for the backup provider to
  remember that archive name for later.
* Supersede backup_get_task_size() method and return stats as part of
  the backup_cleanup() method (in the success case). Currently, this
  only includes the archive size as well, but could be extended later
  to allow custom stats to include in the task notification summary.

 PVE/VZDump.pm           | 60 ++++++++++++++++++++++++++++++++++++-----
 test/vzdump_new_test.pl |  3 +++
 2 files changed, 56 insertions(+), 7 deletions(-)

diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index fd89945e..82914e56 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -217,7 +217,10 @@ sub storage_info {
     $info->{'prune-backups'} = PVE::JSONSchema::parse_property_string('prune-backups', $scfg->{'prune-backups'})
 	if defined($scfg->{'prune-backups'});
 
-    if ($type eq 'pbs') {
+    if (PVE::Storage::storage_has_feature($cfg, $storage, 'backup-provider')) {
+	$info->{'backup-provider'} =
+	    PVE::Storage::new_backup_provider($cfg, $storage, sub { debugmsg($_[0], $_[1]); });
+    } elsif ($type eq 'pbs') {
 	$info->{pbs} = 1;
     } else {
 	$info->{dumpdir} = PVE::Storage::get_backup_dir($cfg, $storage);
@@ -717,6 +720,7 @@ sub new {
 	    $opts->{scfg} = $info->{scfg};
 	    $opts->{pbs} = $info->{pbs};
 	    $opts->{'prune-backups'} //= $info->{'prune-backups'};
+	    $self->{'backup-provider'} = $info->{'backup-provider'} if $info->{'backup-provider'};
 	}
     } elsif ($opts->{dumpdir}) {
 	$add_error->("dumpdir '$opts->{dumpdir}' does not exist")
@@ -1001,7 +1005,7 @@ sub exec_backup_task {
 	    }
 	}
 
-	if (!$self->{opts}->{pbs}) {
+	if (!$self->{opts}->{pbs} && !$self->{'backup-provider'}) {
 	    $task->{logfile} = "$opts->{dumpdir}/$basename.log";
 	}
 
@@ -1011,7 +1015,10 @@ sub exec_backup_task {
 	    $ext .= ".${comp_ext}";
 	}
 
-	if ($self->{opts}->{pbs}) {
+	if ($self->{'backup-provider'}) {
+	    die "unable to pipe backup to stdout\n" if $opts->{stdout};
+	    # the archive name $task->{target} is returned by the start hook a bit later
+	} elsif ($self->{opts}->{pbs}) {
 	    die "unable to pipe backup to stdout\n" if $opts->{stdout};
 	    $task->{target} = $pbs_snapshot_name;
 	} else {
@@ -1029,7 +1036,7 @@ sub exec_backup_task {
 	my $pid = $$;
 	if ($opts->{tmpdir}) {
 	    $task->{tmpdir} = "$opts->{tmpdir}/vzdumptmp${pid}_$vmid/";
-	} elsif ($self->{opts}->{pbs}) {
+	} elsif ($self->{opts}->{pbs} || $self->{'backup-provider'}) {
 	    $task->{tmpdir} = "/var/tmp/vzdumptmp${pid}_$vmid";
 	} else {
 	    # dumpdir is posix? then use it as temporary dir
@@ -1101,6 +1108,11 @@ sub exec_backup_task {
 	if ($mode eq 'stop') {
 	    $plugin->prepare ($task, $vmid, $mode);
 
+	    if ($self->{'backup-provider'}) {
+		my $init_result =
+		    $self->{'backup-provider'}->backup_init($vmid, $vmtype, $task->{backup_time});
+		$task->{target} = $init_result->{'archive-name'};
+	    }
 	    $self->run_hook_script ('backup-start', $task, $logfd);
 
 	    if ($running) {
@@ -1115,6 +1127,11 @@ sub exec_backup_task {
 	} elsif ($mode eq 'suspend') {
 	    $plugin->prepare ($task, $vmid, $mode);
 
+	    if ($self->{'backup-provider'}) {
+		my $init_result =
+		    $self->{'backup-provider'}->backup_init($vmid, $vmtype, $task->{backup_time});
+		$task->{target} = $init_result->{'archive-name'};
+	    }
 	    $self->run_hook_script ('backup-start', $task, $logfd);
 
 	    if ($vmtype eq 'lxc') {
@@ -1141,6 +1158,11 @@ sub exec_backup_task {
 	    }
 
 	} elsif ($mode eq 'snapshot') {
+	    if ($self->{'backup-provider'}) {
+		my $init_result =
+		    $self->{'backup-provider'}->backup_init($vmid, $vmtype, $task->{backup_time});
+		$task->{target} = $init_result->{'archive-name'};
+	    }
 	    $self->run_hook_script ('backup-start', $task, $logfd);
 
 	    my $snapshot_count = $task->{snapshot_count} || 0;
@@ -1183,11 +1205,13 @@ sub exec_backup_task {
 	    return;
 	}
 
-	my $archive_txt = $self->{opts}->{pbs} ? 'Proxmox Backup Server' : 'vzdump';
+	my $archive_txt = 'vzdump';
+	$archive_txt = 'Proxmox Backup Server' if $self->{opts}->{pbs};
+	$archive_txt = $self->{'backup-provider'}->provider_name() if $self->{'backup-provider'};
 	debugmsg('info', "creating $archive_txt archive '$task->{target}'", $logfd);
 	$plugin->archive($task, $vmid, $task->{tmptar}, $comp);
 
-	if ($self->{opts}->{pbs}) {
+	if ($self->{'backup-provider'} || $self->{opts}->{pbs}) {
 	    # size is added to task struct in guest vzdump plugins
 	} else {
 	    rename ($task->{tmptar}, $task->{target}) ||
@@ -1201,7 +1225,8 @@ sub exec_backup_task {
 
 	# Mark as protected before pruning.
 	if (my $storeid = $opts->{storage}) {
-	    my $volname = $opts->{pbs} ? $task->{target} : basename($task->{target});
+	    my $volname = $opts->{pbs} || $self->{'backup-provider'} ? $task->{target}
+	                                                             : basename($task->{target});
 	    my $volid = "${storeid}:backup/${volname}";
 
 	    if ($opts->{'notes-template'} && $opts->{'notes-template'} ne '') {
@@ -1254,6 +1279,10 @@ sub exec_backup_task {
 	    debugmsg ('info', "pruned $pruned backup(s)${log_pruned_extra}", $logfd);
 	}
 
+	if ($self->{'backup-provider'}) {
+	    my $cleanup_result = $self->{'backup-provider'}->backup_cleanup($vmid, $vmtype, 1, {});
+	    $task->{size} = $cleanup_result->{stats}->{'archive-size'};
+	}
 	$self->run_hook_script ('backup-end', $task, $logfd);
     };
     my $err = $@;
@@ -1313,6 +1342,14 @@ sub exec_backup_task {
 	debugmsg ('err', "Backup of VM $vmid failed - $err", $logfd, 1);
 	debugmsg ('info', "Failed at " . strftime("%F %H:%M:%S", localtime()));
 
+	if ($self->{'backup-provider'}) {
+	    eval {
+		$self->{'backup-provider'}->backup_cleanup(
+		    $vmid, $task->{vmtype}, 0, { error => $err });
+	    };
+	    debugmsg('warn', "backup cleanup for external provider failed - $@") if $@;
+	}
+
 	eval { $self->run_hook_script ('backup-abort', $task, $logfd); };
 	debugmsg('warn', $@) if $@; # message already contains command with phase name
 
@@ -1340,6 +1377,8 @@ sub exec_backup_task {
 		};
 		debugmsg('warn', "$@") if $@; # $@ contains already error prefix
 	    }
+	} elsif ($self->{'backup-provider'}) {
+	    $self->{'backup-provider'}->backup_handle_log_file($vmid, $task->{tmplog});
 	} elsif ($task->{logfile}) {
 	    system {'cp'} 'cp', $task->{tmplog}, $task->{logfile};
 	}
@@ -1398,6 +1437,7 @@ sub exec_backup {
     my $errcount = 0;
     eval {
 
+	$self->{'backup-provider'}->job_init($starttime) if $self->{'backup-provider'};
 	$self->run_hook_script ('job-start', undef, $job_start_fd);
 
 	foreach my $task (@$tasklist) {
@@ -1405,11 +1445,17 @@ sub exec_backup {
 	    $errcount += 1 if $task->{state} ne 'ok';
 	}
 
+	$self->{'backup-provider'}->job_cleanup() if $self->{'backup-provider'};
 	$self->run_hook_script ('job-end', undef, $job_end_fd);
     };
     my $err = $@;
 
     if ($err) {
+	if ($self->{'backup-provider'}) {
+	    eval { $self->{'backup-provider'}->job_cleanup(); };
+	    $err .= "job cleanup for external provider failed - $@" if $@;
+	}
+
 	eval { $self->run_hook_script ('job-abort', undef, $job_end_fd); };
 	$err .= $@ if $@;
 	debugmsg ('err', "Backup job failed - $err", undef, 1);
diff --git a/test/vzdump_new_test.pl b/test/vzdump_new_test.pl
index 8cd73075..01f2a661 100755
--- a/test/vzdump_new_test.pl
+++ b/test/vzdump_new_test.pl
@@ -51,6 +51,9 @@ $pve_storage_module->mock(
     activate_storage => sub {
 	return;
     },
+    get_backup_provider => sub {
+	return;
+    },
 );
 
 my $pve_cluster_module = Test::MockModule->new('PVE::Cluster');
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel