openafs.git
2 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
Reviewed-on: https://gerrit.openafs.org/14280
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Kailas Zadbuke <kailashsz@in.ibm.com>
Reviewed-by: Yadavendra Yadav <yadayada@in.ibm.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 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
errors.

Change-Id: Ibd6e76c5ea0dea95e3354d9b34536296f81b4f67
Reviewed-on: https://gerrit.openafs.org/14255
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 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,
                 -Wimplicit-int-float-conversion]
    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-on: https://gerrit.openafs.org/14277
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 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 <https://savannah.gnu.org/patch/?9949>,
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-on: https://gerrit.openafs.org/14273
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

2 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
attribute.

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
Reviewed-on: https://gerrit.openafs.org/14272
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 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-on: https://gerrit.openafs.org/13042
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

2 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
ended.

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
Reviewed-on: https://gerrit.openafs.org/13041
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 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
limitations.

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
Reviewed-on: https://gerrit.openafs.org/14214
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Tested-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 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
return.

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
running.

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-on: https://gerrit.openafs.org/14254
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 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-on: https://gerrit.openafs.org/13256
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 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
days).

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-on: https://gerrit.openafs.org/13255
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 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
Reviewed-on: https://gerrit.openafs.org/14256
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 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-on: https://gerrit.openafs.org/14252
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

2 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 pruiter@sinenomine.net

Change-Id: Ic47e4868b4f81d99fbe3b2e4958778508ae4851f
Reviewed-on: https://gerrit.openafs.org/14242
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

2 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
used.

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-on: https://gerrit.openafs.org/14241
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 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()
      (6058eaec)
  lru_cache_add exported
     mm: fold and remove lru_cache_add_anon() and lru_cache_add_file()
      (6058eaec)

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
Reviewed-on: https://gerrit.openafs.org/14249
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Yadavendra Yadav <yadayada@in.ibm.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 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
Reviewed-on: https://gerrit.openafs.org/14265
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 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
Reviewed-on: https://gerrit.openafs.org/13253
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 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
to.

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
Reviewed-on: https://gerrit.openafs.org/12656
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 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
jobs.

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(yadayada@in.ibm.com) for working with me
on this issue.

Change-Id: I191c8272825c1667bb2150146e04b1dfe36a54e4
Reviewed-on: https://gerrit.openafs.org/14239
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

3 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
reorganization.

Change-Id: I0086189308d2f5e4b321c63f24110d74cda6433c
Reviewed-on: https://gerrit.openafs.org/13254
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 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
Reviewed-on: https://gerrit.openafs.org/12655
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 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-on: https://gerrit.openafs.org/14250
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>

3 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
    'STRUCT_BACKING_DEV_INFO_HAS_NAME'

Note the name field in the backing_dev_info structure was added in
Linux-2.6.32

Change-Id: I20b80e49e8a15a2949003101f24d9ce39f63b59b
Reviewed-on: https://gerrit.openafs.org/14248
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 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
Reviewed-on: https://gerrit.openafs.org/14247
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 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-on: https://gerrit.openafs.org/14246
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

3 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-on: https://gerrit.openafs.org/14244
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 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-on: https://gerrit.openafs.org/14243
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 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-on: https://gerrit.openafs.org/14150
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 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-on: https://gerrit.openafs.org/14115
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 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
do.

Change-Id: Ie4c6e6efb7693e53a02fd009eecd64b47250c848
Reviewed-on: https://gerrit.openafs.org/14149
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>

