From mboxrd@z Thu Jan  1 00:00:00 1970
From: Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [xen-unstable test] 13461: regressions - FAIL
Date: Thu, 5 Jul 2012 12:00:55 +0100
Message-ID: <4FF573E7.8080005@citrix.com>
References: <osstest-13461-mainreport@xen.org>
	<1341467373.14447.9.camel@dagon.hellion.org.uk>
	<20120705092834.GE13884@redhat.com>
	<20469.28420.49684.132247@mariner.uk.xensource.com>
	<1341485867.16599.40.camel@zakaz.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1341485867.16599.40.camel@zakaz.uk.xensource.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Ian Campbell <Ian.Campbell@citrix.com>
Cc: "Daniel P. Berrange" <berrange@redhat.com>, Zhigang Wang <zhigang.x.wang@oracle.com>, Ian Jackson <Ian.Jackson@eu.citrix.com>, "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>
List-Id: xen-devel@lists.xenproject.org

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.