[pve-devel] [PATCH cluster] cfs_lock: fix error handling for lock aquistion

[Thomas Lamprecht]

We checked if a specific error was set or, respectively, not set to
know if we got the lock or not.
The check if we may unlock again was negated and thus could lead to
problems, in specific - rather unlikely - cases.

Add a $got_lock variable which only gets set when we really got the
lock, this is easier to track for humans reading the code and should
cope better against code changes, e.g., it does not breaks just if an
error message typo got corrected a few lines above.

While refactoring for the new variable, set the $noerr parameter of
check_cfs_quorum() as we do not want to die here - that would
changing the error useless.
---

 data/PVE/Cluster.pm | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/data/PVE/Cluster.pm b/data/PVE/Cluster.pm
index 70ce250..6f1bda1 100644
--- a/data/PVE/Cluster.pm
+++ b/data/PVE/Cluster.pm
@@ -861,6 +861,7 @@ my $cfs_lock = sub {
     my ($lockid, $timeout, $code, @param) = @_;
 
     my $res;
+    my $got_lock = 0;
 
     # this timeout is for aquire the lock
     $timeout = 10 if !$timeout;
@@ -887,6 +888,7 @@ my $cfs_lock = sub {
 		if (!(mkdir $filename)) {
 		    (utime 0, 0, $filename); # cfs unlock request
 		} else {
+		    $got_lock = 1;
 		    print STDERR " OK\n";
 		    last;
 		}
@@ -910,14 +912,9 @@ my $cfs_lock = sub {
 
     alarm(0);
 
-    if ($err && ($err eq "got lock request timeout\n") &&
-	!check_cfs_quorum()){
-	$err = "$msg: no quorum!\n";
-    }
+    $err = "$msg: no quorum!\n" if !$got_lock && !check_cfs_quorum(1);
 
-    if (!$err || $err !~ /^got lock timeout -/) {
-	rmdir $filename; # cfs unlock
-    }
+    rmdir $filename if $got_lock; # if we held the lock always unlock again
 
     if ($err) {
         $@ = $err;
-- 
2.11.0