3 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-on: https://gerrit.openafs.org/14148
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 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
Reviewed-on: https://gerrit.openafs.org/14236
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 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 registrar@central.org
as [rt.central.org #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-on: https://gerrit.openafs.org/14235
Reviewed-by: Jeffrey Hutzelman <jhutz@cmu.edu>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>

3 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
Reviewed-on: https://gerrit.openafs.org/14223
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 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-on: https://gerrit.openafs.org/14221
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 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-on: https://gerrit.openafs.org/14220
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 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
Reviewed-on: https://gerrit.openafs.org/13995
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 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
Reviewed-on: https://gerrit.openafs.org/13994
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 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-on: https://gerrit.openafs.org/13982
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 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 tcreech@tcreech.com and kaduk@mit.edu for insight and help
investigating the relevant issues.

FIXES 135041

Change-Id: I23e94ecebbddc8c68a8f4ea918d64efd0f9f9dfd
Reviewed-on: https://gerrit.openafs.org/13972
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 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
a41175cfbbf4d06ccfe14ae54bef8b7464ecd80b
"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
Reviewed-on: https://gerrit.openafs.org/14219
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 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
a41175cfbbf4d06ccfe14ae54bef8b7464ecd80b
"initial-darwin-support-20010327".

No functional change should be incurred by this commit.

Change-Id: I6b74f01b4dd1230559ac8d75f0644071357f38b7
Reviewed-on: https://gerrit.openafs.org/14218
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

4 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-on: https://gerrit.openafs.org/14215
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 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-on: https://gerrit.openafs.org/14191
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 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
src/rx/rx_prototypes.h:

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

However, this appears to be redundant with the declaration in
src/afs/afs_prototypes.h:

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

which was added much earlier with commit
8f2df21ffe59e9aa66219bf24656775b584c122d
"pull-prototypes-to-head-20020821".

Remove the redundant declaration in rx/rx_prototypes.h.

No functional change is incurrred by this commit.

Change-Id: I2032d302e862eed47250357e604cba4f26e89814
Reviewed-on: https://gerrit.openafs.org/14192
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 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
"pull-prototypes-to-head-20020821".

Remove the vestigial comments.

No functional change is incurred by this commit.

Change-Id: Ieef9a4b21e78db8d5427bed7b621ba043663b1d1
Reviewed-on: https://gerrit.openafs.org/14197
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

4 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-on: https://gerrit.openafs.org/14196
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

4 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-on: https://gerrit.openafs.org/14216
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 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
  it.

- 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
Reviewed-on: https://gerrit.openafs.org/13971
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 months agovlserver: Return error when growing beyond 2 GiB 80/14180/4
Andrew Deason [Fri, 1 May 2020 20:02:08 +0000]
vlserver: Return error when growing beyond 2 GiB

In the vlserver, when we add a new vlentry or extent block, we grow
the VLDB by doing something like this:

    vital_header.eofPtr += sizeof(item);

Since we don't check for overflow, and all of our offset-related
variables are signed 32-bit integers, this can cause some odd behavior
if we try to grow the database to be over 2 GiB in size.

To avoid this, change the two places in vlserver code that grow the
database to use a new function, grow_eofPtr(), which checks for 31-bit
overflow. If we are about to overflow, log a message and return an
error.

See the following for a specific example of our "odd behavior" when we
overflow the 2 GiB limit in the VLDB:

With 1 extent block, we can create 14509076 vlentries successfully. On
the 14509077th vlentry, we'll attempt to write the entry to offset
2147483560 (0x7FFFFFA8). Since a vlentry is 148 bytes long, we'll
write all the way through offset 2147483707 (0x8000003B), which is
over the 31-bit limit.

In the udisk subsystem, this results in writing to page numbers
2097151, and -2097152 (since our ubik pages are 1k, and going over the
31-bit limit causes us to treat offsets as negative). These pages
start at physical offsets 2147482688 (0x7FFFFC40) and -2147483584
(-0x7FFFFFC0) in our vldb.DB0 (where offset is page*1024+64).

Modifying each of these pages involves reading in the existing page
first, modifying the parts we are changing, and writing it back. This
works just fine for 2097151, but of course fails for -2097152. The
latter fails in DReadBuffer when eventually our pread() fails with
EINVAL, and causes ubik to log the message:

    Ubik: Error reading database file: errno=22

But when DReadBuffer fails, DReadBufferForWrite assumes this is due to
EOF, and just creates a new buffer for the given page (DNewBuffer).
So, the udisk_write() call ultimately succeeds.

When we go to flush the dirty data to disk when committing the
transaction, after we have successfully written the transaction log,
DFlush() fails for the -2097152 page when the pwrite() call eventually
fails with EINVAL, causing ubik to panic, logging the messages:

    Ubik PANIC:
    Writing Ubik DB modifications

When the vlserver gets restarted by bosserver, we then process the
transaction log, and perform the operations in the log before starting
up (ReplayLog). The log records the actual data we wrote, not split
into pages, and the log-replaying code writes directly to the db
usying uphys_write instead of udisk_write. So, because of this, the
write actually succeeds when replaying the log, since we just write
148 bytes to offset 2147483624 (0x7FFFFFE8), and no negative offsets
are used.

The vlserver will then be able to run, but will be unable to read that
newly-created vlentry, since it involves reading a ubik page beyond
the 31-bit boundary. That means trying to lookup that entry will fail
with i/o errors, and as well as any entry on the same hash chains as
the new entry (since the new entry will be added to the head of the
hash chain). Listing all entries in the database will also just show
an empty database, since our vital_header.eofPtr will be negative, and
we determine EOF by comparing our current blockindex to the value in
eofPtr.

Change-Id: Ie0b7ac61f9121fa265686449efbae8e18edb1896
Reviewed-on: https://gerrit.openafs.org/14180
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>

4 months agovol: Fix format-truncation warning with gcc-10.1 07/14207/2
Cheyenne Wills [Mon, 11 May 2020 20:06:19 +0000]
vol: Fix format-truncation warning with gcc-10.1

Building with gcc-10.1 produces a warning (error if --enable-checking)
in vol-salvage.c

error: ā€˜%sā€™ directive output may be truncated writing up to 755 bytes
       into a region of size 255 [-Werror=format-truncation=]
  809 |     snprintf(inodeListPath, 255, "%s" OS_DIRSEP "salvage.inodes.%s.%d", tdir, name,

Use strdup/asprintf to allocate the buffer dynamically instead of using
a buffer with a hardcoded size.

Change-Id: Ib2f01c2eb73c7abc162be2b1939e55688a81f812
Reviewed-on: https://gerrit.openafs.org/14207
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 months agoauth: Close fd on SetExtendedCellInfo write error 13/14213/2
Andrew Deason [Mon, 18 May 2020 17:09:38 +0000]
auth: Close fd on SetExtendedCellInfo write error

Currently, and since OpenAFS 1.0, if write() fails here, we leak the
file descriptor. A write() failure should be very unlikely, but close
the fd to make sure we avoid the leak.

Change-Id: I4e8ed4216c4aa5041232fc798a7bc59f6a5570d9
Reviewed-on: https://gerrit.openafs.org/14213
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 months agoafs: Free rx/rxevent resources during shutdown 19/13719/4
Andrew Deason [Sun, 21 Jul 2019 23:55:49 +0000]
afs: Free rx/rxevent resources during shutdown

Call shutdown_rx() and shutdown_rxevent() near the end of our shutdown
sequence, in order to free various Rx resources and avoid memory
leaks.

Change-Id: Id2e912295cf760b5ad83057487e6c4c4fadda11b
Reviewed-on: https://gerrit.openafs.org/13719
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

4 months agoLINUX-5.7: replace __pagevec_lru_add with lru_cache_add_file 59/14159/8
Cheyenne Wills [Thu, 30 Apr 2020 16:31:17 +0000]
LINUX-5.7: replace __pagevec_lru_add with lru_cache_add_file

The Linux function __pagevec_lru_add is no longer exported in Linux
5.7-rc1 commit bde07cfc65da5fe6c63fe23f035f5ccc0ffd89e0
"mm/swap.c: not necessary to export __pagevec_lru_add()".

As a replacement, the Linux function lru_cache_add_file can be used for
adding a page to the lru cache.  The internal processing of
lru_cache_add_file manages its own internal pagevec and performs the
following:
     get_page(...)
     if(!pagevec_add(...))
        __pagevec_lru_add_file(...)

Introduce an autoconf test for lru_cache_add_file and replace the calls
associated with __pagevec_lru_add with lru_cache_add_file.

NOTE: see Linux commit a0b8cab3b9b2efadabdcff264c450ca515e2619c
"mm: remove lru parameter from __pagevec_lru_add and remove parts of
pagevec API" as a reference for this change.

The lru_cache_add_file was introduced in Linux 2.6.28, therefore this
change affects systems with Linux 2.6.28 kernels and later.

Change-Id: I12b32fd5061fc136f8b96ef3605e0bab736ca9ed
Reviewed-on: https://gerrit.openafs.org/14159
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

4 months agolibafs: Abstract the Linux lru cache interface 67/14167/5
Cheyenne Wills [Wed, 29 Apr 2020 22:26:02 +0000]
libafs: Abstract the Linux lru cache interface

Define static functions afs_lru_cache_init, afs_lru_cache_add and
afs_lru_cache_finalize to handle interfacing with Linux's lru
facilities.

This change's primary purpose is to isolate the preprocessor
conditionals associated with the details of the system lru interfaces to
just these functions and to simplify the areas that utilize lru caching
by removing the preprocessor conditionals.

As Linux's lru facilities change, additional conditional code will be
needed.

Change-Id: I74c94bb712359975e3fd1df85f1b338b215f61b0
Reviewed-on: https://gerrit.openafs.org/14167
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 months agoafs: Drop GLOCK for RXAFS_GetCapabilities 81/14181/2
Andrew Deason [Sun, 3 May 2020 04:54:55 +0000]
afs: Drop GLOCK for RXAFS_GetCapabilities

We are hitting the net here; we certainly should not be holding
AFS_GLOCK while waiting for the server's response.

Found via FreeBSD WITNESS.

Change-Id: Ie727db27adaeed23ac8cff7665143bae2ce2ede8
Reviewed-on: https://gerrit.openafs.org/14181
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 months agorxkad: Use krb5_enctype_keysize in tkt_DecodeTicket5 03/14203/5
Yadavendra Yadav [Wed, 29 Apr 2020 05:10:05 +0000]
rxkad: Use krb5_enctype_keysize in tkt_DecodeTicket5

Inside tkt_DecodeTicket5 (rxkad/ticket5.c) function, keysize is calculated
using krb5_enctype_keybits and then dividing number of bits by 8. For 3DES
number of keybits are 168, so keysize comes out to 21(168/8). However
actual keysize of 3DES key is 24. This keysize is passed to
_afsconf_GetRxkadKrb5Key where keysize comparison happens, since there is
keysize mismatch it returns AFSCONF_BADKEY.

To fix this issue get keysize from krb5_enctype_keysize function instead
of krb5_enctype_keybits. Thanks to John Janosik (jpjanosi@us.ibm.com)
for analyzing and fixing this issue.

Change-Id: Ia6f70b878feaa91855f9544ec1de81a6196a85a8
Reviewed-on: https://gerrit.openafs.org/14203
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 months agorx: Avoid osi_NetSend during rx shutdown 18/13718/6
Andrew Deason [Sun, 21 Jul 2019 23:48:51 +0000]
rx: Avoid osi_NetSend during rx shutdown

Commit 8d939c08 (rx: avoid nat ping during shutdown) added a call
to shutdown_rx() inside the DARWIN shutdown sequence, before the rx
socket was closed. From the commit message, it sounds like this was
done to avoid NAT pings from calling osi_NetSend during the shutdown
sequence after the rx socket was closed; calling shutdown_rx() before
closing the socket would cause any connections we had to be destroyed
first, avoiding that.

The problem with this is that this means shutdown_rx() is called when
osi_StopNetIfPoller is called, which is much earlier than some other
portions of the shutdown sequence; some of which may hold references
to e.g. rx connections. If we try to, for instance, destroy an rx
connection after shutdown_rx() is called, we could panic.

An earlier version of that commit (gerrit PS1) just tried to insert a
check before the relevant osi_NetSend call, making us just skip the
osi_NetSend if the shutdown sequence had been started. So to avoid the
above issue, try to implement that approach instead. And instead of
doing it just for NAT pings, we can do it for almost all osi_NetSend
calls (besides those involved in the shutdown sequence itself), by
checking this in rxi_NetSend. Also return an error (ESHUTDOWN) if we
skip the osi_NetSend call, so we're not completely silent about doing
so.

This means we also remove the call to shutdown_rx() inside DARWIN's
osi_StopNetIfPoller(). This allows us to interact with Rx objects
during more of the shutdown process in cross-platform code.

Change-Id: I4e631b28d090635aeacd59de0fd237d572f97e93
Reviewed-on: https://gerrit.openafs.org/13718
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

4 months agoAdd more 'fall through' switch comments 25/14125/16
Cheyenne Wills [Fri, 3 Apr 2020 21:00:42 +0000]
Add more 'fall through' switch comments

Commit a455452d (LINUX 5.3: Add comments for fallthrough switch cases)
added the special /* fall through */ comment to various switch/case
blocks, in order to avoid implicit-fallthrough warnings from causing
the build to fail when building the Linux kernel module.

In this commit, add additional /* fall through */ comments to the rest
of the tree where falling through is intentional. Add a "break;" in one
place in dumptool.c where falling through seems like a mistake, and flag
certain functions as AFS_NORETURN to avoid needing to explicitly break
or fallthrough.

Check for the availability of the -Wimplicit-fallthrough compiler flag
and use it when --enable-checking is set, to prevent additional cases
from creeping into the tree.

Note: the -Wimplicit-fallthrough compiler flag was added in gcc 7.

Change-Id: Iae34e7969606603da8358d7cfa5fd04279b218dc
Reviewed-on: https://gerrit.openafs.org/14125
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

4 months agosalvaged: Fix "-parallel all<number>" parsing 01/14201/5
Kailas Zadbuke [Fri, 8 May 2020 03:55:39 +0000]
salvaged: Fix "-parallel all<number>" parsing

In salavageserver -parallel option takes "all<number>" argument.
However the code does not parse the numeric part correctly. Due
to this, only single instance of salvageserver process was running
even if we provide the larger number with "all" argument.

With this fix, numeric part of "all" argument will be parsed
correctly and will start required number of salvageserver instances.

Change-Id: Ib6318b1d57d04fecb84915e2dabe40930ea76499
Reviewed-on: https://gerrit.openafs.org/14201
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 months agocf: Use common macro to test compiler flags 32/14132/7
Cheyenne Wills [Sun, 5 Apr 2020 21:51:17 +0000]
cf: Use common macro to test compiler flags

Use the AX_APPEND_COMPILE_FLAGS macro to test and set compiler
specific flags.

Remove the OPENAFS_GCC_SUPPORTS_MARCH check entirely (and the
associated P5PLUS_KOPTS), since nothing has used it for quite some
time.

Change-Id: Ic9626c52ac62cf83d4b8c787aa5aa966e558a781
Reviewed-on: https://gerrit.openafs.org/14132
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

4 months agoubik: Avoid unlinking garbage during recovery 53/14153/3
Andrew Deason [Mon, 20 Apr 2020 18:03:15 +0000]
ubik: Avoid unlinking garbage during recovery

In urecovery_Interact, if any of our operations fail around
calling DISK_GetFile, we will jump to FetchEndCall and eventually
unlink 'pbuffer'. But if we failed before opening our .DB0.TMP file,
the contents of 'pbuffer' will not be initialized yet.

During most iterations of the recovery loop, the contents of 'pbuffer'
will be filled in from previous loops, and it should always stay the
same, so it's not a big problem. But if this is the first iteration of
the loop, the contents of 'pbuffer' may be stack garbage.

Solve this in two ways. To make sure we don't use garbage contents in
'pbuffer', memset the whole thing to zeroes at the beginning of
urecovery_Interact(). And then to make sure we're not reusing
'pbuffer' contents from previous iterations of the loop, also clear
the first character to NUL each time we arrive at this area of the
recovery code. And avoid unlinking anything if pbuffer starts with a
NUL.

Commit 44e80643 (ubik: Avoid unlinking garbage) fixes the same issue,
but only fixed it in the SDISK_SendFile codepath in remote.c.

Change-Id: Ica39e66efa89562068a4be3a14b2d13594b77f6d
Reviewed-on: https://gerrit.openafs.org/14153
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 months agoUse autoconf-archive m4 from src/external 35/14135/5
Andrew Deason [Sun, 5 Apr 2020 03:35:07 +0000]
Use autoconf-archive m4 from src/external

Switch to using the m4 macros from autoconf-archive in our
src/external mechanism, instead of manually-copied versions in src/cf.
The src/external copy of ax_gcc_func_attribute.m4 is identical to the
existing copy in src/cf, so that should incur no changes. There are
also a few new macros pulled in, but they are currently unused.

Increase our AC_PREREQ in configure.ac to 2.64, to match the AC_PREREQ
in some of the new files.

Change-Id: I8acfe4df7b9a22d9b9e69004c3438034a2dacadb
Reviewed-on: https://gerrit.openafs.org/14135
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 months agoImport of code from autoconf-archive 38/14138/3
Autoconf Archive Maintainers [Tue, 7 Apr 2020 15:23:16 +0000]
Import of code from autoconf-archive

This commit updates the code imported from autoconf-archive to
24358c8c5ca679949ef522964d94e4d1cd1f941a (v2019.01.06)

New files are:
m4/ax_append_compile_flags.m4
m4/ax_append_flag.m4
m4/ax_check_compile_flag.m4
m4/ax_gcc_func_attribute.m4
m4/ax_require_defined.m4

Change-Id: I64e14d1b4d41ebfee82fa92da10239f73e28b4c9
Reviewed-on: https://gerrit.openafs.org/14138
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 months agoAdd autoconf-archive to src/external 33/14133/4
Andrew Deason [Sun, 5 Apr 2020 03:28:21 +0000]
Add autoconf-archive to src/external

Add autoconf-archive to the src/external mechanism, so we can more
easily import and update the AX_* m4 macros we pull in from
autoconf-archive. Commits are imported from
<git://git.savannah.gnu.org/autoconf-archive.git>.

We already have a copy of ax_gcc_func_attribute.m4 in the tree, so
include that in the list of files. While we're here, also include a
few more macros for checking compiler flags, which will be used in
subsequent commits.

Change-Id: I8c6288fc1d48a47837ca08f8b9207e0ada921af8
Reviewed-on: https://gerrit.openafs.org/14133
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 months agoUpdate NEWS for OpenAFS 1.9.0 73/13673/9
Michael Meffie [Fri, 5 Jul 2019 13:28:50 +0000]
Update NEWS for OpenAFS 1.9.0

Add change descriptions for commits not in a stable release.

Change-Id: Ib1d5ce9f558279660abb2473ce8a9fac4fcefa8d
Reviewed-on: https://gerrit.openafs.org/13673
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>

4 months agoSynchronize NEWS with 1.8.5 03/14103/2
Benjamin Kaduk [Fri, 20 Mar 2020 16:17:13 +0000]
Synchronize NEWS with 1.8.5

Pull in all the updates to NEWS that occurred on the 1.8.x branch
in preparation for adding entries for 1.9.0.

Change-Id: I713d1576ef96793f24824f909b26da802b21ec23
Reviewed-on: https://gerrit.openafs.org/14103
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 months agorx: Use _IsLast to check for last call in queue 58/14158/2
Andrew Deason [Sun, 26 Apr 2020 22:26:02 +0000]
rx: Use _IsLast to check for last call in queue

Ever since commits 170dbb3c (rx: Use opr queues) and d9fc4890 (rx: Fix
test for end of call queue for LWP), rx_GetCall checks if the current
call is the last one on rx_incomingCallQueue by doing this:

    opr_queue_IsEnd(&rx_incomingCallQueue, cursor)

But opr_queue_IsEnd checks if the given pointer is the _end_ of the
last; that is, if it's the end-of-list sentinel, not an item on the
actual list. Testing for the last item in a list is what
opr_queue_IsLast is for. This is the same convention that the old Rx
queues used, but 170dbb3c just accidentally replaced queue_IsLast with
opr_queue_IsEnd (instead of opr_queue_IsLast), and d9fc4890 copied the
mistake.

So because this is inside an opr_queue_Scan loop, opr_queue_IsEnd will
never be true, so we'll never enter this block of code (unless we are
the "fcfs" thread). This means that an incoming Rx call can get stuck
in the incoming call queue, if all of the following are true:

 - The incoming call consists of more than 1 packet of incoming data.

 - The incoming call "waits" when it comes in (that is, there are no
   free threads or the service is over quota).

 - The "fcfs" thread doesn't scan the incoming call queue (because it
   is idle when the call comes in, but the relevant service is over
   quota).

