7 months agoubik: Introduce IndexOf() 60/14060/7
Marcio Barbosa [Fri, 28 Feb 2020 02:41:53 +0000]
ubik: Introduce IndexOf()

To make the ubik_Call* functions cleaner, consolidate code that finds
the index of the connection associated with a host into a new function.

No functional change should be incurred by this commit.

Change-Id: I320d7a41221cb533e8d077c412f872152ac43b75
Reviewed-by: Andrew Deason <>
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

7 months agoafs: Handle osi_NewVnode failures 01/13701/8
Andrew Deason [Thu, 18 Jul 2019 21:21:10 +0000]
afs: Handle osi_NewVnode failures

Currently, code inside afs_vcache.c assumes that osi_NewVnode always
returns non-NULL, which means that osi_NewVnode must panic if it
cannot create a new vnode.

All of the callers of afs_GetVCache, afs_NewVCache, etc, already
handle getting a NULL return, though (after all, the given fid may not
exist or be inaccessible due to network errors, etc). So, just
propagate NULL returns from osi_NewVnode up to our callers, to avoid
panics in these situations.

Modify osi_NewVnode on many arches to return an error on allocation
failure, instead of panic'ing.

Change-Id: Ib578b1747590bdf65327d4674e0849811ed999eb
Reviewed-by: Benjamin Kaduk <>
Reviewed-by: Yadavendra Yadav <>
Tested-by: BuildBot <>

7 months agostats: incorrect clock square algorithm 76/14376/2
Mark Vitale [Mon, 18 Sep 2017 23:45:10 +0000]
stats: incorrect clock square algorithm

Since the original IBM code import, OpenAFS has an algorithm for
squaring clock values, implemented identically in three different
places.  This algorithm does not account correctly for microsecs
overflow into seconds, resulting in incorrect "sum-of-squares" values
for queue and execution time in several OpenAFS performance utilities.

Specifically, this code:

        t1.tv_usec += (2 * t2.tv_sec * t2.tv_usec) % 1000000                   \
                      + (t2.tv_usec / 1000)*(t2.tv_usec / 1000)                \
                      + 2 * (t2.tv_usec / 1000) * (t2.tv_usec % 1000) / 1000   \
                      + (((t2.tv_usec % 1000) > 707) ? 1 : 0);                 \

Can allow for the tv_usec field to be increased by a theoretical max
of around:

        t1.tv_usec += 999998                                                   \
                      + 999*999                                                \
                      + 2 * 999 * 999 / 1000                                   \
                      + 1;                                                     \


        t1.tv_usec += 1999996;                                                 \

If t1.tv_usec is already 999999, after this calculation its value
could be as high as 2999995. So just checking once if t1.tv_usec is
over 1000000 is not sufficient, since the resulting value (1999995) is
still over 1000000.

Correct all implementations by repeatedly checking if tv_usec is over
1000000 after the above calculation:

macro                   affected utility
=====================   ============================
afs_stats_SquareAddTo   xstat_cm_test
fs_stats_SquareAddTo    xstat_fs_test
clock_AddSq             rxstat_get_process and _peer

Change-Id: I3145d592ba6bc1556729eac657f43d476c99eede
Tested-by: BuildBot <>
Reviewed-by: Cheyenne Wills <>
Reviewed-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>

7 months agorxstats: correctly report vlserver VL_* RPC stats 75/14375/3
Mark Vitale [Mon, 28 Sep 2020 20:35:38 +0000]
rxstats: correctly report vlserver VL_* RPC stats

Since the original IBM code import, rxstat_get_process and
rxstat_get_peer have reported vlserver VL_* RPC stats as for the
"volserver interface".

Correct this to read "vlserver interface".

Change-Id: Ie65fd41150bed8180ad8792c21a67012084459ab
Tested-by: BuildBot <>
Reviewed-by: Cheyenne Wills <>
Reviewed-by: Benjamin Kaduk <>

7 months agorxstats: correctly distinguish client and server stats 74/14374/3
Mark Vitale [Mon, 28 Sep 2020 19:40:34 +0000]
rxstats: correctly distinguish client and server stats

Commit d3eaa39da3693bba708fa2fa951568009e929550 'rx: Make the rx_call
structure private' inadvertently caused all rxstats (aka rpcstats) to be
recorded as client stats by hardcoding the value for isServer to 1.

Therefore, when peer or process rxstats are enabled for a OpenAFS
component, the rxstat_get_process and rxstat_get_peer utilities will
erroneously report both client and server stats as "accessed as a client".

This is particularly problematic for ubik VOTE_* and DISK_* RPC stats,
for which a given ubik server may be both client and server over time.
In this case, both client and server stats are conflated into the same
"accessed as a client" counters.

Instead, properly pass the value of isServer from
rx_RecordCallStatistics through to rxi_IncrementTimeAndCount.

Note to maintainers:
This bug is only in master and all 1.8.x releases; no 1.6.x releases are

Confusingly, isServer=1 indicates client stats and isServer=0 indicates
server stats.  However, this is a quirk of the original implementation
and wire format of the RXSTATS_* RPCs and cannot be changed.  isServer
is actually shorthand for "remote is server"; thus all RPC client stubs
record their rxstats with isServer == 1, and all RPC server stubs record
their rxstats with isServer == 0.

Change-Id: I2420f807e2c18ddfb9de7093a487825fa2d0a68e
Tested-by: BuildBot <>
Reviewed-by: Cheyenne Wills <>
Reviewed-by: Benjamin Kaduk <>

7 months agovolser: Close dirp on error in ConvertROtoRW 42/14342/3
Marcio Barbosa [Thu, 3 Sep 2020 20:11:34 +0000]
volser: Close dirp on error in ConvertROtoRW

Currently, if SAFSVolConvertROtoRWvolume cannot create a new transaction
for the volume to be converted, it returns without closing the directory
stream opened by it. To prevent this leak, go through a new 'goto done'
destructor if NewTrans fails.

Change-Id: Ie0580e7739ae667f1cd2f9cabb8aaf5e15d3f2dd
Tested-by: BuildBot <>
Reviewed-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>

7 months agobozo: Log each dir and file with bad access rights 30/14330/2
Michael Meffie [Fri, 28 Aug 2020 15:24:10 +0000]
bozo: Log each dir and file with bad access rights

The bosserver directory and file access check stops after finding one
directory or file with incorrect permissions or owner. A log message is
written for this first one found, but more than one directory or file
may have incorrect access rights.

Instead check all of them so the bosserver logs a warning message for
each incorrect director or file permission found.  This should make it
easier to fix all of the file permission problems at once.

Change-Id: Ia3f14800ce036aa390929109a286cf21828e8a35
Tested-by: BuildBot <>
Reviewed-by: Cheyenne Wills <>
Reviewed-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>

7 months agobozo: Add KeyFileExt and rxkad.keytab to access rights check 29/14329/3
Michael Meffie [Fri, 28 Aug 2020 15:23:00 +0000]
bozo: Add KeyFileExt and rxkad.keytab to access rights check

When the KeyFileExt and rxkad.keytab were added to OpenAFS, they were
not added to the bosserver's access rights check. Add these files to the
bosserver access checks, with the same access rights needed for the
original KeyFile.

Also, add the full path for KeyFileExt to the dirpath package (not just
the filename), which was not done when the KeyFileExt was introduced.
This is needed to perform the access checks.

Change-Id: I8c9028e846fad9f15823baeb7cc15a8f80ed5c1c
Tested-by: BuildBot <>
Reviewed-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>

7 months agoafs: remove vestigial externs for afs_xvcache 06/14406/2
Mark Vitale [Wed, 23 Sep 2020 21:32:40 +0000]
afs: remove vestigial externs for afs_xvcache

These have not been needed since src/afs/afs_prototypes.h gained 'extern
afs_rwlock_t afs_xvcache' with commit

Remove the vestigial extern references.

Change-Id: Id6aceff0d5df1f1bed210a3fbf2951c62f35ddbb
Tested-by: BuildBot <>
Reviewed-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>

7 months agoafs: remove vestigial externs for afs_xcbhash 05/14405/2
Mark Vitale [Wed, 23 Sep 2020 21:02:52 +0000]
afs: remove vestigial externs for afs_xcbhash

Commit 64cc7f0ca7a44bb214396c829268a541ab286c69 "afs: Create
afs_StaleVCache" consolidated many references to afs_xcbhash into a new
function afs_StaleVCache.  However, this left many references to 'extern
afs_wrlock_t afs_xcbhash' that are no longer needed.

But actually, many of these have not been needed since
src/afs/afs_prototypes.h gained 'extern afs_rwlock_t afs_xcbhash' with
commit 8f2df21ffe59e9aa66219bf24656775b584c122d

Remove the vestigial extern references.

No functional change is incurred by this commit.

Change-Id: Ie6cfb6d90c52951795378d3b42e041567d207305
Reviewed-by: Andrew Deason <>
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

7 months agoxstat: prevent CPU loop when -period 0 66/14366/4
Mark Vitale [Fri, 18 Sep 2020 16:46:57 +0000]
xstat: prevent CPU loop when -period 0

Historically xstat_cm_test and xstat_fs_test have supported option
'-period <mm>' to specify continuous operaiton for a length of time.  If
'-period 0' was specified, both programs exited immediately.

Beginning with commits 2c1a7e47336c8f8d14dd6c65d53925a9e0e87c66 'xstat:
add xstat_*_Wait functions' and 6b67cac432043a43d7cdfa6af972ab54412aff94
'convert xstat and friends to pthreads', xstat_cm_test and xstat_fs_test
now support -period 0 to run "forever".  This support is implemented in
xstat_cm_Wait and xstat_fs_Wait, respectively.  Although the "wait
forever" logic was added to allow consolidation of similar code in
afsmonitor, it also changed how xstat_cm_test and xstat_fs_test behave
for '-period 0'.

Unfortunately, there is a bug in this support, at least when running on
pthreads.  After the initial 24 minute timer expires, the while (1) will
repeatedly run select with a timeout that is now 0.  This causes the
while loop to consume 100% of the CPU on which this thread is

Instead, modify the wait-forever logic to specify NULL for the select()
timeout value.  Also update the man page to document that '-period 0'
means forever.

Change-Id: I25d0d5be0eedb8bf3de495785b9b03a3e3d45221
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

7 months agoafs: Return to userspace after AFS_NEW_BKG reqs 84/13984/4
Andrew Deason [Fri, 13 Dec 2019 03:00:20 +0000]
afs: Return to userspace after AFS_NEW_BKG reqs

Currently, for AFS_NEW_BKG, background daemons run in the context of a
normal user process (afsd), in order to return to run
userspace-handled background ops. For non-AFS_NEW_BKG when
AFS_DAEMONOP_ENV is defined, background daemons run as kernel threads
instead, and have no corresponding userspace process.

On LINUX, whether or not we run as a kernel thread has some odd
side-effects: at least one example of this is how open file handles
(struct file) are treated when closed. When the last reference to a
struct file is closed, the final free is deferred to an asynchronous
call to be executed "later", in order to avoid issues with lock
inversion. For kernel threads, "later" means the work is schedule on
the global system work queue (schedule_work()), but for userspace
processes, it is scheduled on the task work queue (task_work_add()),
which is run around when the thread returns to userspace. For
background daemons, we never return from the relevant syscall until we
get a userspace background request (or the client is shutting down),

Commit ca472e66 (LINUX: Turn on AFS_NEW_BKG) changed LINUX to use
AFS_NEW_BKG background daemons, so background requests now run as a
normal userspace process, and last-reference file closes are deferred.
Since we may never return to userspace, this means that our file
handles (used for accessing the disk cache) may never get freed,
leading to an unbounded number of file handles remaining open.

This can be seen by seeing the first value in /proc/sys/fs/file-nr
growing without bound (possibly slowly), as accessing /afs causes
background requests. Eventually the number of open files can exceed
the /proc/sys/fs/file-max limit, causing further file opens to fail,
causing various other problems and potentially panics.

To avoid this issue, define a new userspace background op, called
AFS_USPC_NOOP, which gets returned to the afsd background daemon
process. When afsd sees this, it just does nothing and calls the
AFSOP_BKG_HANDLER syscall again, to go into the background daemon loop
again. In afs_BackgroundDaemon, we return the AFS_USPC_NOOP op
whenever there are no pending background requests, or if we've run 100
background requests in a row without a break. This causes us to return
to userspace periodically, preventing any such task-queued work from
building up indefinitely.

Do this for all platforms (currently just LINUX and DARWIN), in order
to simplify the code, and possibly avoid other similar issues, since
staying inside a syscall for so long while doing real work is arguably

Add a documentation comment block for afs_BackgroundDaemon while we're

Thanks to for discovering the file leak.

Change-Id: I1953d73b2142d4128b064f8c5f95a5858d7df131
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

7 months agoubik: Remove unused sampleName 86/14386/2
Andrew Deason [Wed, 14 Oct 2020 01:18:59 +0000]
ubik: Remove unused sampleName

