| Message ID | 20200312145417.106812-3-jandryuk@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | scripts: Use stat to check lock claim | expand |
On Thu, Mar 12, 2020 at 10:54:17AM -0400, Jason Andryuk wrote: > Replace the perl locking check with stat(1). Stat is able to fstat > stdin (file descriptor 0) when passed '-' as an argument. This is now > used to check $_lockfd. stat(1) support for '-' was introduced to > coreutils in 2009. > > After A releases its lock, script B will return from flock and execute > stat. Since the lockfile has been removed by A, stat prints an error to > stderr and exits non-zero. Redirect stderr to /dev/null to avoid > filling /var/log/xen/xen-hotplug.log with "No such file or directory" > messages. > > Placing the stat call inside the "if" condition ensures we only check > the stat output when the command completed successfully. > > This change removes the only runtime dependency of the xen toolstack on > perl. > > Suggested-by: Ian Jackson <ian.jackson@citrix.com> > Signed-off-by: Jason Andryuk <jandryuk@gmail.com> Ian, ping? I already applied patch 1. This patch looks to be good as well. Wei.
Wei Liu writes ("Re: [PATCH 2/2] scripts: Use stat to check lock claim"):
> On Thu, Mar 12, 2020 at 10:54:17AM -0400, Jason Andryuk wrote:
> > Replace the perl locking check with stat(1). Stat is able to fstat
> > stdin (file descriptor 0) when passed '-' as an argument. This is now
> > used to check $_lockfd. stat(1) support for '-' was introduced to
> > coreutils in 2009.
> >
> > After A releases its lock, script B will return from flock and execute
> > stat. Since the lockfile has been removed by A, stat prints an error to
> > stderr and exits non-zero. Redirect stderr to /dev/null to avoid
> > filling /var/log/xen/xen-hotplug.log with "No such file or directory"
> > messages.
> >
> > Placing the stat call inside the "if" condition ensures we only check
> > the stat output when the command completed successfully.
> >
> > This change removes the only runtime dependency of the xen toolstack on
> > perl.
> >
> > Suggested-by: Ian Jackson <ian.jackson@citrix.com>
> > Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
>
> Ian, ping?
>
> I already applied patch 1. This patch looks to be good as well.
Sorry. Yes.
Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
On Mon, Mar 30, 2020 at 02:28:30PM +0100, Ian Jackson wrote: > Wei Liu writes ("Re: [PATCH 2/2] scripts: Use stat to check lock claim"): > > On Thu, Mar 12, 2020 at 10:54:17AM -0400, Jason Andryuk wrote: > > > Replace the perl locking check with stat(1). Stat is able to fstat > > > stdin (file descriptor 0) when passed '-' as an argument. This is now > > > used to check $_lockfd. stat(1) support for '-' was introduced to > > > coreutils in 2009. > > > > > > After A releases its lock, script B will return from flock and execute > > > stat. Since the lockfile has been removed by A, stat prints an error to > > > stderr and exits non-zero. Redirect stderr to /dev/null to avoid > > > filling /var/log/xen/xen-hotplug.log with "No such file or directory" > > > messages. > > > > > > Placing the stat call inside the "if" condition ensures we only check > > > the stat output when the command completed successfully. > > > > > > This change removes the only runtime dependency of the xen toolstack on > > > perl. > > > > > > Suggested-by: Ian Jackson <ian.jackson@citrix.com> > > > Signed-off-by: Jason Andryuk <jandryuk@gmail.com> > > > > Ian, ping? > > > > I already applied patch 1. This patch looks to be good as well. > > Sorry. Yes. > > Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com> Thanks, I will apply this patch shortly. Wei.
diff --git a/tools/hotplug/Linux/locking.sh b/tools/hotplug/Linux/locking.sh index baaaf73508..69aefae6f6 100644 --- a/tools/hotplug/Linux/locking.sh +++ b/tools/hotplug/Linux/locking.sh @@ -41,7 +41,7 @@ claim_lock() # from chiark-utils, except using flock. It has the benefit of # it being possible to safely remove the lockfile when done. # See below for a correctness proof. - local rightfile + local stat while true; do eval "exec $_lockfd<>$_lockfile" flock -x $_lockfd || return $? @@ -50,14 +50,24 @@ claim_lock() # 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 + # file system to find the inum. stat(1) translates '-' into an + # fstat(2) of FD 0. So we just need to arrange the FDs properly + # to get the fstat(2) we need. stat will output two lines like: + # WW.XXX + # YY.ZZZ + # which need to be separated and compared. + if stat=$( stat -L -c '%D.%i' - $_lockfile 0<&$_lockfd 2>/dev/null ) + then + local file_stat + local fd_stat + + # match on literal newline + fd_stat=${stat% +*} + file_stat=${stat#* +} + if [ "$fd_stat" = "$file_stat" ] ; then break; fi + fi # Some versions of bash appear to be buggy if the same # $_lockfile is opened repeatedly. Close the current fd here. eval "exec $_lockfd<&-"