To fix this, just use opr_queue_IsLast here instead of
opr_queue_IsEnd.

Change-Id: I04b90b1279f81dc518eb61e7bd450e3c0be37a77
Reviewed-on: https://gerrit.openafs.org/14158
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

4 months agotests: Give more leeway in rx/event-t 60/14160/2
Andrew Deason [Sat, 25 Apr 2020 23:21:10 +0000]
tests: Give more leeway in rx/event-t

Currently, the rx/event-t tests schedule a bunch of events up to 3
seconds in the future, and then we sleep for 3 seconds to give them a
chance to run. Since we're cutting it so close, this can rarely result
in a few events not being run (observed occasionally on FreeBSD 12.1,
where we failed to run about 3 events out of 10000).

To avoid this, just sleep for 4 seconds instead of 3. Also print out a
little more info regarding the number of fired/cancelled events, so we
can see the event count when it's wrong.

Change-Id: I6269bea2c245aeed00c129ff638423d0fa81ad23
Reviewed-on: https://gerrit.openafs.org/14160
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

4 months agoafs: fix afs_linux_mmap fstrace entry 68/14168/2
Mark Vitale [Thu, 23 Apr 2020 21:49:20 +0000]
afs: fix afs_linux_mmap fstrace entry

The format string for CM_TRACE_GMAP takes 4 substitutions, but
afs_linux_mmap only supplies 3.  This results in malformed output from
fstrace:

