[pve-devel] [PATCH V4 guset-common] Remove noerr form replication.

[Thomas Lamprecht]

On 12/07/2017 12:06 PM, Wolfgang Link wrote:
> We will handle this errors in the API and decide what to do.

Most stuff is indentation change, so mail look more noisy than it is.

In general OK, *but* we need a to handle package versions of this, i.e.:
The package version of guest-common which adds this needs to break earlier
managers and the version of pve-manager which adds the manager patches needs
to add a versioned dependency on guest-common in the respective control file.
It's just the saner and user-friendlier way to do things.

Reviewed-by: Thomas Lamprecht <t.lamprecht at proxmox.com>

> ---
>  PVE/Replication.pm | 95 +++++++++++++++++++++++-------------------------------
>  1 file changed, 41 insertions(+), 54 deletions(-)
> 
> diff --git a/PVE/Replication.pm b/PVE/Replication.pm
> index c25ed44..9bc4e61 100644
> --- a/PVE/Replication.pm
> +++ b/PVE/Replication.pm
> @@ -304,7 +304,7 @@ sub replicate {
>  }
>  
>  my $run_replication_nolock = sub {
> -    my ($guest_class, $jobcfg, $iteration, $start_time, $logfunc, $noerr, $verbose) = @_;
> +    my ($guest_class, $jobcfg, $iteration, $start_time, $logfunc, $verbose) = @_;
>  
>      my $jobid = $jobcfg->{id};
>  
> @@ -313,79 +313,66 @@ my $run_replication_nolock = sub {
>      # we normaly write errors into the state file,
>      # but we also catch unexpected errors and log them to syslog
>      # (for examply when there are problems writing the state file)
> -    eval {
> -	my $state = PVE::ReplicationState::read_job_state($jobcfg);
>  
> -	PVE::ReplicationState::record_job_start($jobcfg, $state, $start_time, $iteration);
> +    my $state = PVE::ReplicationState::read_job_state($jobcfg);
> +
> +    PVE::ReplicationState::record_job_start($jobcfg, $state, $start_time, $iteration);
>  
> -	my $t0 = [gettimeofday];
> +    my $t0 = [gettimeofday];
>  
> -	mkdir $PVE::ReplicationState::replicate_logdir;
> -	my $logfile = PVE::ReplicationState::job_logfile_name($jobid);
> -	open(my $logfd, '>', $logfile) ||
> -	    die "unable to open replication log '$logfile' - $!\n";
> +    mkdir $PVE::ReplicationState::replicate_logdir;
> +    my $logfile = PVE::ReplicationState::job_logfile_name($jobid);
> +    open(my $logfd, '>', $logfile) ||
> +	die "unable to open replication log '$logfile' - $!\n";
>  
> -	my $logfunc_wrapper = sub {
> -	    my ($msg) = @_;
> +    my $logfunc_wrapper = sub {
> +	my ($msg) = @_;
>  
> -	    my $ctime = get_log_time();
> -	    print $logfd "$ctime $jobid: $msg\n";
> -	    if ($logfunc) {
> -		if ($verbose) {
> -		    $logfunc->("$ctime $jobid: $msg");
> -		} else {
> -		    $logfunc->($msg);
> -		}
> +	my $ctime = get_log_time();
> +	print $logfd "$ctime $jobid: $msg\n";
> +	if ($logfunc) {
> +	    if ($verbose) {
> +		$logfunc->("$ctime $jobid: $msg");
> +	    } else {
> +		$logfunc->($msg);
>  	    }
> -	};
> +	}
> +    };
>  
> -	$logfunc_wrapper->("start replication job");
> +    $logfunc_wrapper->("start replication job");
>  
> -	eval {
> -	    $volumes = replicate($guest_class, $jobcfg, $state, $start_time, $logfunc_wrapper);
> -	};
> -	my $err = $@;
> +    eval {
> +	$volumes = replicate($guest_class, $jobcfg, $state, $start_time, $logfunc_wrapper);
> +    };
> +    my $err = $@;
>  
> -	if ($err) {
> -	    my $msg = "end replication job with error: $err";
> -	    chomp $msg;
> -	    $logfunc_wrapper->($msg);
> -	} else {
> -	    $logfunc_wrapper->("end replication job");
> -	}
> +    if ($err) {
> +	my $msg = "end replication job with error: $err";
> +	chomp $msg;
> +	$logfunc_wrapper->($msg);
> +    } else {
> +	$logfunc_wrapper->("end replication job");
> +    }
>  
> -	PVE::ReplicationState::record_job_end($jobcfg, $state, $start_time, tv_interval($t0), $err);
> +    PVE::ReplicationState::record_job_end($jobcfg, $state, $start_time, tv_interval($t0), $err);
>  
> -	close($logfd);
> +    close($logfd);
>  
> -	die $err if $err && !$noerr;
> -    };
> -    if (my $err = $@) {
> -	if ($noerr) {
> -	    warn "$jobid: got unexpected replication job error - $err";
> -	} else {
> -	    die $err;
> -	}
> -    }
> +    die $err if $err;
>  
>      return $volumes;
>  };
>  
>  sub run_replication {
> -    my ($guest_class, $jobcfg, $iteration, $start_time, $logfunc, $noerr, $verbose) = @_;
> +    my ($guest_class, $jobcfg, $iteration, $start_time, $logfunc, $verbose) = @_;
>  
>      my $volumes;
>  
> -    eval {
> -	my $timeout = 2; # do not wait too long - we repeat periodically anyways
> -	$volumes = PVE::GuestHelpers::guest_migration_lock(
> -	    $jobcfg->{guest}, $timeout, $run_replication_nolock,
> -	    $guest_class, $jobcfg, $iteration, $start_time, $logfunc, $noerr, $verbose);
> -    };
> -    if (my $err = $@) {
> -	return undef if $noerr;
> -	die $err;
> -    }
> +    my $timeout = 2; # do not wait too long - we repeat periodically anyways
> +    $volumes = PVE::GuestHelpers::guest_migration_lock(
> +	$jobcfg->{guest}, $timeout, $run_replication_nolock,
> +	$guest_class, $jobcfg, $iteration, $start_time, $logfunc, $verbose);
> +
>      return $volumes;
>  }
>  
>