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

[Roger Pau Monne <roger.pau@citrix.com>]

Ian Campbell wrote:
> 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?

BSD don't use this scripts, so you don't have to worry about this here.