[pve-devel] [PATCH/RFC guest-common 5/6] job_status: simplify fixup of jobs for stolen guests

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

by re-using switch_replication_job_target, or in fact
the new no_lock variant operating on the config itself.

If a job is scheduled for removal and the guest was
stolen, it still makes sense to correct the job entry,
which didn't happen previously.

AFAICT, this was the only user of swap_source_target_nolock

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/ReplicationConfig.pm | 40 ++++++++++++++++------------------------
 PVE/ReplicationState.pm  | 32 +++++++++++++-------------------
 2 files changed, 29 insertions(+), 43 deletions(-)

diff --git a/PVE/ReplicationConfig.pm b/PVE/ReplicationConfig.pm
index fc3e792..815c9c0 100644
--- a/PVE/ReplicationConfig.pm
+++ b/PVE/ReplicationConfig.pm
@@ -230,23 +230,28 @@ sub find_local_replication_job {
 
 # makes old_target the new source for all local jobs of this guest
 # makes new_target the target for the single local job with target old_target
+sub switch_replication_job_target_nolock {
+    my ($cfg, $vmid, $old_target, $new_target) = @_;
+
+    foreach my $id (keys %{$cfg->{ids}}) {
+	my $jobcfg = $cfg->{ids}->{$id};
+
+	next if $jobcfg->{guest} ne $vmid;
+	next if $jobcfg->{type} ne 'local';
+
+	$jobcfg->{target} = $new_target if $jobcfg->{target} eq $old_target;
+	$jobcfg->{source} = $old_target;
+    }
+    $cfg->write();
+}
+
 sub switch_replication_job_target {
     my ($vmid, $old_target, $new_target) = @_;
 
     my $update_jobs = sub {
 	my $cfg = PVE::ReplicationConfig->new();
-	foreach my $id (keys %{$cfg->{ids}}) {
-	    my $jobcfg = $cfg->{ids}->{$id};
-
-	    next if $jobcfg->{guest} ne $vmid;
-	    next if $jobcfg->{type} ne 'local';
-
-	    $jobcfg->{target} = $new_target if $jobcfg->{target} eq $old_target;
-	    $jobcfg->{source} = $old_target;
-	}
-	$cfg->write();
+	$cfg->switch_replication_job_target_nolock($vmid, $old_target, $new_target);
     };
-
     lock($update_jobs);
 }
 
@@ -276,19 +281,6 @@ sub remove_vmid_jobs {
     lock($code);
 }
 
-sub swap_source_target_nolock {
-    my ($jobid) = @_;
-
-    my $cfg = __PACKAGE__->new();
-    my $job = $cfg->{ids}->{$jobid};
-    my $tmp = $job->{source};
-    $job->{source} = $job->{target};
-    $job->{target} = $tmp;
-    $cfg->write();
-
-    return $cfg->{ids}->{$jobid};
-}
-
 package PVE::ReplicationConfig::Cluster;
 
 use base qw(PVE::ReplicationConfig);
diff --git a/PVE/ReplicationState.pm b/PVE/ReplicationState.pm
index 0c92778..e486bc7 100644
--- a/PVE/ReplicationState.pm
+++ b/PVE/ReplicationState.pm
@@ -251,22 +251,21 @@ sub job_status {
 	    # only consider guest on local node
 	    next if $vms->{ids}->{$vmid}->{node} ne $local_node;
 
+	    # source is optional in schema, but we set it automatically
+	    if (!defined($jobcfg->{source})) {
+		$jobcfg->{source} = $local_node;
+		$cfg->write();
+	    }
+
+	    # fix jobs for stolen guest
+	    $cfg->switch_replication_job_target_nolock($vmid, $local_node, $jobcfg->{source})
+		if $local_node ne $jobcfg->{source};
+
 	    my $target = $jobcfg->{target};
-	    if (!$jobcfg->{remove_job}) {
-		# check if vm was stolen (swapped source target)
-		if ($target eq $local_node) {
-		    my $source = $jobcfg->{source};
-		    if (defined($source) && $source ne $target) {
-			$jobcfg = PVE::ReplicationConfig::swap_source_target_nolock($jobid);
-			$cfg->{ids}->{$jobid} = $jobcfg;
-		    } else {
-			# never sync to local node
-			next;
-		    }
-		}
+	    # never sync to local node
+	    next if !$jobcfg->{remove_job} && $target eq $local_node;
 
-		next if !$get_disabled && $jobcfg->{disable};
-	    }
+	    next if !$get_disabled && $jobcfg->{disable};
 
 	    my $state = extract_job_state($stateobj, $jobcfg);
 	    $jobcfg->{state} = $state;
@@ -293,11 +292,6 @@ sub job_status {
 
 	    $jobcfg->{next_sync} = $next_sync;
 
-	    if (!defined($jobcfg->{source}) || $jobcfg->{source} ne $local_node) {
-		$jobcfg->{source} = $cfg->{ids}->{$jobid}->{source} = $local_node;
-		PVE::ReplicationConfig::write($cfg);
-	    }
-
 	    $jobs->{$jobid} = $jobcfg;
 	}
     };
-- 
2.20.1