xCAT/xcat-core
2
0
Fork 0
mirror of https://github.com/xcat2/xcat-core.git synced 2026-09-28 16:20:51 +00:00
Code Issues Releases Wiki Activity

fix(xcat-core): build locks the shared tree refuses to grant

Browse Source 
A build tree can live on an NFS re-export. The kernel refuses locks on one --
"Clients are not allowed to get file locks or delegations from a reexport
server" -- so every flock() there answers errno 524, and a build that takes one
dies before it starts.

buildrpms.pl's per-target lock and BuildUtils.pm's take_build_lock, which
builddebs.pl calls for the Ubuntu core build, are both atomic mkdir claims now.
Each records its owner and names it when it refuses.

A directory is not released by a filehandle closing, which is how both locks
were freed before. buildrpms.pl releases from END, and again in abort_builds
because that handler re-raises the signal with DEFAULT and END blocks do not run
then -- a killed build would otherwise strand the lock for every later one.
BuildUtils returns a small object whose DESTROY releases it, preserving the
caller's "hold the returned value" contract.

Both releases are guarded by owning pid: both scripts fork, and the flock they
replace could not be released by a child.

builddebs_lock.t closed the returned value to prove the lock is released, which
is "Not a GLOB reference" against the new contract. It now lets the value go out
of scope. What it asserts is unchanged: a second build of the same checkout is
refused, and the next one succeeds once the first releases.

Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
This commit is contained in:
Daniel Hilst
2026-09-23 10:39:36 -03:00
parent 2f715ac371
commit 88441f6ec3
3 changed files with 46 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
build-utils/lib/XCAT/BuildUtils.pm
+21 -6
View File
@@ -463,12 +463,27 @@ sub lock_path_for {
# Returns the open handle -- the lock is held for as long as the caller keeps it.
sub take_build_lock {
my ($path, $dir) = @_;
require Fcntl;
my $lockfile = lock_path_for($path, $dir);
open my $fh, '>', $lockfile or die "FATAL: cannot open $lockfile: $!\n";
flock($fh, Fcntl::LOCK_EX() | Fcntl::LOCK_NB())
or die "FATAL: another build of $path already holds $lockfile\n";
return $fh;
require POSIX;
# A directory, not an flock. A build tree can live on an NFS re-export, where the kernel
# refuses locks outright: every attempt answers errno 524. mkdir(2) is arbitrated by the
# server and needs no lock daemon.
my $lockdir = lock_path_for($path, $dir) . '.d';
unless (mkdir $lockdir) {
die "FATAL: cannot take $lockdir: $!\n" unless $! == POSIX::EEXIST();
my $who = ''; if (open(my $h, '<', "$lockdir/owner")) { local $/; $who = <$h> // ''; close $h }
chomp $who;
die "FATAL: another build of $path already holds $lockdir"
. ($who ? " (held by [$who])" : "") . "\n";
}
if (open(my $ow, '>', "$lockdir/owner")) { print {$ow} "pid=$$\n"; close $ow }
# The caller keeps the returned value; release is by pid so a fork cannot free the parent's.
my $owner = $$;
return XCAT::BuildUtils::_BuildLock->new($lockdir, $owner);
}
{ package XCAT::BuildUtils::_BuildLock;
sub new { my ($c,$d,$p)=@_; return bless { dir=>$d, pid=>$p }, $c }
sub DESTROY { my $s=shift; return unless $$ == $s->{pid}; unlink "$s->{dir}/owner"; rmdir $s->{dir} }
}
# The rpm architecture a mock target builds for. A target carries the arch as its
buildrpms.pl
+20 -10
View File
@@ -45,7 +45,7 @@ use FindBin qw($Bin);
use lib "$Bin/build-utils/lib";
use XCAT::BuildUtils qw(git_revision source_date_epoch sh sh_or_die usage buildinfo_text
write_script read_line targetarch_from_target);
use Fcntl qw(:flock); # per-target build lock (concurrency guard; see main())
use POSIX (); # EEXIST, for the directory build lock (see main())
use Getopt::Long qw(GetOptions);
use POSIX qw(strftime);
use Parallel::ForkManager;
@@ -870,9 +870,17 @@ sub merge_core_repos {
# file and the cached chroot dirs left behind are normal mock state, not leaks.)
my %MOCK_INFLIGHT; # ForkManager child pid => mock chroot (-r) name it is building
my $ABORTING = 0;
my $BUILD_LOCK_FH; # per-target build flock; MUST stay file-scoped so the fd (and thus the
# lock) lives for the whole process. A lexical inside main()'s block would
# be DESTROYED at block exit -> lock released before any worker forks.
my $BUILD_LOCK_DIR; # per-target build lock, an atomic mkdir rather than an flock. The build
# tree can live on an NFS re-export, where the kernel refuses locks
# outright ("Clients are not allowed to get file locks or delegations from
# a reexport server"), so every flock there fails with errno 524. mkdir(2)
# is arbitrated by the server and needs no lock daemon. File-scoped so the
# release runs once, from END, for the whole process.
my $BUILD_LOCK_PID; # the pid that took it. ForkManager children inherit $BUILD_LOCK_DIR, and
# their END would release the PARENT's lock -- the flock this replaces
# could not be released by a child, and neither may this.
END { rmdir $BUILD_LOCK_DIR if defined $BUILD_LOCK_DIR and defined $BUILD_LOCK_PID and $$ == $BUILD_LOCK_PID }
my @CHILD_FAILURES; # idents (chroot names) of ForkManager children that exited non-zero
# PIDs of running mock processes whose `-r <chroot>` matches one of @chroots.
@@ -929,6 +937,9 @@ sub abort_builds {
sweep_mock_mounts(@chroots);
}
warn "[buildrpms] abort cleanup done\n";
# The re-raise below kills this process by signal, and END blocks do not run then. Release the
# build lock here or a killed build strands it for every later run.
rmdir $BUILD_LOCK_DIR if defined $BUILD_LOCK_DIR and defined $BUILD_LOCK_PID and $$ == $BUILD_LOCK_PID;
$SIG{$sig} = 'DEFAULT';
kill $sig, $$; # re-raise for the correct exit status
}
@@ -958,9 +969,10 @@ sub main {
my $key = join('-', $opts{targets}->@*)
. ($opts{mock_uniqueext} ? "-$opts{mock_uniqueext}" : "");
$key =~ s/[^A-Za-z0-9._-]/-/g;
my $lock = "/var/lock/buildrpms.$key.lock";
if (open($BUILD_LOCK_FH, '>', $lock)) {
unless (flock($BUILD_LOCK_FH, LOCK_EX | LOCK_NB)) {
my $lock = "/var/lock/buildrpms.$key.lock.d";
{
unless (mkdir $lock) {
die "FATAL: cannot take the build lock $lock: $!\n" unless $! == POSIX::EEXIST();
die "FATAL: another buildrpms.pl is already building target '@{$opts{targets}}'"
. ($opts{mock_uniqueext} ? " (uniqueext=$opts{mock_uniqueext})" : "") . ".\n"
. " ($lock is held). Concurrent builds of the same target collide on the shared\n"
@@ -968,9 +980,7 @@ sub main {
. " cleanup unmounts this build's chroot). Serialize them, or pass a distinct\n"
. " --mock-uniqueext per build.\n";
}
# $BUILD_LOCK_FH is file-scoped, so the fd stays open (lock held) until this process
# exits. Child forks inherit the fd but their exits never release it (the parent's
# still-open fd keeps the lock), which is exactly what we want.
$BUILD_LOCK_DIR = $lock; $BUILD_LOCK_PID = $$;
}
}
xCAT-test/unit/builddebs_lock.t
+5 -3
View File
@@ -44,9 +44,11 @@ like( $@, qr/already holds/, 'and says which checkout is already building' );
my $stable = eval { take_build_lock('/opt/builds/stable/xcat-core', $lockdir) };
ok( $stable, 'a build of a DIFFERENT checkout runs concurrently' );
# Releasing lets the next build in.
close $first;
# Releasing lets the next build in. The lock is a directory now, not an flock on a filehandle --
# an NFS re-export refuses locks outright (errno 524) -- so it is freed when the returned object
# goes out of scope, not when a handle is closed.
undef $first;
my $again = eval { take_build_lock($devel, $lockdir) };
ok( $again, 'the lock is released when the handle is closed' );
ok( $again, 'the lock is released when the returned value goes out of scope' );
done_testing();