Type mismatch, using raw print.
Gn_map vp 0x%lx addr 0x%lx len 0x%x off 0x%x (afs / zcm)raw op
701087775, time 715.322573, pid 9644
p0:0xc0a66ec0 p1:0x8b81a000 p2:131072

Repair the recording of CM_TRACE_GMAP.

Change-Id: I2b7592e68cb42f5ae490ee8771558e5cc5a2181e
Reviewed-on: https://gerrit.openafs.org/14168
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

5 months agotests: Skip SIGBUS test on FreeBSD 45/14145/3
Andrew Deason [Mon, 13 Apr 2020 03:28:29 +0000]
tests: Skip SIGBUS test on FreeBSD

Currently, 'softsig-helper -buserror' causes a SIGBUS on most
platforms, but can result in SIGSEGV on FreeBSD by default (at least
on 11.3-RELEASE). Skip the test on FreeBSD, until we can provide a
more reliable way to generate SIGBUS.

Note that when the sysctl machdep.prot_fault_translation is set to 1,
'softsig-helper -buserror' generates a SIGBUS instead of SIGSEGV,
suggesting that generating a SIGBUS here is the old 'compat' behavior.
When machdep.prot_fault_translation is 0 (the default), the code path
in the FreeBSD kernel that dictates whether to send a SIGBUS or
SIGSEGV in this situation depends on some autodetection heuristics,
and so may produce different results depending on FreeBSD releases or
even compiler settings (due to detection of ABI based on some ELF
notes in the relevant binary).

For some details on this sysctl, see
<https://www.freebsd.org/news/status/report-2019-07-2019-09.html#Signals-delivered-on-unhandled-Page-Faults>
or the FreeBSD source code. In 11.3-RELEASE, the decision to issue a
SIGBUS or SIGSEGV can be found around sys/amd64/amd64/trap.c:355.

Change-Id: Ib75b43cc12302532ee87a3744fc364424f2a3ca6
Reviewed-on: https://gerrit.openafs.org/14145
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

5 months agoFBSD: Avoid holding AFS_GLOCK during vinvalbuf 70/13970/5
Andrew Deason [Wed, 27 Nov 2019 05:39:24 +0000]
FBSD: Avoid holding AFS_GLOCK during vinvalbuf

Currently we call vinvalbuf(9) in a few places while holding
AFS_GLOCK, but AFS_GLOCK is a non-sleepable lock (struct mtx), and
vinvalbuf can sleep. This can trigger a panic in some rare conditions,
with the message:

    Sleeping thread (tid 100179, pid 95481) owns a non-sleepable lock

To avoid this, drop AFS_GLOCK around a few places that call
vinvalbuf().

Change-Id: I58acb144b6ffa007675402e7639b63ff3745dec5
Reviewed-on: https://gerrit.openafs.org/13970
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

5 months agoafs: Fix ifdef indenting in afs_vcache.c 77/13877/5
Andrew Deason [Mon, 16 Sep 2019 04:00:26 +0000]
afs: Fix ifdef indenting in afs_vcache.c

