Re: [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 

> ---
>  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 

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

[Wolfgang Link]

We will handle this errors in the API and decide what to do.
---
 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;
 }
 
-- 
2.11.0


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