The RPC-L type sampleName and related constant UMAXNAMELEN are not
referenced by anything, and have been unused since OpenAFS 1.0. Remove
the unused definitions.

Change-Id: I21a11d9db9ed80547de8685623fb09f9a86934f1
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

8 months agodir: Set srcdir correctly in src/dir/test 84/14384/2
Andrew Deason [Tue, 13 Oct 2020 17:17:37 +0000]
dir: Set srcdir correctly in src/dir/test

srcdir is a magic variable that needs to be set to @srcdir@, not some
relative path like ../../.. (which will usually be somewhere in the
objdir, not srcdir). Set it correctly in here.

Without this, objdir builds can fail with:

    make[4]: Entering directory '...obj/src/dir/test'
    make[4]: *** No rule to make target 'dtest.o', needed by 'dtest'.  Stop.

Which happens because the automatic rule for dtest.o can't be
constructed, since we cannot find dtest.c automatically because srcdir
isn't set properly.

This has been broken since commit 37b4195d (dtest-20021111), but was
not noticeable until commit 192a2ff4 (dir: make dtest buildable
again), since that caused dtest to actually get built.

Also set LIBS correctly in here, using the conventional ${TOP_LIBDIR},
since ${srcdir} no longer points to "../../..".

Change-Id: I539e01a4397c558dc0eda492834b3f9913f71634
Tested-by: BuildBot <>
Reviewed-by: Michael Meffie <>
Reviewed-by: Benjamin Kaduk <>

8 months agobozo: Use libcmd for command line options 45/13845/16
Cheyenne Wills [Fri, 21 Aug 2020 18:53:30 +0000]
bozo: Use libcmd for command line options

Update bosserver to use libcmd for command line parsing.

Change-Id: Iaa55dc33b72983a48089a7b359260916bea2d1e7
Reviewed-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

8 months agoafs: refactor directory checking in DRead 03/13803/9
Mark Vitale [Mon, 4 Mar 2019 06:37:53 +0000]
afs: refactor directory checking in DRead

Commit d566c1cf874d15ca02020894ff0af62c4e39e7bb
'dread-do-validation-20041012' modified directory checking (in the
afs_buffer.c implementation of DRead()) to use size information passed
to DRead, rather than obtained from the cache via afs_CFileOpen.

Because this directory checking does not require any information from
the cache buffers or the cache partition, we can make the check right
away, before searching the cache buffers or calling afs_newslot.

To clarify and simplify, move the directory sanity checking logic to the
beginning of DRead.  Remove the afs_newslot cleanup logic which is no
longer needed.

While here, add Doxygen comments for DRead.

Change-Id: I8cea4e885ece64e760271c8194c126250f87104e
Reviewed-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

8 months agodir: check afs_dir_MakeDir return code in dtest 97/13797/6
Mark Vitale [Thu, 7 Mar 2019 19:31:49 +0000]
dir: check afs_dir_MakeDir return code in dtest

The dtest test program ignores the return from afs_dir_Makedir.

Fix this so errors may be identified in testing.

While here, also improve the diagnostic message for afs_dir_Create
failures, to make it consistent with the new diagnostic message for
afs_dir_MakeDir failures.

Change-Id: Ib882947e01c864344f17faad8a646b2487793f29
Reviewed-by: Andrew Deason <>
Reviewed-by: Michael Meffie <>
Reviewed-by: Cheyenne Wills <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

8 months agodir: dtest should flush on error when creating directories 96/13796/6
Mark Vitale [Wed, 6 Mar 2019 16:27:58 +0000]
dir: dtest should flush on error when creating directories

The dtest -f subcommand (CRTest()) exits immediately if there is an
error while adding files.  This may create an empty, incomplete, or
corrupt directory object on disk because we neglected to call DFlush
before exiting.

Always call DFlush from CRTest() whether it fails or succeeds.

Change-Id: Ia7b4ad00ea6f4f9f788cd75ae726bdadb60ee9c3
Reviewed-by: Andrew Deason <>
Reviewed-by: Cheyenne Wills <>
Reviewed-by: Benjamin Kaduk <>
Reviewed-by: Michael Meffie <>
Tested-by: BuildBot <>

8 months agodir: correct fid type for dtest 95/13795/6
Mark Vitale [Wed, 6 Mar 2019 04:20:10 +0000]
dir: correct fid type for dtest

The dtest utility has had its fid[] arrays defined as 'long' since the
initial IBM import.  Commit 0a98548832472152304410e41306adcc5b91f6a2
'dir: Make test utility build again' converted some - but not all - the
fid arrays to afs_int32.

Allow dtest to operate correctly by converting the rest of the fid
arrays to afs_int32.

Change-Id: I2ebe36272e02cf860577153ab94f3591e1d707e8
Reviewed-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>
Reviewed-by: Cheyenne Wills <>
Reviewed-by: Michael Meffie <>
Tested-by: BuildBot <>

8 months agodir: make dtest buildable again 94/13794/6
Mark Vitale [Wed, 6 Mar 2019 04:11:38 +0000]
dir: make dtest buildable again

Commit 7fe4125fe3435092b75ed29b884d8d3c2d1a2cad 'dir/vol: Die() really
does' overlooked src/dir/test/dtest.c, breaking its build.

Fix the signature of Die() and the makefile so dtest can be built.
In addition, change the Makefile so it is always built.

Change-Id: I18129acbfdaa770987c7f0b8055ff593f776e518
Reviewed-by: Andrew Deason <>
Reviewed-by: Cheyenne Wills <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

8 months agodir: remove unused test files 52/14052/5
Mark Vitale [Fri, 4 Oct 2019 18:52:21 +0000]
dir: remove unused test files

Makefile rules for physio.c and test-salvage.c have been commented out
since the original IBM code import, and were removed in commit
37b4195d603630498664fa0975ea5d5c82f9aa4f 'dtest-20021111' to fix dtest.
However, that commit neglected to remove the source files and other
references to them in

Finish the job by removing the files and references to them.

No functional change is incurred by this commit.

Change-Id: I57527be99cd28a481a86b659d1eb3227af9f1c99
Reviewed-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

8 months agovol: de-orbit test programs 93/13793/6
Mark Vitale [Mon, 4 Mar 2019 03:06:28 +0000]
vol: de-orbit test programs

The updateDirInode and listVicepx utilities are obsolete; they no longer
build, are severely bitrotted, and have been largely replaced by

While here, also remove other objects that have not been built by default
since before the original IBM import:
- ILIST ilist.exe
- NAMEI_PROGS nicreate, nincdec, nino, nilist

Remove all of them from the tree.

Change-Id: I8f68ec425cce5e84bcc5f41d598eec23102109de
Reviewed-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

8 months agoMake OpenAFS 1.9.0 62/14362/2 openafs-devel-1_9_0
Benjamin Kaduk [Fri, 18 Sep 2020 15:56:44 +0000]
Make OpenAFS 1.9.0

Update version strings for the first 1.9.x development release.

Change-Id: I0d0e204ffe8d64d7c0f794f313c0f24ccea12783
Reviewed-by: Michael Meffie <>
Reviewed-by: Cheyenne Wills <>
Reviewed-by: Stephan Wiesand <>
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

8 months agoImport NEWS from OpenAFS 1.8.6 44/14344/4
Benjamin Kaduk [Fri, 4 Sep 2020 15:56:36 +0000]
Import NEWS from OpenAFS 1.8.6

Stay up to date with the stable branch at least until the initial
version of the new release series.

Change-Id: Iefcd9cc039399cd4cbbcc0474c2cabffa7780305
Reviewed-by: Benjamin Kaduk <>
Tested-by: Benjamin Kaduk <>

8 months agoUpdate 1.9.0 NEWS for recent changes 43/14343/4
Benjamin Kaduk [Fri, 4 Sep 2020 15:55:19 +0000]
Update 1.9.0 NEWS for recent changes

Add some entries for the commits that landed since the previous update.

Change-Id: I74820ee5a07c3fb539f233b2bd0c30aab262ba74
Tested-by: BuildBot <>
Reviewed-by: Michael Meffie <>
Reviewed-by: Benjamin Kaduk <>

9 months agoDARWIN: disable kextutil check for versions requiring notarization 22/14222/3
Mark Vitale [Tue, 12 May 2020 16:59:31 +0000]
DARWIN: disable kextutil check for versions requiring notarization

Our kextutil signing check will fail for releases that require
notarization (Mojave 10.14.5 and up, Catalina 10.15 all versions),
because we aren't notarized yet at the time of the check.

Instead, disable the check for those releases.

Change-Id: Iec1b74d18ae02cdd031ed3194ffb9900aa8a1b55
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

9 months agodumpscan: Don't call cb_dirent twice 08/14308/4
Thomas L. Kula [Thu, 14 May 2009 18:08:40 +0000]
dumpscan: Don't call cb_dirent twice

This fixes a bug where p->cb_dirent is called twice, if
it exists.

Change-Id: I7a7a6abf522b62eb310d003a61b3bbcdcda9e850
Reviewed-by: Andrew Deason <>
Reviewed-by: Michael Meffie <>
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

9 months agoRevert "vos: take RO volume offline during convertROtoRW" 39/14339/3
Marcio Barbosa [Mon, 31 Aug 2020 19:56:56 +0000]
Revert "vos: take RO volume offline during convertROtoRW"

This reverts commit 32d35db64061e4102281c235cf693341f9de9271. While that
commit did fix the mentioned problem, depending on "vos" to set the
volume to be converted as "out of service" is not ideal. Instead, this
volume should be set as offline by the SAFSVolConvertROtoRWvolume RPC,
executed on the volume server.

The proper fix for this problem will be introduced by another commit.

Change-Id: I0ce5ba793fe3c07e535225191b74eeb402ab5bfd
Reviewed-by: Cheyenne Wills <>
Reviewed-by: Michael Meffie <>
Reviewed-by: Andrew Deason <>
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

9 months agobuild: Add rpm target 14/14114/17
Michael Meffie [Mon, 24 Aug 2020 17:12:13 +0000]
build: Add rpm target

Add a top-level makefile target to build RPMs for Red Hat distributions
from the currently checked out commit. The resulting rpms are placed in
the packages/rpmbuild/RPMS/<arch> directory.

The rpm target is intended to be a convenience for testing changes to
the rpm packaging or generating packages for local testing.

Change-Id: Id951eb2b03629be59f6258e89e8356fe1fde1ff5
Reviewed-by: Andrew Deason <>
Reviewed-by: Cheyenne Wills <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

9 months agomakesrpm: Support custom version strings 16/14116/16
Michael Meffie [Fri, 1 May 2020 18:05:24 +0000]
makesrpm: Support custom version strings

The script generates a source RPM by creating a temporary
rpmbuild workspace, populating the SOURCES and SPECS directories in that
workspace, running rpmbuild to build the source RPM, and finally copying
the resulting source RPM out of the temporary workspace.

The name of the source RPM file created by rpmbuild depends on the
package version and release strings. Unfortunately, the format of the
source RPM file name changed around OpenAFS 1.6.0, so has
special logic to find the version string and extra code depending on the
detected OpenAFS version.

Instead of trying to predict the name of the resulting source RPM file
from the OpenAFS version string, and having different logic for old
versions of OpenAFS, use a filename glob to find resulting source RPM
file name in the temporary rpmbuild workspace.

Remove the major, minor, and patch level variables, which were only used
to guess the name of the resulting source RPM file name.

Convert '-' characters to '_' in the package version and package
release, since the '-' character is reserved by rpm as a field

While here, add the --dir option to specify the path of the generated
source RPM, and change the 'srpm' makefile target to use the new --dir
option, instead of changing the current directory before running  Also, add a dependency on the 'dist' makefile target,
since the the source and document tarballs are required to build the
source RPM.

Add pod documentation and add the --help (-h) option to print a brief
help message, and add the --man option to print the full man page.

With this change, we can build a source RPM even when the .version file
in the file has a custom format or was created from a
checkout of the master branch or other non-release reference.

Change-Id: I7320afe6ac1f77d4dd38fcc194d41678fde5c950
Tested-by: BuildBot <>
Reviewed-by: Andrew Deason <>
Reviewed-by: Cheyenne Wills <>
Reviewed-by: Benjamin Kaduk <>

9 months agoCorrect our contributor's code of conduct 20/14320/2
Stephan Wiesand [Tue, 25 Aug 2020 21:34:39 +0000]
Correct our contributor's code of conduct

There are no races. Racism does exist though.

Change-Id: I0a4cde55a5f470649eb99c5d7f30c9cec86d9baa
Tested-by: BuildBot <>
Reviewed-by: Michael Meffie <>
Reviewed-by: Mark Vitale <>
Reviewed-by: Benjamin Kaduk <>

9 months agoUKERNEL: Build linktest with COMMON_CFLAGS 24/14324/3
Andrew Deason [Wed, 26 Aug 2020 20:41:00 +0000]
UKERNEL: Build linktest with COMMON_CFLAGS