Change-Id: Ib566156184cb3f64a0983babd5d9f7883c84cc85
Reviewed-on: https://gerrit.openafs.org/13877
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>

5 months agoFBSD: Remove MA_* abstractions 43/13843/6
Andrew Deason [Sun, 8 Sep 2019 21:10:40 +0000]
FBSD: Remove MA_* abstractions

In FBSD/osi_vnops.c, we have a few abstractions (e.g. MA_VOP_UNLOCK)
that used to expand to different things for older FreeBSD versions.
Currently, they always expand to the same thing, so just remove the
abstractions.

While we are changing these calls, also change one instance of
MA_VOP_LOCK to vn_lock (instead of VOP_LOCK), since we're not usually
supposed to call VOP_LOCK directly, according to the VOP_LOCK(9)
manpage. The MA_VOP_LOCK call was added in commit bd707fb7
(freebsd-almost-working-client-20020216), seemingly by mistake.

Change-Id: Ia0f28fe658057e87d9103a72296ab899dc762fb6
Reviewed-on: https://gerrit.openafs.org/13843
Reviewed-by: Tim Creech <tcreech@tcreech.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

5 months agoFBSD: Build vnode_if.h before libafs objs 83/13983/3
Tim Creech [Sat, 14 Dec 2019 03:24:57 +0000]
FBSD: Build vnode_if.h before libafs objs

Currently, if we are building with -j2 or higher, we can easily fail
to build some libafs objects because vnode_if.h does not exist yet.
vnode_if.h is generated by the FreeBSD build, but none of our objects
depend on it, so during parallel builds it may not be available by the
time we build, for example, src/external/heimdal/hcrypto/sha256.c.

This results in build errors that can look like this:

    --- sha256-kernel.o ---
cc -I. -I.. -I../nfs [...]/src/external/heimdal/hcrypto/sha256.c
    In file included from [...]/src/external/heimdal/hcrypto/sha256.c:34:
    In file included from [...]/src/crypto/hcrypto/kernel/config.h:30:
    In file included from [...]/src/afs/sysincludes.h:354:
    /usr/src/sys/sys/vnode.h:588:10: fatal error: 'vnode_if.h' file not found
    #include "vnode_if.h"
             ^~~~~~~~~~~~
    1 error generated.
    *** [sha256-kernel.o] Error code 1

    make[4]: stopped in [...]/src/libafs/MODLOAD
    1 error

To avoid this, make all of our libafs objects depends on vnode_if.h.

[adeason@dson.org: Expanded commit message.]

Change-Id: I5a7a6ece8d5fbe6cf1a5b94451c8e8ae93fdc55f
Reviewed-on: https://gerrit.openafs.org/13983
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

5 months agotests: Run perl via 'env' 44/14144/2
Andrew Deason [Mon, 13 Apr 2020 01:16:55 +0000]
tests: Run perl via 'env'

The 'perl' binary may not be /usr/bin/perl, depending on the system.
For example, on modern FreeBSD it tends to be /usr/local/bin/perl
instead.

To avoid relying on perl to be in a specific location, just run via
/usr/bin/env instead, so we pick up perl from $PATH instead.

Change-Id: Ic8dc247c82342ff79dfa80426c489ccb8e3e1450
Reviewed-on: https://gerrit.openafs.org/14144
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

5 months agoFBSD: Remove LOCKPARENT/ISLASTCN lookup logic 78/12578/7
Tim Creech [Sun, 5 Mar 2017 23:15:58 +0000]
FBSD: Remove LOCKPARENT/ISLASTCN lookup logic

Currently, our afs_vop_lookup on FBSD tries to only lock 'dvp' for
ISDOTDOT requests when LOCKPARENT and ISLASTCN are set. There are a
couple of problems with this:

- The conditional locking logic involving LOCKPARENT/ISLASTCN is only
  relevant in very old FreeBSD releases (per-fs checking of these
  flags for parent locking went away around the FreeBSD 6 era).

- Our current logic here is wrong anyway, since we try to lock 'dvp'
  twice when those flags are set. This was mostly introduced by commit
  2f6be821 (FBSD: band-aid vnode locking in lookup), which added a
  lock/unlock pair for 'dvp' around the lock for 'vp', even though
  'dvp' was unlocked several lines earlier.

This means that if we hit the relevant code path, we will deadlock,
since we try to lock 'dvp' twice. To avoid this, just remove the
relevant logic for LOCKPARENT/ISLASTCN, since it is only relevant for
old FreeBSD releases that are not supported by us or FreeBSD.

Add and rearrange some comments around here to try to more explicitly
explain the relevant locking rules.

[adeason@dson.org: Commit message rewrite, adding comments, removing
old FreeBSD code.]

Change-Id: Iaa2c55d82c50d5a8ab42c67b0996a2b4fb6e09e6
Reviewed-on: https://gerrit.openafs.org/12578
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

5 months agoFBSD: Remove unused 'wantparent' logic 43/14143/2
Andrew Deason [Mon, 13 Apr 2020 03:40:14 +0000]
FBSD: Remove unused 'wantparent' logic

In afs_vop_lookup, the 'wantparent' variable doesn't actually change
any logic in the function. In the if() clause that it's used, the
value of 'wantparent' is only ever used if cnp->cn_nameiop is RENAME
and ISLASTCN is set. But if both of those are true, then the second
half of the if() conditional will always be true, so the value of
'wantparent' doesn't matter.

So to remove this confusing unused logic, remove the 'wantparent'
local var, and all its associated logic.

Issue spotted by kaduk@mit.edu.

Change-Id: Ia63b88d67d21cc2b81a0c25aa31ea60ab202b0a7
Reviewed-on: https://gerrit.openafs.org/14143
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

5 months agoFBSD: Add support for FreeBSD 11.3 92/13792/7
Andrew Deason [Mon, 19 Aug 2019 00:59:50 +0000]
FBSD: Add support for FreeBSD 11.3

Change-Id: Ibe3496f06da83a0b30182ea92081bae41fe766f3
Reviewed-on: https://gerrit.openafs.org/13792
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

5 months agoLINUX: Always crref after _settok_setParentPag 47/14147/2
Yadavendra Yadav [Wed, 15 Apr 2020 10:33:00 +0000]
LINUX: Always crref after _settok_setParentPag

Commit b61eac78 (Linux: setpag() may replace credentials) changed
PSetTokens2 to call crref() after _settok_setParentPag(), since
changing the parent PAG may change our credentials structure. But that
commit did not update the old pioctl PSetTokens, so -setpag
functionality remained broken on Linux for utilities that called the
old pioctl ('klog' is one such utility).

To fix this, we could copy the same code from PSetTokens2 into
PSetTokens. But instead just move this code into _settok_setParentPag
itself, to avoid code duplication. This commit also refactors
_settok_setParentPag a little to make the platform-specific ifdefs a
little easier to read through.

Change-Id: I65a165ebb1d823e690926de31b28a7728d2561b9
Reviewed-on: https://gerrit.openafs.org/14147
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Yadavendra Yadav <yadayada@in.ibm.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

5 months agoLINUX: Copy session keys to parent in SetToken 46/14146/5
Yadavendra Yadav [Wed, 15 Apr 2020 10:33:00 +0000]
LINUX: Copy session keys to parent in SetToken

