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

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

by using switch_replication_job_target_nolock.

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

Changes from v1:
    * split into two parts (see previous patch)

 PVE/ReplicationConfig.pm | 13 -------------
 PVE/ReplicationState.pm  | 32 +++++++++++++-------------------
 2 files changed, 13 insertions(+), 32 deletions(-)

diff --git a/PVE/ReplicationConfig.pm b/PVE/ReplicationConfig.pm
index 6ba87ae..fd856a0 100644
--- a/PVE/ReplicationConfig.pm
+++ b/PVE/ReplicationConfig.pm
@@ -279,19 +279,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 80d45d6..81a1b31 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