Currently, 'linktest' in libuafs is built with a weird custom rule
that specifies several various CFLAGS and LDFLAGS, etc. One
side-effect of this is that linktest is built without specifying -O,
even if optimization is otherwise enabled.

Normally nobody would care about the optimization of linktest, since
it's never supposed to be run, but this can cause an error when
building with -D_FORTIFY_SOURCE=1 on some systems (such as RHEL7):

    In file included from /usr/include/sys/types.h:25:0,
                     from /.../src/config/afsconfig.h:1485,
                     from /.../src/libuafs/linktest.c:15:
    /usr/include/features.h:330:4: error: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Werror=cpp]
     #  warning _FORTIFY_SOURCE requires compiling with optimization (-O)
    cc1: all warnings being treated as errors
    make[3]: *** [linktest] Error 1

For now, to fix this just include $(COMMON_CFLAGS) in the flags we
give for linktest, so $(OPTMZ) also gets pulled in, and building
linktest gets a little closer to a normal compilation step.

Change-Id: I3362dcfe8407825ab88854ae59da4188ed16be9d
Reviewed-by: Michael Meffie <>
Tested-by: BuildBot <>
Reviewed-by: Cheyenne Wills <>
Reviewed-by: Benjamin Kaduk <>

9 months agoptserver: Remove duplicate ubik_SetLock in listSuperGroups 38/14338/3
Jan Iven [Tue, 1 Sep 2020 12:51:25 +0000]
ptserver: Remove duplicate ubik_SetLock in listSuperGroups

It looks like a call to ubik_SetLock(.. LOCKREAD) was left in
place in listSuperGroups after locking was moved to ReadPreamble
in commit a6d64d70 (ptserver: Refactor per-call ubik initialisation)
When compiled with 'supergroups', and once contacted by
"pts mem -expandgroups ..", ptserver will therefore abort() with
Ubik: Internal Error: attempted to take lock twice
This patch removes the superfluous ubik_SetLock.

FIXES 135147

Change-Id: I8779710a6d68e4126fc482123b576690d86e4225
Tested-by: BuildBot <>
Reviewed-by: Michael Meffie <>
Reviewed-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>

9 months agoINSTALL: document the minimum Linux kernel level 05/14305/5
Cheyenne Wills [Mon, 24 Aug 2020 17:10:30 +0000]
INSTALL: document the minimum Linux kernel level

The change associated with gerrit #14300 removed support for older
Linux kernels (2.6.10 and earlier).

The commit 'Import of code from autoconf-archive' (d8205bbb4) introduced
a check for Autoconf 2.64.  Autoconf 2.64 was released in 2009.

The commit ' Use libtoolize -i, and .gitignore generated
build-tools' (a7cc505d3) introduced a dependency on libtool's  '-i'
option.  Libtool supported the '-i' option with libtool 1.9b in 2004.

Update the INSTALL instructions to document a minimum Linux kernel
level and the minimum levels for autoconf and libtool.

Notes: RHEL4 (EOL in 2017) had a 2.6.9 kernel and RHEL5 has a 2.6.18
kernel. RHEL5 has libtool 1.5.22 and autoconf 2.59, RHEL6 has libtool
2.2.6 and autoconf 2.63, and RHEL7 has libtool 2.4.2 and autoconf 2.69.

Change-Id: I235eeffa4adb152e05aab7aca839700816e62c83
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

9 months agoafs: Avoid NatPing event on all connection 12/14312/3
Yadavendra Yadav [Thu, 20 Aug 2020 20:24:00 +0000]
afs: Avoid NatPing event on all connection

Inside release_conns_user_server, connection vector is traversed and after
destroying a connection new eligible connection is found on which NatPing
event will be set. Ideally there should be only one connection on which
NatPing should be set but in current code while traversing all connection
of server a NatPing event is set on all connections to that server. In
cases where we have large number of connection to a server this can lead
to huge number of “RX_PACKET_TYPE_VERSION” packets sent to a server.
Since this happen during Garbage collection of user structs, to simulate
this issue below steps were tried

  - had one script which “cd” to a volume mount and then script sleeps for
    large time.
  - Ran one infinite while loop where above script was called using PAG
    based tokens (As new connection will be created for each PAG)
  - Instrumented the code, so that we hit above code segment where NatPing
    event is set. Mainly reduced NOTOKTIMEOUT to 60 sec.

To fix this issue set NatPing on one connection and once it is set break
from “for” loop traversing the server connection.

Change-Id: Ia38cec0403fde76cdd59aa664bd261481e2edee6
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>
Reviewed-by: Andrew Deason <>

9 months agovos: avoid 'half-locked' volume after interrupted 'vos rename' 57/14157/3
Mark Vitale [Mon, 20 Apr 2020 18:51:08 +0000]
vos: avoid 'half-locked' volume after interrupted 'vos rename'

Reported symptoms:

If a 'vos rename' is interrupted after it has locked the volume and
replaced the VLDB entry, but before it has unlocked the volume, the
volume will remain locked.  However, the locked volume will NOT be
listed as locked in any vos commands that display locked status (see
below for details).


Most vos write operations lock the VLDB volume entry before proceeding,
then release the volume lock when finished.  This is accomplished via
VL_SetLock and VL_ReleaseLock, respectively.

VL_SetLock always sets these members in the VLDB volume entry:
- flags is modified to set the required VLOP_* code bit as specified
- LockAFSid is set to 0 (never implemented)
- LockTimestamp is set to the current time

VL_ReleaseLock always sets them as follows:
- flags is cleared of any VLOP_* code bit
- LockAFSid is set to 0 (never implemented)
- LockTimestamp is set to 0

VL_ReplaceEntry(N) may also optionally clear each of these members:
- flags operation bits may be explicitly cleared via LOCKREL_OPCODE
- LockAFSid may be explicitly cleared via LOCKREL_AFSID
- LockTimestamp may be explicitly cleared via LOCKREL_TIMESTAMP

When all 3 options are specified, VL_ReplaceEntry also does the
functional equivalent of a VL_ReleaseLock.  Most vos operations use this
method.  However, when no lock release options are specified on
VL_ReplaceEntry(N), the VLDB entry is simply replaced with the supplied
entry.  This includes whatever flags values are specified in the
supplied entry; therefore, this amounts to an additional, implicit way
to set or modify the flags.

Root cause:

'vos rename' (UV_RenameVolume) is the only vos operation that does all
of the following things:
- accepts a replacement volume entry that was obtained before VL_SetLock
  (and thus does NOT have any lock flags set)
- issues VL_SetLock (which sets the lock flag in the VLDB)
- issues VL_ReplaceEntry(N) with the original unlocked entry, and with
  no lock release options (thus with explicit intent to leave the lock
  flag unchanged, but inadvertently doing an implicit clear of the lock
  flag in the VLDB)
- (performs some additional volserver work)
- issues VL_ReleaseLock to release the volume lock

Therefore, if 'vos rename' is cancelled or killed before reaching the
final VL_ReleaseLock step, the VLDB entry is left with the lock flags
cleared but the LockTimestamp still set.  As we will see below, this
'half-locked' state produces confusing results from other vos commands.

Detection of locked state:

The 'vos lock' command (and all other vos commands that issue
VL_SetLock) use the lock timestamp to determine if a volume is locked.

However, several other vos commands ('vos listvldb <vol>', 'vos examine
<vol>', 'vos listvldb -locked') use the VLDB entry's lock flags (not the
lock timestamp) to determine if the volume is locked.  Therefore, if the
lock flags have been cleared but the lock timestamp is still set, these
commands fail to detect that the volume is still locked.  Yet an
administrator's 'vos lock <volume>' will still fail with:

  Could not lock VLDB entry for volume <volume>
  VLDB: vldb entry is already locked

This is the external manifestation of the 'half-locked' state.

Workaround and fix:

This scenario has a simple workaround: 'vos unlock <volume>'.  However,
to avoid this confusing outcome in the first place, modify the 'vos
rename' logic so that the lock flags are no longer inadvertently
cleared.  Now, if the 'vos rename' is interrupted before the volume is
unlocked, it will still appear locked in normal vos command output.

Change-Id: I6cc16d20c4487de4e9a866c6f0c89d950efd2f7d
Tested-by: BuildBot <>
Reviewed-by: Andrew Deason <>
Reviewed-by: Michael Meffie <>
Reviewed-by: Cheyenne Wills <>
Reviewed-by: Benjamin Kaduk <>

9 months agorxgen: remove dead code hndle_param_tail 22/14322/2
Mark Vitale [Tue, 25 Aug 2020 16:37:09 +0000]
rxgen: remove dead code hndle_param_tail

Since the original IBM code import, hndle_param_tail has been dead code.
It was later ifdef'd out in commit 8f2df21ffe59

Remove the dead code from the tree.

No functional change is incurred by this commit.

Change-Id: I29128eecc93a5871f5bb9369c3983baf5b537beb
Tested-by: BuildBot <>
Reviewed-by: Cheyenne Wills <>
Reviewed-by: Michael Meffie <>
Reviewed-by: Benjamin Kaduk <>

9 months agobos: suppress unnecessary warn if -noauth 06/14306/2
Marcio Barbosa [Tue, 18 Aug 2020 13:56:26 +0000]
bos: suppress unnecessary warn if -noauth

Commit d008089a7 (Add interface to select client security objects)
consolidated the code that selects the client security objects into a
set of new interfaces. Before this commit, the "bos: running
unauthenticated" message, which warns the user when an unauthenticated
connection is established, used to be suppressed if the -noauth flag was

Similarly to commit b3c16324e (ubik: Make ugen_ClientInit honor
noAuthFlag), recover the original behavior avoiding warn messages about
unauthenticated connections if the -noauth flag is provided.

Change-Id: Iaf0ac6bd91ea160256823512f060afc94b5926bf
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

9 months agovlserver: fix missing read-only entries from ListAttributesN2 54/14154/7
Michael Meffie [Thu, 16 Apr 2020 20:29:09 +0000]
vlserver: fix missing read-only entries from ListAttributesN2

The ListAttributesN2() RPC can fail to list read-only entries under
certain circumstances. This RPC is used by the `vos listvldb` command to
retrieve vldb entries (unless the -name option is given). The `vos
listvldb` command fails to list volume entries when run with the
'-server' option for volumes that have read-only replicas, but have not
been released.

Consider the following example volume:

    $ vos create a test
    $ vos addsite a test
    $ vos addsite a test
    $ vos listvldb
        RWrite: 536870921
        number of sites -> 3
           server partition /vicepa RW Site
           server partition /vicepa RO Site  -- Not released
           server partition /vicepa RO Site  -- Not released

`vos listvldb` fails to find the volume when the search is limited to
server 'fs2':

    $ vos listvldb -server
    VLDB entries for server
    Total entries: 0

Instead of the expected results:

    $ vos listvldb -server
        RWrite: 536870921
        number of sites -> 3
           server partition /vicepa RW Site
           server partition /vicepa RO Site  -- Not released
           server partition /vicepa RO Site  -- Not released

This situation makes it difficult to remove old server addresses from
the vldb.  In this situation, 'vos remaddrs' and 'vos changeaddr
-remove' commands will complain the server addresses are still in use by
volume entries, however running 'vos listvldb -server' will not show
which volumes entries are in use.

The entries are not listed for unreleased volumes because the
ListAttributesN2() RPC is currently checking the volume VLF_ROEXISTS
flag, instead of the server site flags (serverFlags) to determine when
the entry is a read-only site. The volume VLF_ROEXISTS flag is set when
a volume is released.

To fix this, make ListAttributesN2 check for the VLSF_ROVOL site flag,
instead of the VLF_ROEXISTS entry flag.

Change-Id: Ib636fbe016d1d2f5b117624d9930dba83ebcef8a
Reviewed-by: Andrew Deason <>
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

9 months agoLINUX 5.9: Remove HAVE_UNLOCKED_IOCTL/COMPAT_IOCTL 00/14300/7
Cheyenne Wills [Mon, 17 Aug 2020 14:20:11 +0000]

Linux-5.9-rc1 commit 'fs: remove the HAVE_UNLOCKED_IOCTL and
HAVE_COMPAT_IOCTL defines' (4e24566a) removed the two referenced macros
from the kernel.

The support for unlocked_ioctl and compat_ioctl were introduced in
Linux 2.6.11.

Remove references to HAVE_UNLOCKED_IOCTL and HAVE_COMPAT_IOCTL using
the assumption that they were always defined.


With this change, building against kernels 2.6.10 and older will fail.
RHEL4 (EOL in March 2017) used a 2.6.9 kernel.  RHEL5 uses a 2.6.18

In linux-2.6.33-rc1 the commit messages for "staging: comedi:
Remove check for HAVE_UNLOCKED_IOCTL" (00a1855c) and "Staging: comedi:
remove check for HAVE_COMPAT_IOCTL" (5d7ae225) both state that all new
kernels have support for unlocked_ioctl/compat_ioctl so the checks can
be removed along with removing support for older kernels.

Change-Id: Idd2716f3573ea455f8a5e1535bca584af0787717
Reviewed-by: Benjamin Kaduk <>
Tested-by: Benjamin Kaduk <>

9 months agovos: avoid CreateVolume when restoring over an existing volume 08/14208/7
Michael Meffie [Fri, 15 May 2020 16:01:44 +0000]
vos: avoid CreateVolume when restoring over an existing volume

Currently, the UV_RestoreVolume2 function always attempts to create a
new volume, even when doing a incremental restore over an existing
volume.  When the volume already exists, the volume creation operation
fails on the volume server with a VVOLEXISTS error. The client will then
attempt to obtain a transaction on the existing volume. If a transaction
is obtained, the incremental restore operation will proceed. If a full
restore is being done, the existing volume is removed and a new empty
volume is created.

Unfortunately, the failed volume creation is logged to by the volume
server, and so litters the log file with:

    Volser: CreateVolume: Unable to create the volume; aborted, error code 104

To avoid polluting the volume server log with these messages, reverse
the logic in UV_RestoreVolume2. Assume the volume already exists and try
to get the transaction first when doing an incremental restore. Create a
new volume if the transaction cannot be obtained because the volume is
not present.  When doing a full restore, remove the existing volume, if
one exists, and then create a new empty volume.

Change-Id: I8bdc13130d12c81cd2cd18a9484852708cac64d7
Tested-by: BuildBot <>
Reviewed-by: Marcio Brito Barbosa <>
Tested-by: Marcio Brito Barbosa <>
Reviewed-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>

9 months agotests: Accommodate c-tap-harness 4.7 95/14295/5
Michael Meffie [Tue, 4 Aug 2020 14:34:07 +0000]
tests: Accommodate c-tap-harness 4.7

The SOURCE and BUILD environment variables have been changed to
C_TAP_SOURCE and C_TAP_BUILD in the new version of c-tap-harness.  The
runtests command syntax has changed as well.

Convert all of the old SOURCE and BUILD environment variables to the new

Add the required -l command line option to specify the test list.

Add the new runtests -v option to run the tests in verbose mode to make
it easier to see which tests failed.

Change-Id: I209a6dc13d6cd1507519234fce1564fc4641e70b
Tested-by: BuildBot <>
Reviewed-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>

9 months agoImport of code from c-tap-harness 94/14294/2
Russ Allbery [Tue, 4 Aug 2020 00:59:25 +0000]
Import of code from c-tap-harness

This commit updates the code imported from c-tap-harness to
abdb66561ffd4d2f238fdb06f448ccf09d80c059 (release/4.7)

Upstream changes are:

Daniel Collins (1):
      Add is_blob() test function.

Daniel Kahn Gillmor (1):
      LICENSE: use https for all URLs

Daria Brashear (1):
      Add verbose mode environment variable to runtests

Julien ÉLIE (2):
      Document -v in usage and comments of runtests
      Avoid realloc of zero length in tests/runtests.c

Marc Dionne (1):
      Add test_cleanup_register_with_data

Russ Allbery (115):
      clang --analyze cleanups for runtests
      Modernize POD tests
      Update README to my current layout
      Explicitly note that test programs must be executable
      Fix comment typo in tests/runtests.c
      Switch to a copyright-format 1.0 LICENSE file
      Flush harness output after each line
      Show the test count as ? when the plan is deferred
      More correctly backspace over test counts when aborting
      Refactor test list handling
      Allow passing tests on the runtests command line
      Don't allow command-line arguments if a list was given
      Search for tests under the name given as well
      Release 2.0
      Fix backward incompatibility when searching for tests
      Document decision to ignore TAP version directives
      Release 2.1
      Document different runtests behavior in bail handling
      Change exit status of bail to 255
      Release 2.2
      Add a new test_cleanup_register C API
      Add warn_unused_result attributes
      Add portability for warn_unsed_result attributes to tap/macros.h
      Minor coding style fix (spacing) in runtests.c
      Split the runtests usage string for ISO C90 string limits
      Include stddef.h
      Diagnose failure to register the exit handler
      Use diag internally in the basic C TAP library
      Some additional comments about cleanup functions
      Move repetitive printing code in the C TAP library to a macro
      Set a flag when bailing for more correct cleanup
      Change my email address to
      Release 2.3
      Add diag_file_add and diag_file_remove functions
      Don't die for unknown files passed to diag_file_remove
      Release 2.4
      Update comment about AIX and WCOREDUMP
      Don't test for NULL before calling free
      Be more careful about file descriptors in child processes
      Run cleanup functions in non-primary processes as well
      Release 3.0
      Update collective package copyright notices at start of LICENSE
      Check integer overflows on memory allocation, fix string creation
      Switch POD spelling test to use Lancaster consensus variable
      Add new bnrealloc API for brealloc with checked multiplication
      Rename nrealloc to reallocarray
      Return the test status from test functions
      Fix the overflow check for breallocarray
      Fix the overflow check for xreallocarray in runtests
      Restructure test result reallocation in runtests
      Change diag and sysdiag to always return true
      Release 3.1
      Fix typos in basic.c and basic.h
      Fix usage message when running runtests with no arguments
      Update introductory runtests comments for current syntax
      Add the -l flag to suggested runtests invocation in README
      Support comments and blank lines in test lists
      Release 3.2
      Update licensing information
      Various improvements to verbose support
      Compile warning-free with Clang, check Autoconf macros
      Release 3.3
      Remove unnecessary assert.h include in tap/basic.c
      Fix some additional -v documentation issues
      Rebalance usage to avoid too-long strings
      Fix segfault in runtests with empty test list
      Release 3.4
      Document running autogen if starting from Git
      Rename autogen to bootstrap
      Support and prefer C_TAP_SOURCE and C_TAP_BUILD
      Fix comment typo in tests/runtests.c
      Add missing va_end to is_double
      Release 4.0
      Fix all non-https URLs
      Add is_bool C test function
      Add DocKnot metadata and a Markdown README file
      Update documentation for new DocKnot standards
      Release 4.1
      Use more defaults from DocKnot templates
      Fix new fall-through warning in GCC 7
      Use compiler warnings from rra-c-util, fix issues
      Merge pull request #4 from solemnwarning/master
      Coding style fixes and NEWS for is_blob
      Re-enable -Wunknown-pragmas for GCC
      Avoid zero-length realloc allocations in breallocarray
      Update copyright date on tests/runtests.c
      Release 4.2
      Add SPDX-License-Identifier headers to source files
      Add and run new check-cppcheck target
      Fix instructions for running one test
      Identify values as left and right
      Fix is_string comparisons with NULL pointers
      Add support for running tests under valgrind
      Replace putc with fprintf
      Update shared files from rra-c-util
      Release 4.3
      Update NEWS date for 4.3 release
      Collapse some copyright dates
      NEWS and coding style for test_cleanup_register_with_data
      Remove unused variables caught by Clang scan-build
      Update to rra-c-util 8.0
      Fix error checking in bstrndup
      Release 4.4
      Add support for C++
      Document that C TAP Harness can be built as C++
      Release 4.5
      Regenerate README files
      Reformat using clang-format 10
      Update to rra-c-util 8.1
      Release 4.6
      Fix spelling errors caught by codespell
      Protect the test suite against C_TAP_VERBOSE
      Switch to GitHub Actions for CI
      Add NEWS entry for GCC 10 warning fixes
      Release 4.7

Change-Id: I5a78215bf99b53bd848f0fa6bb9092deab38f24e
Reviewed-by: Andrew Deason <>
Tested-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>

10 months agoafs: Always define our own osi_timeval32_t 38/14238/5
Andrew Deason [Tue, 2 Jun 2020 18:37:00 +0000]
afs: Always define our own osi_timeval32_t

Since OpenAFS 1.0, osi_GetTime has taken a timeval-like pointer, which
contains 32-bit fields (the actual type has been called either
osi_timeval_t or osi_timeval32_t over time). For platforms that have a
native timeval-like type with 32-bit fields, we just define
osi_timeval32_t to that type, and elsewhere we define our own struct
to be osi_timeval32_t. For platforms that use the native timeval, we
can then define osi_GetTime() to just be, e.g., microtime().

This approach is difficult to maintain, though, because we must keep
track of whether 'struct timeval' contains 32-bit fields on each
platform, which can depend on many factors. It's easy to make mistakes
(the current tree already contains mistakes), and there's not much

To avoid all of this, just always define osi_timeval32_t to be our own
struct with afs_int32 fields, and provide definitions for osi_GetTime
that convert from the native time struct to our osi_timeval32_t. This
does mean that for some platforms we do an unnecessary type
conversion, but this is a small price to pay for more straightforward
and maintainable code.

To be a little more sure that our types are correct, change
osi_GetTime to be defined as an inline function instead of a macro.

At the same time, do a similar conversion for the KERNEL
implementation of the rx clock_GetTime function. Get rid of
platform-specific mess, and do a straightforward type conversion
between osi_timeval32_t and struct clock in an inline function.

Change-Id: I18819acb556a2a7f1b6da6994db9783c48108934
Reviewed-by: Michael Meffie <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

10 months agoafs: Move osi_GetTime out of param.h 37/14237/3
Andrew Deason [Tue, 2 Jun 2020 18:12:14 +0000]
afs: Move osi_GetTime out of param.h

Most platforms currently #define osi_GetTime in their param.h. This is
really redundant, since the definition of osi_GetTime almost never
changes for a given platform, so we end up with many copies of the
same osi_GetTime definition for a given platform.

Move osi_GetTime out of param.h for these platforms, and define it in
osi_machdep.h instead, which is where most platform-specific
definitions go.

For DFBSD, we don't have an osi_machdep.h at all yet, so create a new
one to contain the osi_GetTime definition. Currently we don't build
libafs at all on DFBSD, but do this anyway so we don't lose the
existing osi_GetTime definition.

For NBSD, we were providing (conflicting!) definitions for osi_GetTime
in param.h and in osi_machdep.h. Just remove the definitions in
param.h, since those should have been getting overridden by the
osi_machdep.h definition.

Change-Id: I7097d9fe2fcd38c06ecc275e8fe3a2c69c9d0436
Tested-by: BuildBot <>
Reviewed-by: Michael Meffie <>
Reviewed-by: Benjamin Kaduk <>

10 months agoafs: Avoid using logical OR when setting f_fsid 92/14292/4
Cheyenne Wills [Mon, 27 Jul 2020 18:31:35 +0000]
afs: Avoid using logical OR when setting f_fsid

Building with clang-10 produces the warning/error message
    warning: converting the result of '<<' to a boolean always evaluates
    to true [-Wtautological-constant-compare]
for the expression
    abp->f_fsid = (AFS_VFSMAGIC << 16) || AFS_VFSFSID;

The message is because a logical OR '||' is used instead of a bitwise
OR '|'.  The result of this expression will always set the f_fsid
member to a 1 and not the intended value of AFS_VFSMAGIC combined with

Update the expression to use a bitwise OR instead of the logical OR.

Note: This will change value stored in the f_fsid that is returned from

Using a logical OR has existed since OpenAFS 1.0 for hpux/solaris and in
UKERNEL since OpenAFS 1.5 with the commit 'UKERNEL: add uafs_statvfs'

Change-Id: I3e85ba48058ac68e3e3ac7f277623f660187926c
Reviewed-by: Andrew Deason <>
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

10 months agoafs: Set AFS_VFSFSID to a numerical value 79/14279/9
Cheyenne Wills [Mon, 27 Jul 2020 18:31:03 +0000]
afs: Set AFS_VFSFSID to a numerical value

Currently when UKERNEL is defined, AFS_VFSFSID is always set to
AFS_MOUNT_AFS, which is a string for many platforms for UKERNEL.

Update src/afs/afs.h to insure that the define for AFS_VFSFSID is a
numeric value when building UKERNEL.

Clean up the preprocessor indentation in src/afs/afs.h in the area
around the AFS_VFSFSID defines.

Thanks to for pointing out a much easier solution
for resolving this problem.

Change-Id: I618fc4c89029a6cca2ca6f530b8f65399299a9d1
Reviewed-by: Andrew Deason <>
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

10 months agoclang-10: ignore fallthrough warning in generated code 75/14275/9
Cheyenne Wills [Thu, 23 Jul 2020 21:43:42 +0000]
clang-10: ignore fallthrough warning in generated code

Clang-10 will not recognize '/* fall through */' as an indicator to
turn off the fallthrough warning due to the lack of a 'break' in a case

Code generated by flex uses the '/* fall through */' comments to turn
off compiler warnings for fallthroughs in case statements.

For code generated by flex, ignore the implicit-fallthrough via pragma
or disable the warning via a compile time flag.

Add new env variable "CFLAGS_NOIMPLICIT_FALLTHROUGH" to selectively
disable the compile check in Makefiles when checking is enabled.

Change-Id: I4c054defda03daa2aeb645ae2271dfa0cb54925f
Reviewed-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

10 months agoclang-10: use AFS_FALLTHROUGH for case fallthrough 74/14274/9
Cheyenne Wills [Mon, 27 Jul 2020 14:33:03 +0000]
clang-10: use AFS_FALLTHROUGH for case fallthrough

Clang-10 will not recognize '/* fallthrough */' as an indicator to
turn off the fallthrough diagnostic due to the lack of a 'break' in a
case statement.  Clang-10 requires the '__attribute__((fallthrough))'
statement to disable the diagnostic.

In addition clang-10 is finding additional locations where fall throughs

Determine if the compiler supports '__attribute__((fallthrough))' to
disable the implicit fallthrough diagnostic.

Define a new macro 'AFS_FALLTHROUGH' that will disable the fallthrough
diagnostic. Set it as a wrapper for the Linux kernel's 'fallthrough'
macro if available, otherwise set it as a wrapper macro for
'__attribute__((fallthrough))' if the compiler supports it.

Update CODING to document the use of AFS_FALLTHROUGH when needing to
fallthrough between case statements.

Replace the '/* fallthrough */' comments with AFS_FALLTHROUGH, and add

Replace some fallthroughs with a break (or goto) if the flow was was
just to a break (or goto).

e.g.   case x:                 case x:
           somestmt;               somestmt;
       case y:                 case y:
           break;                  break;

Correct a mis-indented brace '}' in src/WINNT/afsd/smb3.c

Note, the clang maintainers have rejected the use of comments as a flag
to turn off the fall through warnings.

Change-Id: Ia5da10fc14fc1874baca035a3cf471e618e0d5f5
Tested-by: BuildBot <>
Reviewed-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>

10 months agoredhat: Add make to the dkms-openafs pre-requirements 66/14266/3
Michael Meffie [Thu, 2 Jul 2020 01:50:09 +0000]
redhat: Add make to the dkms-openafs pre-requirements

If `make` is not installed before dkms-openafs, the OpenAFS kernel
module is not built during the dkms-openafs package installation.

The failure happens in the "checking if linux kernel module build works"
configure step, which invokes `make` to check the linux buildsystem.
configure fails when `make` is not available, and gives the unhelpful
suggestion (in this case) of configuring with --disable-kernel module.

Running the configure.log in the dkms build directory shows:

    configure:7739: checking if linux kernel module build works
    make -C /lib/modules/4.18.0-193.6.3.el8_2.x86_64/build M=/var/lib/dkms/openafs/...
    ./configure: line 7771: make: command not found
    configure: failed using Makefile:

Avoid this build failure by adding `make` to the list of dkms-openafs
package pre-requirements.

Change-Id: I98b3508341eea1df4fa7b6f43e88add1bda9ee2c
Tested-by: BuildBot <>
Reviewed-by: Cheyenne Wills <>
Reviewed-by: Benjamin Kaduk <>

10 months agovol: Blank opts in VOptDefaults 80/14280/2
Andrew Deason [Fri, 29 May 2020 17:57:50 +0000]
vol: Blank opts in VOptDefaults

Instead of needing to set every single field in the 'opts' structure
individually, blank the whole thing to make sure the entire struct is
initialized. Remove the now-redundant lines that initialize various
items to 0.

Change-Id: I799cdb55becd66a8f3d6ec2f81338843038d0abd
Tested-by: BuildBot <>
Reviewed-by: Michael Meffie <>
Reviewed-by: Kailas Zadbuke <>
Reviewed-by: Yadavendra Yadav <>
Reviewed-by: Benjamin Kaduk <>

10 months agovolser: Don't NUL-pad failed pread()s in dumps 55/14255/3
Andrew Deason [Tue, 23 Jun 2020 03:54:52 +0000]
volser: Don't NUL-pad failed pread()s in dumps

Currently, the volserver SAFSVolDump RPC and the 'voldump' utility
handle short reads from pread() for vnode payloads by padding the
missing data with NUL bytes. That is, if we request 4k of data for our
pread() call, and we only get back 1k of data, we'll write 1k of data
to the volume dump stream followed by 3k of NUL bytes, and log
messages like this:

    1 Volser: DumpFile: Error reading inode 1234 for vnode 5678
    1 Volser: DumpFile: Null padding file: 3072 bytes at offset 40960

This can happen if we hit EOF on the underlying file sooner than
expected, or if the OS just responds with fewer bytes than requested
for any reason.

The same code path tries to do the same NUL-padding if pread() returns
an error (for example, EIO), padding the entire e.g. 4k block with
NULs. However, in this case, the "padding" code often doesn't work as
intended, because we compare 'n' (set to -1) with 'howMany' (set to 4k
in this example), like so:

    if (n < howMany)

Here, 'n' is signed (ssize_t), and 'howMany' is unsigned (size_t), and
so compilers will promote 'n' to the unsigned type, causing this
conditional to fail when n is -1. As a result, all of the relevant log
messages are skipped, and the data in the dumpstream gets corrupted
(we skip a block of data, and our 'howFar' offset goes back by 1). So
this can result in rare silent data corruption in volume dumps, which
can occur during volume releases, moves, etc.

To fix all of this, remove this bizarre NUL-padding behavior in the
volserver. Instead:

- For actual errors from pread(), return an error, like we do for I/O
  errors in most other code paths.

- For short reads, just write out the amount of data we actually read,
  and keep going.

- For premature EOF, treat it like a pread() error, but log a slightly
  different message.

For the 'voldump' utility, the padding behavior can make sense if a
user is trying to recover volume data offline in a disaster recovery
scenario. So for voldump, add a new switch (-pad-errors) to enable the
padding behavior, but change the default behavior to bail out on

Change-Id: Ibd6e76c5ea0dea95e3354d9b34536296f81b4f67
Tested-by: BuildBot <>
Reviewed-by: Cheyenne Wills <>
Reviewed-by: Benjamin Kaduk <>

10 months agobutc: fix int to float conversion warning 77/14277/6
Cheyenne Wills [Thu, 16 Jul 2020 21:52:00 +0000]
butc: fix int to float conversion warning

Building with clang-10 results in 2 warnings/errors associated with
with trying to convert 0x7fffffff to a floating point value.

    tcmain.c:240:18: error: implicit conversion from 'int' to 'float'
                 changes value from 2147483647 to 2147483648 [-Werror,
    if ((total > 0x7fffffff) || (total < 0))    /* Don't go over 2G */

and the same conversion warning on the statement on the following line:
    total = 0x7fffffff;

Use floating point and decimal constants instead of the hex constants.

For the test, use 2147483648.0 which is cleanly represented by a float.
Change the comparison in the test from '>' to '>='.

If the total value exceeds 2G, just assign the max value directly to the
return variable.

Change-Id: I79b2afa006496a756bd7b50976050c24827aa027
Reviewed-by: Andrew Deason <>
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

10 months agoautoconf: fix detection for fallthrough attribute 73/14273/5
Cheyenne Wills [Thu, 16 Jul 2020 21:07:15 +0000]
autoconf: fix detection for fallthrough attribute

Due to bug <>,
ax_gcc_func_attribute.m4 fails to properly detect __attribute__((fallthrough))
in clang. Until this is fixed in autoconf-archive upstream, fix our
local copy of ax_gcc_func_attribute.m4, so we can detect
__attribute__((fallthrough)) to make --enable-checking work with clang.

Change-Id: I80a4557384f8e1438344e48bfe722e20c8773882
Reviewed-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

10 months agocf: Make local copy of ax_gcc_func_attribute.m4 72/14272/4
Cheyenne Wills [Thu, 16 Jul 2020 21:05:13 +0000]
cf: Make local copy of ax_gcc_func_attribute.m4

Make a local copy of ax_gcc_func_attribute from autoconf-archive. This
is needed in order to fix a bug in the detection of the fallthrough

Remove ax_gcc_func_attribute.m4 from src/external/autoconf-archive/m4.
Update LICENSE file to point to the local copy in src/cf.

Change-Id: I6c4244d2cd4edab4262c1820435c00419d85303b
Tested-by: BuildBot <>
Reviewed-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>

10 months agorx: prevent leakage of non-cached rx_connections (pthread) 42/13042/5
Mark Vitale [Fri, 20 Apr 2018 04:57:28 +0000]
rx: prevent leakage of non-cached rx_connections (pthread)

The rxi_connectionCache (AFS_PTHREAD_ENV only) allows applications to
reuse rx_connection structs.  Cached rx_connections are obtained via
rx_GetCachedConnection and released via rx_ReleaseCachedConnection.
This feature is used most heavily by libadmin and kauth, but there are
other users in the tree as well.

For instance, ubikclient routines ubik_ClientInit and ubik_ClientDestroy
call rx_ReleaseCachedConnections (if AFS_PTHREAD_ENV) when disposing of
their rx_connections.  Unfortunately, in many cases these rx_connections
were obtained via rx_NewConnection, _not_ from the cache via
rx_GetCachedConnection.  In those cases, rx_ReleaseCachedConnection will
not find the rx_connection in the rxi_connectionCache, and thus it
returns without doing anything.

Therefore, when ubik_ClientInit is passed an existing ubik_client (for
re-initialization) that contains rx_connections NOT allocated via
rx_GetCachedConnection, those connections are not destroyed, but will be
silently leaked.  Similarly, ubik_ClientDestroy will leak its
rx_connections when it frees the ubik_client struct.

For example, the fileserver host package calls ubik_ClientInit (via
hpr_Initialize) and ubik_ClientDestroy (via hpr_End) to manage
connections to the ptserver.  However, these connections were obtained
via rx_NewConnection, not rx_GetCachedConnection.  If the fileserver has
a failed call to the ptserver that sets prfail=1, the next RPC scheduled
for that client (in CallPreamble) will refresh the thread's ubik_client
(viced_uclient_key) by calling hprEnd -> ubik_ClientDestroy ->
rx_ReleaseCachedConnection.  The "released" connections will be leaked.

This problem exists in all versions of OpenAFS going back to IBM 1.0.
Starting with 1.8.x, many components that were formerly LWP-only are now
pthreaded and thus susceptible to this leak.

It seems difficult and error-prone to identify all possible code paths
that may pass a non-cached rx_connection to rx_ReleaseCachedConnection,
and convert them to obtain connections via rx_GetCachedConnection.

Instead, prevent all existing and future leaks by modifying the connection
cache to:
- flag all rx_connections it allocates
- correctly release any rx_connection it is passed, whether they came
  from the cache or not.

Change-Id: Ibe164ccd30a8ddd799438c28fd6e1d8a0a9040dd
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

10 months agorx: fix out-of-range value for RX_CONN_NAT_PING 41/13041/4
Mark Vitale [Mon, 30 Apr 2018 22:34:28 +0000]
rx: fix out-of-range value for RX_CONN_NAT_PING

Commit 496fb87372555f6acddd4fd88b03c94c85f48511 ("rx: avoid nat ping
until connection is attached") introduced functionality to defer turning
on NAT ping for server connections until after reachability had been
established for the client.

Unfortunately, this feature could never work correctly because it
assigned an out-of-range flag value of 256 (0x100) for the u_char flags
field. Instead of calling this out as an error, both gcc and Solaris cc
elide this flag so that it is never set in
rx_SetConnSecondsUntilNatPing(), Furthermore, the test in
rxi_ConnClearAttachWait() will always fail; therefore
rxi_ScheduleNatKeepAliveEvent is never called after attach wait has

Fortunately, this bug is currently moot - not actually exposed in
OpenAFS. (It was discovered by inspection). This is because there are
currently no rx_connection objects in the tree that have both NAT ping
and checkReach (rx_SetCheckReach) enabled. I also searched git history
and found no time when this bug could ever have been exposed. This does
raise the question of why the original commit was needed; but instead of
reverting the original commit, this commit attempts to fix it.

To prevent problems if NAT ping and checkReach are ever both enabled for
an rx_connection, enlarge the rx_connection flags member so that the
RX_CONN_NAT_PING value is no longer out of range.

Change-Id: Ib667ece632f66fa5c63a76398acb3153fed6f9c3
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

11 months agoauth: Avoid cellconfig.c stdio renaming 14/14214/3
Andrew Deason [Mon, 18 May 2020 17:38:31 +0000]
auth: Avoid cellconfig.c stdio renaming

Since commit 35777145 (solaris-fopen-sucks-20060916), cellconfig.c has
redirected fopen, fclose, and fgets to local functions on
non-64bit-sparc Solaris, in order to work around that platform's stdio

Commit 7c431f7571 (auth: retire writeconfig.c) moved the contents of
writeconfig.c into cellconfig.c. The previous writeconfig.c contained
some calls to stdio, including calling fprintf() on a pointer returned
by fopen() in that file.

Because fopen() was redirected to our local version, this means that
afsconf_SetExtendedCellInfo() calls fopen() to get an
afsconf_iobuffer*, and passes that pointer to the real system
fprintf() later on (instead of a native FILE*). The compiler does warn
about this, but this only happens on Solaris, where --enable-checking
is not implemented, so the build never fails.

To avoid this, remove the #defines for fopen, fgets, and fclose.
Instead, change all of the old cellconfig.c callers to explicitly call
afsconf_fopen, afsconf_fgets, and afsconf_fclose. On the affected
Solaris platforms, we keep our local definitions, and for other
platforms, we just make those functions call their system stdio
equivalents. For the code that was pulled in from writeconfig.c,
callers will just call the system fopen, fprintf, and fclose.

We still keep our local afsconf_FILE* definition on all platforms, so
the compiler will still do typechecking for our local afsconf_f*
functions on all platforms. So now if we make a mistake, it should be
a mistake on all platforms, so platforms with --enable-checking should
flag the error.

Change-Id: I4064d7f5ee82d5acab04a33b01c0603564a391e8
Tested-by: BuildBot <>
Reviewed-by: Michael Meffie <>
Reviewed-by: Mark Vitale <>
Tested-by: Mark Vitale <>
Reviewed-by: Benjamin Kaduk <>

11 months agoafs: Let afs_ShakeLooseVCaches run longer 54/14254/3
Andrew Deason [Fri, 26 Jul 2019 20:28:44 +0000]
afs: Let afs_ShakeLooseVCaches run longer

Currently, when afs_ShakeLooseVCaches runs osi_TryEvictVCache, we
check if osi_TryEvictVCache slept (i.e. dropped afs_xvcache/GLOCK). If
we sleep over 100 times, then we stop trying to evict vcaches and

If we have recently accessed a lot of AFS files, this limitation can
severely reduce our ability to keep our number of vcaches limited to a
reasonable size. For example:

Say a Linux client runs a process that quickly accesses 1 million
files (a simple 'find' command) and then does nothing else. A few
minutes later, afs_ShakeLooseVCaches is run, but since all of the
newly accessed vcaches have dentries attached to them, we will sleep
on each one in order to try to prune the attached dentries. This means
that afs_ShakeLooseVCaches will evict 100 vcaches, and then return,
leaving us with still almost 1 million vcaches. This will happen
repeatedly until afs_ShakeLooseVCaches finally works its way through
all of the vcaches (which takes quite a while, if we only clear 100 at
once), or the dentries get pruned by other means (such as, if Linux
evicts them due to memory pressure).

The limit of 100 sleeps was originally added in commit 29277d96
(newvcache-dont-spin-20060128), but the current effect of it was
largely introduced in commit 9be76c0d (Refactor afs_NewVCache). It
exists to ensure that afs_ShakeLooseVCaches doesn't take forever to
run, but the limit of 100 sleeps may seem quite low, especially if
those 100 sleeps run very quickly.

To avoid the situation described above, instead of limiting
afs_ShakeLooseVCaches based on a fixed number of sleeps, limit it
based on how long we've been running, and set an arbitrary limit of
roughly 3 seconds. Only check how long we've been running after 100
sleeps like before, so we're not constantly checking the time while

Log a new warning if we exit afs_ShakeLooseVCaches prematurely if
we've been running for too long, to help indicate what is going on.

Change-Id: I65729ace748e8507cc0d5c26dec39e74d7bff5d2
Reviewed-by: Cheyenne Wills <>
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

11 months agoafs: Skip bulkstat if stat cache looks full 56/13256/4
Andrew Deason [Mon, 16 Jul 2018 21:53:34 +0000]
afs: Skip bulkstat if stat cache looks full

Currently, afs_lookup() will try to prefetch dir entries for normal
dirs via bulkstat whenever multiple pids are reading that dir.
However, if we already have a lot of vcaches, ShakeLooseVCaches may be
struggling to limit the vcaches we already have. Entering
afs_DoBulkStat can make this worse, since we grab afs_xvcache
repeatedly, we may kick out other vcaches, and we'll possibly create
30 new vcaches that may not even be used before they're evicted.

To try to avoid this, skip running afs_DoBulkStat if it looks like the
stat cache is really full.

Change-Id: I1634530170a189f32cb962dd7df28f88bc758b71
Reviewed-by: Cheyenne Wills <>
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

11 months agoafs: Log warning when we detect too many vcaches 55/13255/4
Andrew Deason [Mon, 16 Jul 2018 21:44:14 +0000]
afs: Log warning when we detect too many vcaches

Currently, afs_ShakeLooseVCaches has a kind of warning that is logged
when we fail to free up any vcaches. This information can be useful to
know, since it may be a sign that users are trying to access way more
files than our configured vcache limit, hindering performance as we
constantly try to evict and re-create vcaches for files.

However, the current warning is not clear at all to non-expert users,
and it can only occur for non-dynamic vcaches (which is uncommon these

To improve this, try to make a general determination if it looks like
the stat cache is "stressed", and log a message if so after
afs_ShakeLooseVCaches runs (for all platforms, regardless of dynamic
vcaches). Also try to make the message a little more user-friendly,
and only log it (at most) once per 4 hours.

Determining whether the stat cache looks stressed or not is difficult
and arguably subjective (especially for dynamic vcaches). This commit
draws a few arbitrary lines in the sand to make the decision, so at
least something will be logged in the cases where users are constantly
accessing way more files than our configured vcache limit.

Change-Id: I022478dc8abb7fdef24ccc06d477b349cca759ac
Reviewed-by: Cheyenne Wills <>
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

11 months agoviced: propagate return from CleanupTimedOutCallBacks_r 56/14256/2
Mark Vitale [Thu, 25 Jun 2020 15:45:19 +0000]
viced: propagate return from CleanupTimedOutCallBacks_r

The fileserver's FiveMinuteCheckLWP periodically calls
CleanupTimedOutCallBacks, and logs an informational messages if the
return code indicates that any callbacks were discarded.

However, since the original IBM code import,  CleanupTimedOutCallBacks
has 1) ignored the return value from CleanupTimedOutCallBacks_r and 2)
unconditionally returned 0.  This makes the informational message
essentially dead code.

Instead, check the code from CleanupTimedOutCallBacks_r and pass it back
to the caller.

Change-Id: I631831c398e43431b79f4a3a0c6f01307ac0c05e
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

11 months agoLINUX: Close cacheFp if no ->readpage in fastpath 52/14252/5
Andrew Deason [Fri, 19 Jun 2020 02:16:09 +0000]
LINUX: Close cacheFp if no ->readpage in fastpath

In afs_linux_readpage_fastpath, if we discover that our disk cache fs
has no ->readpage function, we'll 'goto out', but we never close our
cacheFp. To make sure we close it, add a filp_close() call to the
'goto out' cleanup code.

Change-Id: I371c1d7ec51b03447fbcbe58fb89be7be0235022
Reviewed-by: Cheyenne Wills <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

11 months agoLINUX: Don't panic on some file open errors 42/14242/9
Cheyenne Wills [Thu, 2 Jul 2020 19:39:27 +0000]
LINUX: Don't panic on some file open errors

Commit 'LINUX: Return NULL for afs_linux_raw_open error' (f6af4a155)
updated afs_linux_raw_open to return NULL on some errors, but still
panics if obtaining the dentry fails.

Commit 'afs: Verify osi_UFSOpen worked' (c6b61a451) updated callers of
osi_UFSOpen to verify whether or not the open was successful.  This
meant osi_UFSOpen (and routines it calls) could pass back an error
indication rather than panic when an error is encountered.

Update afs_linux_raw_open to return a failure instead of panic if unable
to obtain a dentry.

Update osi_UFSOpen to return a NULL instead of panic if unable to obtain
memory or fails to open the file. All callers of osi_UFSOpen handle a
fail return, though some will still issue a panic.

Update afs_linux_readpage_fastpath and afs_linux_readpages to not panic
if afs_linux_raw_open fails.  Instead of panic, return an error.

For testing, an error can be forced by removing a file from the
cache directory.

Note this work is based on a commit by

Change-Id: Ic47e4868b4f81d99fbe3b2e4958778508ae4851f
Reviewed-by: Andrew Deason <>
Reviewed-by: Mark Vitale <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

11 months agoafs: Avoid panics on failed return from afs_CFileOpen 41/14241/8
Cheyenne Wills [Fri, 19 Jun 2020 14:01:14 +0000]
afs: Avoid panics on failed return from afs_CFileOpen

afs_CFileOpen is a macro that invokes the open "method" of the
afs_cacheOps structure, and for disk caches the osi_UFSOpen function is

Currently osi_UFSOpen will panic if there is an error encountered while
opening a file.

Prepare to handle osi_UFSOpen function returning a NULL instead of
issuing a panic (future commit).

Update callers of afs_CFileOpen to test for an error and to return an
error instead of issuing a panic.

While this commit eliminates some panics, it does not address some of the
more complex cases associated with errors from afs_CFileOpen.

Change-Id: I2bdd525633dd44ebf8e26fcfd7059dfdfffb6142
Reviewed-by: Andrew Deason <>
Reviewed-by: Mark Vitale <>
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

11 months agoLINUX 5.8: use lru_cache_add 49/14249/8
Cheyenne Wills [Thu, 25 Jun 2020 16:43:53 +0000]
LINUX 5.8: use lru_cache_add

With Linux-5.8-rc1 commit 'mm: fold and remove lru_cache_add_anon() and
lru_cache_add_file()' (6058eaec), the lru_cache_add_file function is
removed since it was functionally equivalent to lru_cache_add.

Replace lru_cache_add_file with lru_cache_add.

Introduce a new autoconf test to determine if lru_cache_add is present

For reference, the Linux changes associated with the lru caches:

  __pagevec_lru_add introduced before v2.6.12-rc2

  lru_cache_add_file introduced in v2.6.28-rc1
  __pagevec_lru_add_file replaces __pagevec_lru_add in v2.6.28-rc1
     vmscan: split LRU lists into anon & file sets (4f98a2fee)

  __pagevec_lru_add removed in v5.7 with a note to use lru_cache_add_file
     mm/swap.c: not necessary to export __pagevec_lru_add() (bde07cfc6)

  lru_cache_add_file removed in v5.8
     mm: fold and remove lru_cache_add_anon() and lru_cache_add_file()
  lru_cache_add exported
     mm: fold and remove lru_cache_add_anon() and lru_cache_add_file()

Openafs will use:
  lru_cache_add on 5.8 kernels
  lru_cache_add_file from 2.6.28 through 5.7 kernels
  __pagevec_lru_add/__pagevec_lru_add_file on pre 2.6.28 kernels

Change-Id: I79ebe4a81425bf8a8a327ddf2d3474aff9df039d
Tested-by: BuildBot <>
Reviewed-by: Andrew Deason <>
Reviewed-by: Yadavendra Yadav <>
Reviewed-by: Benjamin Kaduk <>

11 months agoRecode a couple files from ISO 8859-1 to UTF-8 65/14265/2
Benjamin Kaduk [Wed, 1 Jul 2020 04:55:45 +0000]
Recode a couple files from ISO 8859-1 to UTF-8

Reported by Debian's lintian(1).
The CellServDB, as an externally maintained file, is left unchanged.

Change-Id: I3bf241b924cb8cd7799a4c3e799f6acd375b2e8a
Tested-by: BuildBot <>
Reviewed-by: Michael Meffie <>
Reviewed-by: Benjamin Kaduk <>

11 months agoafs: Bound afs_DoBulkStat dir scan 53/13253/3
Andrew Deason [Sun, 8 Jul 2018 20:00:02 +0000]
afs: Bound afs_DoBulkStat dir scan

Currently, afs_DoBulkStat will scan the entire directory blob, looking
for entries to stat. If all or almost all entries are already stat'd,
we'll scan through the entire directory, doing nontrivial work on
each entry (we grab afs_xvcache, at least). All of this work is pretty
pointless, since the entries are already cached and so we won't do
anything. If many processes are trying to acquire afs_xvcache, this
can contribute to performance issues.

To avoid this, provide a constant bound on the number of entries we'll
search through: nentries * 4. The current arbitrary limits cap
nentries at 30, so this means we're capping the afs_DoBulkStat search
to 120 entries.

Change-Id: I66e9af5b27844ddf6cf37c8286fcc65f8e0d3f96
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

11 months agoafs: Avoid needless W-locks for afs_FindVCache 56/12656/4
Andrew Deason [Thu, 13 Jul 2017 22:40:36 +0000]
afs: Avoid needless W-locks for afs_FindVCache

The callers of afs_FindVCache must hold at least a read lock on
afs_xvcache; some hold a shared or write lock (and set IS_SLOCK or
IS_WLOCK in the given flags). Two callers (afs_EvalFakeStat_int and
afs_DoBulkStat) currently hold a write lock, but neither of them need

In the optimal case, where afs_FindVCache finds the given vcache, this
means that we unnecessarily hold a write lock on afs_xvcache. This can
impact performance, since afs_xvcache can be a very frequently
accessed lock (a simple operation like afs_PutVCache briefly holds a
read lock, for example).

To avoid this, have afs_DoBulkStat hold a shared lock on afs_xvcache,
upgrading to a write lock when needed. afs_EvalFakeStat_int doesn't
ever need a write lock at all, so just convert it to a read lock.

Change-Id: I5bd58b9e3a577c9e1ebf1bc3719e65a6c0af5cb8
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

11 months agoutil: Handle serverLogMutex lock across forks 39/14239/8
Kailas Zadbuke [Wed, 3 Jun 2020 10:14:08 +0000]
util: Handle serverLogMutex lock across forks

If a process forks when another thread has serverLogMutex locked, the
child process inherits the locked serverLogMutex. This causes a deadlock
when code in the child process tries to lock serverLogMutex, since we
can never unlock serverLogMutex because the locking thread no longer
exists. This can happen in the salvageserver, since the salvageserver
locks serverLogMutex in different threads, and forks to handle salvage

To avoid this deadlock, we register handlers using pthread_atfork()
so that the serverLogMutex will be held during the fork. The fork will
be blocked until the worker thread releases the serverLogMutex. Hence the
serverLogMutex will be held until the fork is complete and it will be
released in the parent and child threads.

Thanks to Yadavendra Yadav( for working with me
on this issue.

Change-Id: I191c8272825c1667bb2150146e04b1dfe36a54e4
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

11 months agoafs: Split out bulkstat conditions into a function 54/13254/3
Andrew Deason [Mon, 16 Jul 2018 21:08:13 +0000]
afs: Split out bulkstat conditions into a function

Our current if() statement for determining whether we should run
afs_DoBulkStat to prefetch dir entries is a bit large, and grows over
time. Split this logic out into a separate function to make it easier
to maintain, and add some comments to help explain each condition.

This commit should have no visible effects; it's just code

Change-Id: I0086189308d2f5e4b321c63f24110d74cda6433c
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

11 months agoafs: Change VerifyVCache2 calls to VerifyVCache 55/12655/4
Andrew Deason [Thu, 13 Jul 2017 22:40:21 +0000]
afs: Change VerifyVCache2 calls to VerifyVCache

afs_VerifyVCache is a macro that (on most platforms) effectively
expands to:

    if ((avc->f.states & CStatd)) {
        return 0;
    } else {
        return afs_VerifyVCache2(...);

Some callers call afs_VerifyVCache2 directly, since they already check
for CStatd for other reasons. A few callers currently call
afs_VerifyVCache2, but without guaranteeing that CStatd is not set.
Specifically, in afs_getattr and afs_linux_VerifyVCache, CStatd could
be set while afs_CreateReq drops GLOCK. And in afs_linux_readdir,
CStatd could be cleared at multiple different points before the
VerifyVCache call.

This can result in afs_VerifyVCache2 acquiring a write-lock on the
vcache, even when CStatd is already set, which is an unnecessary
performance hit.

To avoid this, change these call sites to use afs_VerifyVCache instead
of calling afs_VerifyVCache2 directly, which skips the write lock when
CStatd is already set.

Change-Id: I7b75c9755af147b42a48160fa90c9849f2f03ddb
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

11 months agoLINUX: replace BUG() call with osi_Panic() in osi_linux_free 50/14250/2
Mark Vitale [Thu, 18 Jun 2020 17:43:35 +0000]
LINUX: replace BUG() call with osi_Panic() in osi_linux_free

If osi_linux_free fails, it printf's an error message, then calls BUG().
This is the sole open-coded call to BUG() in OpenAFS; all other calls
to BUG() are indirect via osi_Panic().

For consistency, eliminate this direct BUG() call by replacing the
printf and BUG() with an equivalent osi_Panic().  This also ensures that
the error messsage is logged as critical, and prefixed with "openafs:".

Change-Id: Id319dffa859308528a66991bbbc522ca49552d51
Reviewed-by: Benjamin Kaduk <>
Tested-by: Benjamin Kaduk <>

11 months agoLINUX 5.8: do not set name field in backing_dev_info 48/14248/4
Cheyenne Wills [Wed, 17 Jun 2020 00:35:46 +0000]
LINUX 5.8: do not set name field in backing_dev_info

Linux-5.8-rc1 commit 'bdi: remove the name field in struct
backing_dev_info' (1cd925d5838)

Do not set the name field in the backing_dev_info structure if it is
not available. Uses an existing config test

Note the name field in the backing_dev_info structure was added in

Change-Id: I20b80e49e8a15a2949003101f24d9ce39f63b59b
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

11 months agoLINUX 5.8: Replace kernel_setsockopt with new funcs 47/14247/4
Cheyenne Wills [Thu, 18 Jun 2020 22:39:22 +0000]
LINUX 5.8: Replace kernel_setsockopt with new funcs

Linux 5.8-rc1 commit 'net: remove kernel_setsockopt' (5a892ff2facb)
retires the kernel_setsockopt function. In prior kernel commits new
functions (ip_sock_set_*) were added to replace the specific functions
performed by kernel_setsockopt.

Define new config test 'HAVE_IP_SOCK_SET' if the 'ip_sock_set' functions
are available. The config define 'HAVE_KERNEL_SETSOCKOPT' is no longer
set in Linux 5.8.

Create wrapper functions that replace the kernel_setsockopt calls with
calls to the appropriate Linux kernel function(s) (depending on what
functions the kernel supports).

Remove the unused 'kernel_getsockopt' function (used for building with
pre 2.6.19 kernels).

For reference
    Linux 2.6.19 introduced kernel_setsockopt
    Linux 5.8 removed kernel_setsockopt and replaced the functionality
              with a set of new functions (ip_sock_set_*)

Change-Id: I517b674303c5decc19313d9de51d04ddef36b421
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

11 months agotests: Modernize writekeyfile.c 46/14246/4
Andrew Deason [Wed, 17 Jun 2020 17:23:46 +0000]
tests: Modernize writekeyfile.c

tests/auth/writekeyfile.c contains some code used to generate
tests/auth/KeyFile, which is used to test code interpreting the
old-style KeyFile format. This code currently has a few problems:

- We don't check the results of afstest_mkdtemp, which could allow
  symlink attacks from other users on the system.

- We duplicate some logic from afstest_BuildTestConfig, in order to
  build a temporary config dir.

- writekeyfile isn't built or run by default (it only exists to
  generate KeyFile, so it's almost never run), so eventual bitrot is
  quite likely, and the existing code already generates warnings.

To avoid this, change writekeyfile.c to use the existing
afstest_BuildTestConfig to generate a local config dir. To ensure we
avoid bitrot, build writekeyfile by default, and create a test to run
it, to make sure it can generate a KeyFile as expected.

Note that the KeyFile.short we test against is different than the
KeyFile currently in the tree. The existing KeyFile was generated from
an older OpenAFS release, which always generated 100-byte KeyFiles,
even if we only have a few keys. The current codebase only writes out
as much key data as needed, so the generated KeyFiles are shorter (but
still understandable by older OpenAFS releases).

Keep the old 100-byte KeyFile around, since that's what older OpenAFS
would generate, and create a new KeyFile.short to test against, to
make sure our code for generating KeyFiles doesn't change any further.

Change-Id: Ibe9246c6dd808ed2b2225dd7be2b27bbdee072fd
Reviewed-by: Cheyenne Wills <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

11 months agotests: Use usleep instead of nanosleep 44/14244/5
Cheyenne Wills [Tue, 16 Jun 2020 21:20:20 +0000]
tests: Use usleep instead of nanosleep

Commit "Build tests by default" 68f406436cc21853ff854c514353e7eb607cb6cb
changes the build so tests are always built.

On Solaris 10 the build fails because nanosleep is in librt, which we do
not link against.

Replace nanosleep with usleep.  This avoids introducing extra configure
tests just for Solaris 10.

Note that with Solaris 11 nanosleep was moved from librt to libc, the
standard C library.

Change-Id: I6639f32bb8c8ace438e0092a866f06561dad54f1
Reviewed-by: Andrew Deason <>
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

11 months agotests: Emulate mkdtemp when not available 43/14243/8
Cheyenne Wills [Wed, 17 Jun 2020 19:08:18 +0000]
tests: Emulate mkdtemp when not available

Commit "Build tests by default" 68f406436cc21853ff854c514353e7eb607cb6cb
changes the build so tests are always built.

On Solaris 10 Update 10 and earlier the build fails because the mkdtemp
function is not available.

Introduce a wrapper 'afstest_mkdtemp' that uses mkdtemp if available,
otherwise uses mktemp/mkdir.

Change-Id: I0118f838ed9a89927e2ddac4cad822574601558a
Reviewed-by: Andrew Deason <>
Tested-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>

11 months agomake-release: Run git describe once 50/14150/3
Michael Meffie [Thu, 16 Apr 2020 13:41:41 +0000]
make-release: Run git describe once

Run git describe once at the beginning of make-release to find the
version information used to derive the tarball file names and saved in
the .version file.

This is a cleanup and refactoring change to prepare for a future commit.

Change-Id: I0debeeffa5d2c63ab1498588766cb36424d15cd5
Reviewed-by: Cheyenne Wills <>
Tested-by: BuildBot <>
Reviewed-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>

11 months agomake-release: Create output directory if needed 15/14115/5
Michael Meffie [Fri, 27 Mar 2020 15:29:24 +0000]
make-release: Create output directory if needed

Automatically create the --dir directory if it does not already exist,
which makes this script slightly easier to use. Remove the now
uneeded mkdir from the top-level makefile.

Change-Id: I1f4561120a70263b0b2b194e65fec55fb5666f40
Reviewed-by: Cheyenne Wills <>
Tested-by: BuildBot <>
Reviewed-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>

12 months agomake-release: Remove unused optional version argument 49/14149/4
Michael Meffie [Thu, 16 Apr 2020 11:21:51 +0000]
make-release: Remove unused optional version argument

The make-release help shows an optional version argument, but in fact
the version info is always generated from the git tag name argument,
which makes sense when creating releases.

Continue to throw away the second positional argument just in case
someone is still passing a second argument, but issue a warning if they

Change-Id: Ie4c6e6efb7693e53a02fd009eecd64b47250c848
Reviewed-by: Cheyenne Wills <>
Reviewed-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: Benjamin Kaduk <>

12 months agomake-release: Clean up whitespace and spelling 48/14148/3
Michael Meffie [Thu, 16 Apr 2020 11:37:39 +0000]
make-release: Clean up whitespace and spelling

Fix whitespace errors, convert tabs to spaces, fix spelling errors, and
fix pod markup in the make-release script.

Change-Id: I24ede59d44a8818d89de454c0935586fccbd5d9a
Reviewed-by: Cheyenne Wills <>
Tested-by: BuildBot <>
Reviewed-by: Andrew Deason <>
Reviewed-by: Benjamin Kaduk <>

12 months agoafs: Remove osi_GetuTime 36/14236/2
Andrew Deason [Tue, 2 Jun 2020 16:12:58 +0000]
afs: Remove osi_GetuTime

osi_GetuTime has always been #define'd to be the same thing as
osi_GetTime, ever since OpenAFS 1.0. Get rid of this redundant macro,
and just use osi_GetTime instead.

Change-Id: Ic826aeaa17314019b79cfb2df04a79309aa31db5
Tested-by: BuildBot <>
Reviewed-by: Michael Meffie <>
Reviewed-by: Benjamin Kaduk <>

12 months agoafs/viced: New UAE (unified_afs) error codes 35/14235/2
Jeffrey Altman [Sun, 31 May 2020 17:05:02 +0000]
afs/viced: New UAE (unified_afs) error codes

The following registrations werte submitted to
as [ #135105].

  UAECANCELED, "Operation canceled"            (49733499L)
  UAENOTRECOVERABLE, "State not recoverable"   (49733500L)
  UAENOTSUP, "Not supported"                   (49733501L)
  UAEOTHER, "Other"                            (49733502L)
  UAEOWNERDEAD, "Owner dead"                   (49733503L)
  UAEPROCLIM, "Too many processes"             (49733504L)
  UAEDISCON, "Graceful shutdown in progress"   (49733505L)

Change-Id: I1458b8a9441b3826756ca67af70eee5e835d989f
Reviewed-by: Jeffrey Hutzelman <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: Benjamin Kaduk <>

12 months agoutil: Fix segfault in the func ConstructLocalPath 23/14223/3
Cheyenne Wills [Fri, 29 May 2020 16:36:13 +0000]
util: Fix segfault in the func ConstructLocalPath

The function ConstructLocalPath will segfault if passed a NULL for
the command path parameter.

Update ConstructLocalPath to test the passed command path for a NULL
and return ENOENT.

The segfault can be triggered by setting up a BosConfig with a dafs
bnode that does not contain all the required parms.  This setup results
in bosserver segfaulting.  With the fix, bosserver now logs an error and
exits cleanly.

Change-Id: I26015c8accd829f3101b073964777b41d16b07f7
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

12 months agoDARWIN: ensure OpenAFS.pkg is signed 21/14221/3
Mark Vitale [Mon, 11 May 2020 00:53:22 +0000]
DARWIN: ensure OpenAFS.pkg is signed

Installation fails because the OpenAFS.pkg was inadvertently omitted
from the codesign logic.

Ensure that the package is signed.

Change-Id: I0745146bc523750912dd6ee95fc16a70572be175
Reviewed-by: Marcio Brito Barbosa <>
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

12 months agoDARWIN: ensure PrefPane materials are properly signed 20/14220/3
Mark Vitale [Mon, 11 May 2020 00:51:59 +0000]
DARWIN: ensure PrefPane materials are properly signed

Notarization fails because some prefPane materials were inadvertently
omitted by the codesign logic.

Ensure that these objects are properly signed.

Change-Id: Ifc58e6f834a3237b7991257ee85de4e90fc3da12
Reviewed-by: Marcio Brito Barbosa <>
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

12 months agovol: Avoid building devname.c on AFS_NAMEI_ENV 95/13995/4
Andrew Deason [Sat, 21 Dec 2019 03:02:45 +0000]
vol: Avoid building devname.c on AFS_NAMEI_ENV

Everything in devname.c is for the inode vol backend, so skip building
it when AFS_NAMEI_ENV is defined.

While we're doing this, alter the #ifdefs inside this file to assume
that we're not on XBSD, DARWIN, or LINUX, since those platforms are
all namei-only.

Change-Id: I3a46568940e1a865a381c1ac7e98aea94df9f3ef
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

12 months agovol: Indent ifdef maze in devname.c 94/13994/4
Andrew Deason [Sat, 21 Dec 2019 03:01:13 +0000]
vol: Indent ifdef maze in devname.c

Change-Id: I371eb1d79ae9fb3f07af993be834af6f6b59c100
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

12 months agoFBSD: Add support for FreeBSD 12.1 82/13982/5
Tim Creech [Tue, 10 Dec 2019 02:13:58 +0000]
FBSD: Add support for FreeBSD 12.1

Change-Id: I5779c586b6b1255de0ee0dea66b09f3a5dffddc1
Reviewed-by: Andrew Deason <>
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

12 months agoFBSD: Ignore VI_DOOMED vnodes 72/13972/6
Andrew Deason [Mon, 25 Nov 2019 04:36:17 +0000]
FBSD: Ignore VI_DOOMED vnodes

Currently on FreeBSD, osi_TryEvictVCache calls vgone() for our vnode
after checking if the given vcache is in use. vgone() then calls our
VOP_RECLAIM operation, which calls afs_vop_reclaim, which calls
afs_FlushVCache to finally actually flush the vcache.

The current approach has at least the following major issues:

- In afs_vop_reclaim, we return success even if afs_FlushVCache()
  fails. This allows FreeBSD to reuse the vnode for another file, but
  the vnode is still being referenced by our vcache, which is
  referenced by the global VLRU and various other structures. This
  causes all kinds of weird errors, since we try to use the underlying
  vnode for different files.

- After the relevant checks in osi_TryEvictVCache are done, another
  thread can acquire a new reference to our vcache (this can happen
  while vgone() is running up until the vnode is locked). This new
  reference will cause afs_FlushVCache to fail.

- Our afs_vop_reclaim callback is called while the vnode is locked,
  and can acquire afs_xvcache. Other code locks the vnode while
  afs_xvcache is already held (such as afs_PutVCache -> vrele). This
  can lead to deadlocks if two threads try to run these codepaths for
  the same vnode at the same time.

- afs_vop_reclaim optionally acquires afs_xvcache based on the return
  value of CheckLock(&afs_xvcache). However, CheckLock just returns if
  that lock is locked by anyone, not if the current thread holds the
  lock. This can result in the rest of the function running without
  afs_xvcache actually being held if we drop AFS_GLOCK at any point.

- osi_TryEvictVCache() tries to vn_lock() the target vnode, but we may
  already have another vnode locked in the current thread. If the
  vnode we're trying to evict is a descendant of a vnode we already
  have locked, this can deadlock.

To fix these issues, make some changes to how our vcache management
works on FreeBSD:

- Do not allow anyone to hold a new reference on a VI_DOOMED vnode.
  We do this by checking for VI_DOOMED in osi_vnhold, and returning an
  error if VI_DOOMED is set.

- In afs_vop_reclaim, panic if afs_FlushVCache fails. With the new
  VI_DOOMED check, afs_FlushVCache show now never fail; and if it
  somehow does, panic'ing immediately is better than corrupting
  various structures and panic'ing later on.

- Move around some of the relevant locking in afs_vop_reclaim to fix
  the lock-related issues.

- In osi_TryEvictVCache, don't wait for the vnode lock (LK_NOWAIT);
  treat the vnode as "in use" if we can't immediately obtain the lock.

Thanks to and for insight and help
investigating the relevant issues.

FIXES 135041

Change-Id: I23e94ecebbddc8c68a8f4ea918d64efd0f9f9dfd
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

12 months agoDARWIN: remove vestigial etap_event_t typedefs 19/14219/2
Mark Vitale [Mon, 11 May 2020 02:13:13 +0000]
DARWIN: remove vestigial etap_event_t typedefs

These typedefs have been present since commit
"initial-darwin-support-20010327"; at least some of this material was
obtained directly from IBM after the initial code import.

Based on research of old Darwin source code and kernel documentation,
the Event Trace Analysis Package (ETAP) was a lock-profiling interface
provided in older versions of Mach and xnu.  ETAP was not enabled by
default; the kernel had to be recompiled with certain options to enable
it.  Support for ETAP was removed from the xnu tree sometime between
xnu-517 (10.3 Panther) and xnu-792 (10.4 Tiger), although some
references remain in the latter under PPC support (osfmk/ppc/hw_lock.s).
All remaining references to etap_event_t disappeared when PPC support
was removed, some time between xnu-1456.1.26 (10.6 Snow Leopard) and
xnu-1699.24.8 (10.7.2 Lion).

Therefore, it is possible that these typedefs were needed in the past by
(IBM/Transarc) AFS to support use of some lock APIs (e.g.,
simple_lock_init, usimple_lock_init) after the ETAP code was withdrawn
from xnu.  However, these typedefs have probably always been vestigial
for OpenAFS, because OpenAFS has never used any lock API that took
etap_event_t as an argument.

Regardless, OpenAFS does not need these definitions to build and run on
any currently supported version of macOS.

Remove the vestigial code.

No functional change should be incurred by this commit.

Change-Id: I39b3f82a8933d15ef5b5de5eb92366c0a31f8bb6
Tested-by: BuildBot <>
Reviewed-by: Marcio Brito Barbosa <>
Reviewed-by: Benjamin Kaduk <>

12 months agoDARWIN: remove errant typedef for etap_event_t 18/14218/3
Mark Vitale [Mon, 11 May 2020 02:07:39 +0000]
DARWIN: remove errant typedef for etap_event_t

This code has been dead since its introduction, because XAFS_DARWIN_ENV
is a typo for AFS_DARWIN_ENV.

Introduced from day 1 of DARWIN support with commit

No functional change should be incurred by this commit.

Change-Id: I6b74f01b4dd1230559ac8d75f0644071357f38b7
Reviewed-by: Marcio Brito Barbosa <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

12 months agoConvert all osi_timeval_t to osi_timeval32_t 15/14215/5
Mark Vitale [Mon, 18 May 2020 18:19:25 +0000]
Convert all osi_timeval_t to osi_timeval32_t

Since commit 130144850c6d05bc69e06257a5d7219eb98697d8 "xstat: cm xstat
time values are 32 bit", OpenAFS has had two timeval definitions:
osi_timeval_t and osi_timeval32_t.  Since they are functionally
equivalent, convert all references to osi_timeval_t to osi_timeval32_t.
This makes clear that this struct is always expected to contain 32-bit
members for tv_sec and tv_usec.

There are still a few platforms where osi_timeval32_t is mistakenly
defined with 64-bit members; these will be addressed in future commits.

No functional change should be incurred by this commit.

Change-Id: I3e8e44235e813571723fcd114194f6cb83de90e4
Reviewed-by: Andrew Deason <>
Tested-by: BuildBot <>
Reviewed-by: Cheyenne Wills <>
Reviewed-by: Benjamin Kaduk <>

12 months agoUKERNEL: remove dead code osi_SetTime 91/14191/7
Mark Vitale [Mon, 4 May 2020 21:35:05 +0000]
UKERNEL: remove dead code osi_SetTime

osi_SetTime has been dead code since the original IBM code import.
Remove it from the tree.

No functional change is incurred by this commit.

Change-Id: I25612a044ad550d798003979afc6845e502ebe3b
Reviewed-by: Andrew Deason <>
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

12 months agoUKERNEL: remove redundant declaration of osi_GetTime 92/14192/7
Mark Vitale [Tue, 5 May 2020 15:26:00 +0000]
UKERNEL: remove redundant declaration of osi_GetTime

Commit c861bb0d779b54236b63eda87d9dfaf7792d1659 "Additional UKERNEL
headers, prototyping and other fixes" added the following lines to

  #if defined(UKERNEL) && !defined(osi_GetTime)
  extern int osi_GetTime(struct timeval *tv);

However, this appears to be redundant with the declaration in

  #ifdef UKERNEL
  extern int osi_GetTime(struct timeval *tv);

which was added much earlier with commit

Remove the redundant declaration in rx/rx_prototypes.h.

No functional change is incurrred by this commit.

Change-Id: I2032d302e862eed47250357e604cba4f26e89814
Reviewed-by: Andrew Deason <>
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

12 months agoafs: remove commented xstats externs 97/14197/9
Mark Vitale [Thu, 16 Apr 2020 13:02:00 +0000]
afs: remove commented xstats externs

Extern declarations for the xstats recording areas have been commented
out since 8f2df21ffe59e9aa66219bf24656775b584c122d

Remove the vestigial comments.

No functional change is incurred by this commit.

Change-Id: Ieef9a4b21e78db8d5427bed7b621ba043663b1d1
Reviewed-by: Benjamin Kaduk <>
Reviewed-by: Andrew Deason <>
Tested-by: BuildBot <>

12 months agoafs: remove stats dead code 96/14196/9
Mark Vitale [Sun, 5 Apr 2020 21:10:42 +0000]
afs: remove stats dead code

afs_GetCMSTats, afs_AddToMean, and macro AFS_MEANCNT have been dead code
since the original IBM code import.  Remove them from the tree.

No functional change is incurred by this commit.

Change-Id: Icd6aeff7896d69a4d334531b5e0c632d807457ce
Reviewed-by: Benjamin Kaduk <>
Reviewed-by: Andrew Deason <>
Tested-by: BuildBot <>

12 months agoLINUX 5.6: define osi_timeval32_t for 32-bit Linux 16/14216/6
Mark Vitale [Mon, 18 May 2020 21:20:26 +0000]
LINUX 5.6: define osi_timeval32_t for 32-bit Linux

For 32-bit Linux (e.g., arch i586), AFS_LINUX_64BIT_KERNEL is not
defined, so osi_timeval32_t is defined as a typedef of the native
'timeval'.  However, as of commit
c766d1472c70d25ad475cf56042af1652e792b23 "y2038: hide
timeval/timespec/itimerval/itimerspec types" (Linux 5.6), the native
timeval struct is no longer available.  On such a kernel, the OpenAFS
build will fail because osi_timeval32_t is not properly defined.

Instead, add new conditionals to properly define osi_timeval32_t for
this platform.

Change-Id: I1eddeeb3651dcd3c55920ab1d2ad2838f4729bdd
Reviewed-by: Cheyenne Wills <>
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

12 months agoafs: Refactor osi_vnhold/AFS_FAST_HOLD 71/13971/6
Andrew Deason [Tue, 19 Nov 2019 05:17:12 +0000]
afs: Refactor osi_vnhold/AFS_FAST_HOLD

Make a few changes to osi_vnhold and AFS_FAST_HOLD:

- Currently, the second argument of osi_vnhold ("retry") is never used
  by any implementation. Get rid of it.

- AFS_FAST_HOLD() is the same as osi_vnhold(). Get rid of
  AFS_FAST_HOLD, and just have all callers use osi_vnhold instead.

- Allow osi_vnhold to return an error, and adjust callers to handle

- Change osi_vnhold to be a real function, instead of a macro, to make
  nontrivial implementations less cumbersome.

Most platforms never return an error from osi_vnhold(), so the added
code paths to check the return value of osi_vnhold() will not trigger.
However, this lets us add future commits that do make osi_vnhold()
return an error.

Change-Id: Id2f3717be6c305d06305685247ac789815e1ebf7
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>