Commit 48589b5d (Linux: Restore aklog -setpag functionality for kernel
2.6.32+) added code to SetToken() to copy our session keyring to the
parent process, in order to implement -setpag functionality. But this
was removed from SetToken() in commit 1a6d4c16 (Linux: fix aklog
-setpag to work with ktc_SetTokenEx), when the same code was moved to
ktc_SetTokenEx().

Add this code back to SetTokens(), so -setpag functionality can work
again with utilities that use older functions like ktc_SetToken, like
'klog'.

Change-Id: I68c9bf2e19783ea6f84b4c5ebf2ef188d1d8d6ad
Reviewed-on: https://gerrit.openafs.org/14146
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

5 months agoredhat: add make to the build requirements 19/14119/3
Michael Meffie [Fri, 20 Mar 2020 22:17:56 +0000]
redhat: add make to the build requirements

`make` is not necessarily installed, even if when all the other build
requirements are installed.

Add `make` to the list build requirements to complete the build
requirements. With this change it is possible to build the packages
after running the `yum-builddep` to install all of the needed build
requirements.

Change-Id: I032ba1f23d08468c5e21edc5662b20cc9498d1c9
Reviewed-on: https://gerrit.openafs.org/14119
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

5 months agovlserver: Correctly pad nvlentry for "O" RPCs 39/14139/2
Andrew Deason [Tue, 7 Apr 2020 18:15:31 +0000]
vlserver: Correctly pad nvlentry for "O" RPCs

For our old-style "O" RPCs (e.g. VL_CreateEntry, instead of
VL_CreateEntryN), vlserver calls vldbentry_to_vlentry to convert to
the internal 'struct nvlentry' format. After all of the sites have
been copied to the internal format, we fill the remaining sites by
setting the serverNumber to BADSERVERID. For nvldbentry_to_vlentry, we
do this for NMAXNSERVERS sites, but for vldbentry_to_vlentry, we do
this for OMAXNSERVERS.

The thing is, both functions are filling in entries for a 'struct
nvlentry', which has NMAXNSERVERS 'serverNumber' entries. So for
vldbentry_to_vlentry, we are skipping setting the last few sites
(specifically, NMAXNSERVERS-OMAXNSERVERS = 13-8 = 5).

This can easily cause our O-style RPCs to write out entries to disk
that have uninitialized sites at the end of the array. For example, an
entry with one site should have server numbers that look like this:

    serverNumber = {1, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255}

That is, one real serverid (a '1' here), followed by twelve
BADSERVERIDs.

But for a VL_CreateEntry call, the 'struct nvlentry' is zeroed out
before vldbentry_to_vlentry is called, and so the server numbers in
the written entry look like this:

    serverNumber = {1, 255, 255, 255, 255, 255, 255, 255, 0, 0, 0, 0, 0}

That is, one real serverid (a '1' here), followed by seven
BADSERVERIDs, followed by five '0's.

Most of the time, this is not noticeable, since our code that reads in
entries from disk stops processing sites when we encounter the first
BADSERVERID site (see vlentry_to_nvldbentry). However, if the entry
has 8 sites, then none of the entries will contain BADSERVERID, and so
we will actually process the trailing 5 bogus sites. This would appear
as 5 extra volume sites for a volume, most likely all for the same
server.

For VL_CreateEntry, the vlentry struct is always zeroed before we use
it, so the trailing sites will always be filled with 0. For
VL_ReplaceEntry, the trailing sites will be unchanged from whatever
was read in from the existing disk entry.

To fix this, just change the relevant loop to go through NMAXNSERVERS
entries, so we actually go to the end of the serverNumber (et al)
array.

This may appear similar to commit ddf7d2a7 (vlserver: initialize
nvlentry elements after read). However, that commit fixed a case
involving the old vldb database format (which hopefully is not being
used). This commit fixes a case where we are using the new vldb
database format, but with the old RPCs, which may still be used by old
tools.

Change-Id: Ic6882d1452963ca93403748917c313068acfdaab
Reviewed-on: https://gerrit.openafs.org/14139
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

5 months agoredhat: fix rpmbuild warnings 18/14118/3
Michael Meffie [Fri, 20 Mar 2020 21:53:22 +0000]
redhat: fix rpmbuild warnings

Fix warnings issued by recent versions of rpmbuild:

    warning: Macro expanded in comment on line 110: %{afsvers}/...
    warning: extra tokens at the end of %endif directive in line 1469:
             %endif  # build_userspace
    warning: line 331: It's not recommended to have unversioned Obsoletes:
             Obsoletes: openafs-client-compat

The first two warnings are just issues with comments, which apparently
are not completely ignored by rpmbuild.  The third issue is a warning
about an unversioned "Obsoletes" directive. Remove the old Obsoletes for
openafs-client-compat, which was obsoleted no later than the 1.4.x
series (more than 10 years ago).

While here clean up the spec by removing the old cvs $Revsion$ keyword
from the comments at the top of the file, and removing an old commented
out setup directive.

Change-Id: I8d7a050ea6a0cc7a2d9a6af9a91d25ce545586e7
Reviewed-on: https://gerrit.openafs.org/14118
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

5 months agoopr: Allow non-2^x for n_buckets in opr_cache_init 22/14122/3
Andrew Deason [Mon, 30 Mar 2020 19:21:21 +0000]
opr: Allow non-2^x for n_buckets in opr_cache_init

Currently, opr_cache_init requires that opts->n_buckets is a power of
2 (since our underlying opr_dict requires this). However, callers may
want to pick a number of buckets based on some other value. Requiring
each caller to calculate the nearest power-of-2 is annoying, so
instead just have opr_cache_init itself calculate a nearby power of 2.

That is, with this commit, opts->n_buckets is allowed to not be a
power of 2; when it's not a power of 2, opr_cache_init will calculate
the next highest power of 2 and use that as the number of buckets.

Change-Id: Icd3c56c1fe0733e3dac964ea9a98ff7b436254e6
Reviewed-on: https://gerrit.openafs.org/14122
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

5 months agolibafs: Serialize INSTDIRS/DESTDIRS and COMPDIRS 37/14137/2
Andrew Deason [Sun, 5 Apr 2020 21:29:52 +0000]
libafs: Serialize INSTDIRS/DESTDIRS and COMPDIRS

Our libafs build logic involves a few targets that 'cd' into a
per-kernel subdir: notably INSTDIRS and DESTDIRS (the targets to 'make
install' or 'make dest' our kernel modules) and COMPDIRS (the target
to setup/build the kernel module).

Both of these potentially 'cd' into a subdirectory (e.g. MODLOAD64),
and run some make rules. Since INSTDIRS and COMPDIRS are different
targets and don't depend on each other for many platforms, running
those rules can happen in parallel. After they 'cd' into the relevant
dir, they run a new 'make' in a subshell, and so underlying rules for
building e.g. AFS_component_version_number.c are not serialized.

So for a parallel build on, say, Solaris, we can encounter errors when
two sub-makes try to make AFS_component_version_number.c at the same
time, which looks something like this (with various lines output from
other sub-processes mixed in):

    cd src && cd sys && gmake install
    gmake[3]: Leaving directory '/[...]/src/libuafs'
    rm -f AFS_component_version_number.c.NEW
    /opt/developerstudio12.6/bin/cc [...] -D_KERNEL -DSYSV -dn -m64 -xmodel=kernel -xvector=%none -xregs=no%float  -Wu,-save_args  -o AFS_component_version_number.o -c AFS_component_version_number.c
    mv: cannot access AFS_component_version_number.c.NEW
    gmake[4]: *** [/[...]/src/config/Makefile.version:13: AFS_component_version_number.c] Error 2
    gmake[4]: Leaving directory '/[...]/src/libafs/MODLOAD64'
    gmake[3]: *** [Makefile:85: solaris_instdirs] Error 2
    gmake[3]: *** Waiting for unfinished jobs....

To avoid this, just make INSTDIRS and DESTDIRS depend on COMPDIRS, so
we can make sure they don't run at the same time.

Change-Id: I2510e1894c44dd0864cf2eab5613b805342b6718
Reviewed-on: https://gerrit.openafs.org/14137
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

5 months agobutc: rename local var tapeblocks to numTapeblocks 28/14128/3
Cheyenne Wills [Wed, 1 Apr 2020 15:38:05 +0000]
butc: rename local var tapeblocks to numTapeblocks

The local variable tapeblocks in GetConfigParams matches a global
variable.  Rename the local variable to avoid confusion with the global
name.

Change-Id: I1c30433696a35a74978ef0c23881c82054b416c5
Reviewed-on: https://gerrit.openafs.org/14128
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

5 months agobuild: remove unused LINUX_PKGREL from configure.ac 17/14117/2
Michael Meffie [Mon, 23 Mar 2020 13:46:05 +0000]
build: remove unused LINUX_PKGREL from configure.ac

This change removes the unused LINUX_PKGREL definition from the
configure.ac file.

Commit 6a27e228bac196abada96f34ca9cd57f32e31f5c converted the setting of
the RPM package version and release values in the openafs.spec file from
autoconf to the makesrpm.pl script. That commit left LINUX_PKGREL in
configure.ac because it was still referenced by the Debian packaging,
which was still in-tree at that time.

Commit ada9dba0756450993a8e57c05ddbcae7d1891582 removed the last trace
of the Debian packaging, but missed the removal of the LINUX_PKGREL.

Change-Id: I17aeccdb38078faa413f2cd3a935b43238982606
Reviewed-on: https://gerrit.openafs.org/14117
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>

5 months agovos: Print "done" in non-verbose 'vos remsite' 27/14127/2
Andrew Deason [Thu, 2 Apr 2020 03:59:38 +0000]
vos: Print "done" in non-verbose 'vos remsite'

Currently, 'vos remsite' always prints the message "Deleting the
replication site for volume %lu ...", and then calls VDONE if the
operation is successful. VDONE prints the trailing "done", but only if
-verbose is turned on, and so if -verbose is not specified, the output
of 'vos remsite' looks broken:

    $ vos remsite fs1 vicepa vol.foo
    Deleting the replication site for volume 1234 ...Removed replication site fs1 /vicepa for volume vol.foo

To fix this, unconditionally print the trailing "done", instead of
going through VDONE, so 'vos remsite' output now looks like this:

    $ vos remsite fs1 vicepa vol.foo
    Deleting the replication site for volume 1234 ... done
    Removed replication site fs1 /vicepa for volume vol.foo

Change-Id: I0b42f4cb9b695331bf047243bf6ae4a1cdbb89c4
Reviewed-on: https://gerrit.openafs.org/14127
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

5 months agoAvoid duplicate definitions of globals 06/14106/4
Cheyenne Wills [Wed, 1 Apr 2020 15:48:57 +0000]
Avoid duplicate definitions of globals

GCC 10 changed a default flag from -fcommon to -fno-common.  See
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85678 for some background.

