Re: [Xen-devel] [xen-unstable test] 13461: regressions - FAIL

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Daniel P. Berrange writes ("Re: [Xen-devel] [xen-unstable test] 13461: 
regressions - FAIL"):
> Yes, as you say flock() operates on the inode, so if something deletes
> and recreates the file, future flocks will operate differently. Ideally
> you should just never rm the files at all.
> 
> If you need to 'rm' them, then to avoid this, you must do two things
> 
>  - Only 'rm /foo' while holding the lock on /foo
>  - Record the inode before acquiring the lock. After acquiring the
>    lock check whether the inode on disk is the same. If not,
>    release the lock & repeat.

It seems more logical to me to check the inum of the open fd against
the file.  Something like this perhaps (untested):

diff -r ad08cd8e7097 tools/hotplug/Linux/locking.sh
--- a/tools/hotplug/Linux/locking.sh    Thu Jul 05 11:00:28 2012 +0100
+++ b/tools/hotplug/Linux/locking.sh    Thu Jul 05 11:39:59 2012 +0100
@@ -30,6 +30,7 @@ _setlockfd()
     done
     _lockdict[$i]="$1"
     let _lockfd=200+i
+    let _lockfile="$LOCK_BASEDIR/$1"
 }
 
 
@@ -37,13 +38,32 @@ claim_lock()
 {
     mkdir -p "$LOCK_BASEDIR"
     _setlockfd $1
-    eval "exec $_lockfd>>$LOCK_BASEDIR/$1"
-    flock -x $_lockfd
+    # The locking strategy is identical to that from with-lock-ex(1)
+    # from chiark-utils, except using flock.  It has the benefit of
+    # it being possible to safely remove the lockfile when done.
+    local rightfile
+    while true; do
+        eval "exec $_lockfd>>$lockfile"
+        flock -x $_lockfd
+        # We can't just stat /dev/stdin or /proc/self/fd/$_lockfd or
+        # use bash's test -ef because those all go through what is
+        # actually a synthetic symlink in /proc and we aren't
+        # guaranteed that our stat(2) won't lose the race with an
+        # rm(1) between reading the synthetic link and traversing the
+        # file system to find the inum.  Perl is very fast so use that.
+        rightfile=$( perl -e '
+            open STDIN, "<&'$_lockfd'" or die $!;
+            my $fd_inum = (stat STDIN)[1]; die $! unless defined $fd_inum;
+            my $file_inum = (stat $ARGV[0])[1];
+            print "y\n" if $fd_inum eq $file_inum;
+                             ' "$_lockfile" )
+        if [ x$rightfile = xy ]; then break; fi
+    done
 }
 
 
 release_lock()
 {
     _setlockfd $1
-    flock -u $_lockfd
+    rm "$_lockfile"
 }

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel