[pve-devel] [PATCH manager] Fix #1015: vzdump: send email on early errors

Mon Jun 6 11:02:20 CEST 2016

comments inline:

> VZDump->new() dies when a tmpdir or dumpdir is configured
> but does not exist. At this point the error is not being
> reported via email.
>
> This also moves the instantiation of VZDump into the worker
> since new() can now call sendmail() on error.
>
> Additionally rather than only showing a single error if both
> tmpdir and dumpdir don't exist, both are included in the
> message.
> ---
>   PVE/API2/VZDump.pm |  4 ++--
>   PVE/VZDump.pm      | 26 ++++++++++++++++++++++----
>   2 files changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
> index 91b8a27..2e90078 100644
> --- a/PVE/API2/VZDump.pm
> +++ b/PVE/API2/VZDump.pm
> @@ -120,8 +120,6 @@ __PACKAGE__->register_method ({
>   	$rpcenv->check($user, "/storage/$param->{storage}", [ 'Datastore.AllocateSpace' ])
>   	    if $param->{storage};
>
> -	my $vzdump = PVE::VZDump->new($cmdline, $param, $skiplist);
> -
>   	my $worker = sub {
>   	    my $upid = shift;
>
> @@ -129,6 +127,8 @@ __PACKAGE__->register_method ({
>   		die "interrupted by signal\n";
>   	    };
>
> +	    my $vzdump = PVE::VZDump->new($cmdline, $param, $skiplist);
> +
>   	    eval {
>   		$vzdump->getlock($upid); # only one process allowed
>   	    };
> diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
> index 3101b6a..5ee6f0b 100644
> --- a/PVE/VZDump.pm
> +++ b/PVE/VZDump.pm
> @@ -391,13 +391,21 @@ sub sendmail {
>       return if (!$ecount && !$err && ($notify eq 'failure'));
>
>       my $stat = ($ecount || $err) ? 'backup failed' : 'backup successful';
> -    $stat .= ": $err" if $err;
> +    if ($err) {
> +	if ($err =~ /\n/) {

should it not be /\n/m for multiline matches?

> +	    $stat .= ": multiple probles";

s/probles/problems/

> +	} else {
> +	    $stat .= ": $err";
> +	    $err = undef;
> +	}
> +    }
>
>       my $hostname = `hostname -f` || PVE::INotify::nodename();
>       chomp $hostname;
>
>       # text part
> -    my $text = sprintf ("%-10s %-6s %10s %10s  %s\n", qw(VMID STATUS TIME SIZE FILENAME));
> +    my $text = $err ? "$err\n\n" : '';
> +    $text .= sprintf ("%-10s %-6s %10s %10s  %s\n", qw(VMID STATUS TIME SIZE FILENAME));
>       foreach my $task (@$tasklist) {
>   	my $vmid = $task->{vmid};
>   	if  ($task->{state} eq 'ok') {
> @@ -433,6 +441,7 @@ sub sendmail {
>
>       # html part
>       my $html = "<html><body>\n";
> +    $html .= "<p>" . (escape_html($err) =~ s/\n/<br>/gr) . "</p>\n" if $err;
>       $html .= "<table border=1 cellpadding=3>\n";
>       $html .= "<tr><td>VMID<td>NAME<td>STATUS<td>TIME<td>SIZE<td>FILENAME</tr>\n";
>
> @@ -566,19 +575,28 @@ sub new {
>   	$opts->{storage} = 'local';
>       }
>
> +    my $errors = '';
> +
>       if ($opts->{storage}) {
>   	my $info = storage_info ($opts->{storage});
>   	$opts->{dumpdir} = $info->{dumpdir};
>   	$maxfiles = $info->{maxfiles} if !defined($maxfiles) && defined($info->{maxfiles});
>       } elsif ($opts->{dumpdir}) {
> -	die "dumpdir '$opts->{dumpdir}' does not exist\n"
> +	$errors .= "dumpdir '$opts->{dumpdir}' does not exist"
>   	    if ! -d $opts->{dumpdir};
>       } else {
>   	die "internal error";
>       }
>
>       if ($opts->{tmpdir} && ! -d $opts->{tmpdir}) {
> -	die "tmpdir '$opts->{tmpdir}' does not exist\n";
> +	$errors .= "\n" if $errors;
> +	$errors .= "tmpdir '$opts->{tmpdir}' does not exist";
> +    }
> +
> +    if ($errors) {
> +	eval { $self->sendmail([], 0, $errors); };
> +	debugmsg ('err', $@) if $@;
> +	die "$errors\n";
>       }
>
>       $opts->{maxfiles} = $maxfiles if defined($maxfiles);
>