The change in gcc 10 results in build link-time errors. For example:
   ../../src/xstat/.libs/liboafs_xstat_cm.a(xstat_cm.o):(.bss+0x2050):
       multiple definition of `numCollections';

Ensure that only one definition for global data objects exist and change
references to use "extern" as needed.

To ensure that future changes do not introduce duplicated global
definitions, add the -fno-common flag to XCFLAGS when using the
configure --enable-checking setting.

Change-Id: I6780dd995fe6fb6c2102765ff3484c18e1e1cd58
Reviewed-on: https://gerrit.openafs.org/14106
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

5 months agovos: Properly print volume transaction flags 26/14126/3
Andrew Deason [Wed, 1 Apr 2020 02:19:18 +0000]
vos: Properly print volume transaction flags

Currently, the code in 'vos status' treats the 'iflags' and 'vflags'
of a transaction like an enumerated type; that is, we only check if
'iflags' is equal to ITOffline or ITBusy, etc. But both of these flags
fields are bitfields; any combination of the relevant flags could
theoretically be set.

Practically speaking, we only ever set at most one of the flags in
'iflags', but if anything ever did set more than one flag, our output
would look broken (we'd print "attachFlags:" without any flags).

For 'vflags', multiple flags are often set at once: the most common
combination is VTDeleteOnSalvage|VTOutOfService. So currently, we
usually print "attachFlags:" without any actual flags, since the
'vflags' field isn't exactly equal to VTDeleteOnSalvage (instead it's
set to VTDeleteOnSalvage|VTOutOfService). And if we ever did see just
VTDeleteOnSalvage set by itself, the way the switch() cases fall
through to each other, we'd print out that _all_ flags are set.

To fix all of this, just test for the individual flag bits instead.

Change-Id: Ib4d207bc713f0ef8eb51b9dbeaf2af50395536ee
Reviewed-on: https://gerrit.openafs.org/14126
Tested-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

5 months agoLINUX: Introduce afs_d_path 21/13721/3
Andrew Deason [Tue, 23 Jul 2019 18:50:31 +0000]
LINUX: Introduce afs_d_path

Move our preprocessor logic around d_path into an osi_compat.h
wrapper, called afs_d_path. This just makes it a little easier to use
d_path, and moves a tiny bit of #ifdef cruft away from real code.

Change-Id: I2032eda3fef18be6e77e3bf362ec5ce641e1d76d
Reviewed-on: https://gerrit.openafs.org/13721
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

5 months agoafs: Detect VIOCPREFETCH special case properly 01/13301/6
Andrew Deason [Fri, 24 Aug 2018 18:03:24 +0000]
afs: Detect VIOCPREFETCH special case properly

Currently, afs_syscall_pioctl handles the VIOCPREFETCH pioctl as a
special case, calling into a different code path to handle
backgrounding the prefetch operation. However, we detect that we're
handling a VIOCPREFETCH operation just by looking at the lower 8 bits
of the given opcode. This means that any pioctl that ends in 0x0F will
trigger this codepath, such as if we add a 'C' or 'O' pioctl that uses
code 0x0F.

We only want to catch VIOCPREFETCH requests for this code path, so fix
the check to also check if we're processing a 'V' pioctl.

Change-Id: Ica8c2364f96aa3c8b4d2213bebd9a1e4cb6fa730
Reviewed-on: https://gerrit.openafs.org/13301
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

5 months agotests: Wait for server start in auth/superuser-t 09/14109/4
Andrew Deason [Tue, 24 Mar 2020 16:59:48 +0000]
tests: Wait for server start in auth/superuser-t

The auth/superuser-t test runs an Rx server and client in two child
processes. If the client process tries to contact the server before
the server has started listening on its port, some tests involving
RPCs can fail (notably test 39, "Can run a simple RPC").

Normally if we try to contact a server that's not there, Rx will try
resending its packets a few times, but on Linux with AFS_RXERRQ_ENV,
if the port isn't open at all, we can get an ICMP_PORT_UNREACH error,
which causes the relevant Rx call to die immediately with
RX_CALL_DEAD.

This means that if the auth/superuser-t client is only just a bit
faster than the server starting up, tests can fail, since the server's
port is not open yet.

To avoid this, we can wait until the server's port is open before
starting the client process. To do this, have the server process send
a SIGUSR1 to the parent after rx_Init() is called, and have the parent
process wait for the SIGUSR1 (waiting for a max of 5 seconds before
failing). This should guarantee that the server's port will be open by
the time the client starts running.

Note that before commit 086d1858 (LINUX: Include linux/time.h for
linux/errqueue.h), AFS_RXERRQ_ENV was mistakenly disabled on Linux
3.17+, so this issue was probably not possible on recent Linux before
that commit.

Change-Id: I0032a640b83c24f72c03e7bea100df5bc3d9ed4c
Reviewed-on: https://gerrit.openafs.org/14109
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>

6 months agoLINUX: Clear lock 'pid' fields with NULL 08/14108/2
Andrew Deason [Tue, 24 Mar 2020 16:34:51 +0000]
LINUX: Clear lock 'pid' fields with NULL

Currently, when we release a lock, we set the e.g. pid_writer field to
0, to clear out any previous pid that was set. On Linux, the
pid_writer field is a pointer, and sparse(1) complains about using a
plain integer 0 in this way:

      CHECK   [...]/afs_axscache.c
    [...]/afs_axscache.c:24:19: warning: Using plain integer as NULL pointer
    [...]/afs_axscache.c:68:9: warning: Using plain integer as NULL pointer
    [...]/afs_axscache.c:88:5: warning: Using plain integer as NULL pointer
    [...]/afs_axscache.c:111:13: warning: Using plain integer as NULL pointer
    [...]/afs_axscache.c:121:17: warning: Using plain integer as NULL pointer
    [...]/afs_axscache.c:126:17: warning: Using plain integer as NULL pointer
    [...]/afs_axscache.c:154:13: warning: Using plain integer as NULL pointer
    [...]/afs_axscache.c:165:9: warning: Using plain integer as NULL pointer

This doesn't break anything, but it spews out quite a lot of warnings
when building with sparse(1) available. To just reduce this noise a
bit, assign these fields to actual NULL.

Since some other platforms do use a plain integer in these fields
(they are an actual pid), define 'MyPid_NULL' to use '0' or 'NULL'
depending on the platform. Define MyPid_NULL to NULL only on Linux;
this causes us to still assign 0 to a pointer on some platforms, but
Linux is the only one that complains, so only bother using NULL on
Linux for now.

Change-Id: I35fcb896ceaa346c330622cfc2913b2975295836
Reviewed-on: https://gerrit.openafs.org/14108
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

6 months agorxgen: Properly generate brief union default arm 07/14107/2
Andrew Deason [Tue, 10 Mar 2020 21:05:47 +0000]
rxgen: Properly generate brief union default arm

Commit 13ae3de3 (Add "brief" option to rxgen) added the -b option to
rxgen, which (among other things) makes rxgen stop including the name
of an RPC-L union type within its fields. That is, instead of this:

    struct foo_type {
        afs_int32 foo_tag;
        union {
            /* ... */
        } foo_type_u;
    };

rxgen -b generates this:

    struct foo_type {
        afs_int32 foo_tag;
        union {
            /* ... */
        } u;
    };

And all of the autogenerated XDR code is altered to use the 'u' field
instead of foo_type_u. However, if a 'default:' arm is defined in the
definition for the RPC-L union, the autogenerated XDR code still tries
to reference the non-brief name (e.g. foo_type_u). This causes a build
failure when actually trying to compile the generated .xdr.c, like so:

    foo.xdr.c:809:39: error: 'foo_type' has no member named 'foo_type_u'
        if (!xdr_bytes(xdrs, (char **)&objp->foo_type_u.xxx, &__len, FOO_MAX)) {
                                           ^
    foo.xdr.c:812:11: error: 'foo_type' has no member named 'foo_type_u'
        *(&objp->foo_type_u.xxx) = __len;

This happens because the portion of emit_union() that generates the
XDR code for the default arm wasn't updated to use a different
formatting string when 'brief_flag' is set, like the rest of
emit_union.

To fix this, just check for brief_flag and use 'briefformat'
accordingly, like the other code that checks for brief_flag.

Currently nothing in the tree uses the default arm of RPC-L unions
with 'rxgen -b', but external callers could, or our future code may do
so.

Change-Id: Ifcebfc48a3a64c68fee12ba0d177ae19b0956c58
Reviewed-on: https://gerrit.openafs.org/14107
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

6 months agoubik: death to SVOTE_GetSyncSite 43/14043/10
Marcio Barbosa [Thu, 27 Feb 2020 22:28:14 +0000]
ubik: death to SVOTE_GetSyncSite

The SVOTE_GetSyncSite RPC was intended to provide the IP address of the
current sync-site. Unfortunately, the RPC-L incorrectly defined ahost as
an input argument instead of an output argument. As a result, the IP
address in question is not returned to the callers of SVOTE_GetSyncSite.
Moreover, calls to this RPC must be made through connections associated
with the VOTE_SERVICE_ID. Sadly, the ubik_Call* functions call
SVOTE_GetSyncSite using connections associated with the USER_SERVICE_ID.
Consequently, the server getting this request returns RXGEN_OPCODE,
meaning that this RPC is not implemented by the service in question.

Since RPC arguments cannot be changed without causing compatibility
issues between different client / server versions and the RPC in
question is being called through the wrong service id, remove
SVOTE_GetSyncSite and its callers. Considering that in all versions of
OpenAFS calls to this RPC always return RXGEN_OPCODE, no behavior
change is introduced by this commit.

Also, remove the "chaseCount logic" from the ubik_Call* functions.
This logic prevents the loop counter from being moved backwards
indefinitely, resulting in an infinite loop. Fortunately, without the
VOTE_GetSyncSite() calls this counter cannot be moved backwards more
than once.

Change-Id: Idd071583e8f67109e003f7a5675de02a235e5809
Reviewed-on: https://gerrit.openafs.org/14043
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

6 months agotests: Add cache-t to .gitignore in tests/opr 02/14102/2
Cheyenne Wills [Fri, 20 Mar 2020 18:03:48 +0000]
tests: Add cache-t to .gitignore in tests/opr

Commit 48fbb45 (opr: Introduce opr_cache) added a new test (cache-t),
but did not update the .gitignore file for it.

Change-Id: I6de6130257a62f495ac942c05937eb109ce84a75
Reviewed-on: https://gerrit.openafs.org/14102
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

6 months agotests: Add core to .gitignore in tests 01/14101/2
Cheyenne Wills [Fri, 20 Mar 2020 17:54:23 +0000]
tests: Add core to .gitignore in tests

opr/softsig-t can produce a core file as part of its test.

Change-Id: I3bc7e587151e5915038e31887018889a7ffa6993
Reviewed-on: https://gerrit.openafs.org/14101
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>