[pve-devel] [PATCH v4 pve-zsync 2/4] Improve read-modify-write enclosures

[Fabian Ebner]

Previously inside sync we just called update_job directly, now
we make sure to read the latest verison of the job first.

Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
---

$job is still used outside of such enclosures in sync_path
but it is only passed along as a variable and we don't
want to hold the cron and state lock for the full duration
of syncing. Anything actually modifying the job after
reading it should now be enclosed with a lock.

 pve-zsync | 65 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 38 insertions(+), 27 deletions(-)

diff --git a/pve-zsync b/pve-zsync
index 9a6625d..cfd8c08 100755
--- a/pve-zsync
+++ b/pve-zsync
@@ -589,16 +589,30 @@ sub sync {
 
 	my $date = get_date();
 	my $job;
-	eval {
-	    $job = get_job($param);
-	};
 
-	if ($job && defined($job->{state}) && $job->{state} eq "syncing") {
-	    die "Job --source $param->{source} --name $param->{name} is syncing at the moment";
-	}
+	my $dest;
+	my $source;
+	my $vm_type;
 
-	my $dest = parse_target($param->{dest});
-	my $source = parse_target($param->{source});
+	locked("$CONFIG_PATH/cron_and_state.lock", sub {
+	    eval { $job = get_job($param); };
+
+	    if ($job && defined($job->{state}) && $job->{state} eq "syncing") {
+		die "Job --source $param->{source} --name $param->{name} is syncing at the moment";
+	    }
+
+	    $dest = parse_target($param->{dest});
+	    $source = parse_target($param->{source});
+
+	    $vm_type = vm_exists($source, $param->{source_user});
+	    $source->{vm_type} = $vm_type;
+
+	    if ($job) {
+		$job->{state} = "syncing";
+		$job->{vm_type} = $vm_type if !$job->{vm_type};
+		update_state($job);
+	    }
+	}); #cron and state lock
 
 	my $sync_path = sub {
 	    my ($source, $dest, $job, $param, $date) = @_;
@@ -613,15 +627,6 @@ sub sync {
 
 	};
 
-	my $vm_type = vm_exists($source, $param->{source_user});
-	$source->{vm_type} = $vm_type;
-
-	if ($job) {
-	    $job->{state} = "syncing";
-	    $job->{vm_type} = $vm_type if !$job->{vm_type};
-	    locked("$CONFIG_PATH/cron_and_state.lock", sub { update_state($job); });
-	}
-
 	eval{
 	    if ($source->{vmid}) {
 		die "VM $source->{vmid} doesn't exist\n" if !$vm_type;
@@ -645,19 +650,25 @@ sub sync {
 	    }
 	};
 	if (my $err = $@) {
-	    if ($job) {
-		$job->{state} = "error";
-		locked("$CONFIG_PATH/cron_and_state.lock", sub { update_state($job); });
-		print "Job --source $param->{source} --name $param->{name} got an ERROR!!!\nERROR Message:\n";
-	    }
+	    locked("$CONFIG_PATH/cron_and_state.lock", sub {
+		eval { $job = get_job($param); };
+		if ($job) {
+		    $job->{state} = "error";
+		    update_state($job);
+		}
+	    });
+	    print "Job --source $param->{source} --name $param->{name} got an ERROR!!!\nERROR Message:\n";
 	    die "$err\n";
 	}
 
-	if ($job) {
-	    $job->{state} = "ok";
-	    $job->{lsync} = $date;
-	    locked("$CONFIG_PATH/cron_and_state.lock", sub { update_state($job); });
-	}
+	locked("$CONFIG_PATH/cron_and_state.lock", sub {
+	    eval { $job = get_job($param); };
+	    if ($job) {
+		$job->{state} = "ok";
+		$job->{lsync} = $date;
+		update_state($job);
+	    }
+	});
     }); #sync lock
 }
 
-- 
2.20.1