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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2012-07-05 at 11:40 +0100, Ian Jackson wrote:
> 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"

you mean $_lockfile here I think.

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

Won't this need to become $(PERL) (or @PERL@ and some seddery at install
time) for the benefit of BSD?

Does stating $_lockfile before and after the flock, in addition to
comparing both with $_lockfd, close the race you are worried about?

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

I suppose there is no point in unlocking too?

>  }
> 
> Ian.



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