Re: [Xen-devel] xen 4.4.0 unnecessary dependency on Perl

[Anthony Wright <anthony@xxxxxxxxxxxxxxx>]

----- Original Message -----
> Ian Campbell writes ("Re: [Xen-devel] xen 4.4.0 unnecessary dependency
> on Perl"):
> > On Mon, 2014-07-28 at 21:25 +0100, Anthony Wright wrote:
> > > To me it would seem wise to minimise the number of xen's
> > > dependencies
> > > and so use Python instead or if speed is really important here, a
> > > custom
> > > C program. We have a custom Linux distro that includes xen and to
> > > us the
> > > size of the distro is very important. This single 8 line section
> > > of code
> > > introduces a dependency on Perl and so adds another 10M to 20M.
> >
> > Please send a patch.
> >
> > Now that xend is gone Python is reasonably optional as well I think
> > (it's needed for pygrub but not a lot else IIRC). In the absence of
> > any
> > other suitable facilities in a base system that might point to a
> > custom
> > C utility being the best approach, but you should probably wait for
> > other feedback (from Ian Jackson, CCd, in particular) before picking
> > an
> > approach.
> 
> The perl fragment there is comparing inode numbers. I think that can
> be done with stat(1). For the fd stdin, it can be done with `stat -L
> /dev/stdin'.
> 
> Ian.
Below is a patch that converts the perl script into shell script. I'm new to 
submitting patches, so I apologise if this is the wrong way to do it.

I believe the patch provides the same functionality as the perl, but it does 
exactly what the comment says isn't good enough. Having said that from reading 
around the rest of the locking file and examining the code, I'm inclined to 
ignore the comment. Perl isn't adding anything from a performance perspective 
as it's the stat() kernel call where all the work is done, and I can't see any 
race condition with rm.

If we open the file and get a valid $_lockfd (confirmed as flock succeeding), 
then while the $_lockfd is open it will always have a valid inode, and it's 
inode number will always be the same, thus the first stat will always succeed. 
With the second stat I can't envisage any race condition with rm. If $_lockfile 
has been removed, the stat() will fail. If it's been removed and re-created, 
the stat() will succeed, but return a different inode number. If it hasn't been 
touched then stat() will succeed and return the same inode number as $_lockfd. 
All these cases are handled, and there is no potential for a race.

Anthony

diff -ur xen-4.4.0.orig/tools/hotplug/Linux/locking.sh 
xen-4.4.0/tools/hotplug/Linux/locking.sh
--- xen-4.4.0.orig/tools/hotplug/Linux/locking.sh       2014-03-10 
10:43:57.000000000 +0000
+++ xen-4.4.0/tools/hotplug/Linux/locking.sh    2014-07-30 13:30:46.864655020 
+0100
@@ -46,19 +46,11 @@
     while true; do
         eval "exec $_lockfd<>$_lockfile"
         flock -x $_lockfd || return $?
-        # 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
+
+        _fd_inum=`stat -L -c '%i' /proc/self/fd/$_lockfd`
+        _file_inum=`sh -c "stat -c '%i' $_lockfile || true"`
+        if [ x$_fd_inum = x$_file_inum ]; then break; fi
+
        # Some versions of bash appear to be buggy if the same
        # $_lockfile is opened repeatedly. Close the current fd here.
         eval "exec $_lockfd<&-"



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