Signals and pthreaded applications are a poor match. OpenAFS has had
the softsig system (currently in src/util/softsig.c) in an attempt to
alleviate some of these problems. However, that implementation itself
has a number of problems. It uses signal functions that are unsafe in
pthreaded applications, and uses pthread_kill within its signal
handlers. Over the years it has been responsible for a number of
portability bugs.
The old implementation continues to receive signals in the main thread
of the application. However, the handler code is run within a seperate
signal handler thread. When the main thread receives a signal a stub
handler is invoked, which simply pthread_kill()s the signal handler
thread.
The new implementation simplifies things by only receiving signals in
the handler thread. It uses only pthread-compatible signal functions,
and invokes no code from within async signal handlers.
A complete test suite is supplied.
Change-Id: I4bac68c2f853f1e7578b54ddced3833a97dd3f82
Reviewed-on: http://gerrit.openafs.org/6947
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Chas Williams <3chas3@gmail.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
The current version of BreakUpPath is slightly broken, since
commit 4e68282e26 -- it has two
output parameters but takes only one length parameter for the
size of the output buffers passed in. The callers ended up using
the shorter of the buffer lengths in question, so there is not
a risk of a buffer overrun, but long paths would not be properly
handled.
There is not really any need to pass in a length at all, since
what is going on is conceptually strdup, and there is no real
need to use strlcpy at all. Make the change from strlcpy to
str(n)dup, and adjust callers to free the outputs as appropriate.
While here, convert writeFile() to use goto and a cleanup handler
to avoid leaks.
Change-Id: Ib742cb73a6d70aa863c8d30423416887b977677b
Reviewed-on: http://gerrit.openafs.org/11874
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
We no longer have to define a ->write operation, and we can't
expect the underlying disk cache filesystem to have one. Use
the new __vfs_read/write helpers that will select the operation
to use based on what's available for that particular filesystem.
Change-Id: Iab923235308ff57348ffc2dc6d718dd64040656b
Reviewed-on: http://gerrit.openafs.org/11849
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Chas Williams <3chas3@gmail.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Make the test here a bit more specific. do_sync_read no longer
exists, but we don't use it for new kernels. Trying to define it
here in terms of generic_file_read is not helpful as that doesn't
exist anymore.
Change-Id: Iffb059716165436c3439e66db15002cdec5dfc16
Reviewed-on: http://gerrit.openafs.org/11848
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Chas Williams <3chas3@gmail.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Commit 5b3c104296 changed/removed some
uses of abs() on unsigned time values. While the previous use of abs()
was indeed incorrect, the result wasn't necessarily much better, even
though it built with recent compilers, since it only checked for skew
in one direction.
Define and use a macro to correctly evaluate the conditionals in 64-bit
precision, avoiding C's integer promotion rules which prefer unsigned types
(Date) to signed types of the same width (time_t on 32-bit systems).
Change-Id: Ifcbe59e73942a52a8635cb0f43cce94fdeea85a3
Reviewed-on: http://gerrit.openafs.org/11850
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
afsd.c starts with a block comment listing the flags supported by the
afsd command. As the code has evolved this list has not been kept up
to date. Bring the list up to date. Some obsolete options no longer
have any backing code. These are marked OBSOLETE. Some obsolete
options have code that says they are now deprecated. These are
marked IGNORED.
Additionally fix a typo in backuptree's help text.
Change-Id: I90ddf4db826c891bf1daf6959f94feee17d35f78
Reviewed-on: http://gerrit.openafs.org/11857
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Remove several solaris specific headers from afs_util.c which are no
longer needed.
Change-Id: Id3874c90448215dc506b7ab9b5e695c2aeef50f3
Reviewed-on: http://gerrit.openafs.org/11856
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
This change reworks numerous places which formerly used potentially
large on-stack buffers (of size AFSDIR_PATH_MAX) for constructing or
storing pathnames. Instead, these buffers are now allocated from the
heap, either by using asprintf() to build a pathname in a correctly
sized buffer or, where necessary, using malloc() to allocate a buffer
of size AFSDIR_PATH_MAX.
A few occurrances of AFSDIR_PATH_MAX-sized buffers are not changed;
these are generally either globals or are contained within another
data structure that is already allocated on the heap.
[kaduk@mit.edu convert to cleanup-handler memory management where
appropriate]
Change-Id: Ib1986187a1c467e867d50280aaf1d8a86d9108c8
Reviewed-on: http://gerrit.openafs.org/9985
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
volume.c defined its own ffs() macro if AFS_HAVE_FFS was not
defined. Now that ffs() is in opr, just use it from there always.
Change-Id: Ia80a439924541be236b3221b9480143b511c885a
Reviewed-on: http://gerrit.openafs.org/11674
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Linux 2.4 has been desupported by OpenAFS.
Change-Id: I7803d718ca9d2760a799f1ac2c438f8e6df8b9b9
Reviewed-on: http://gerrit.openafs.org/11806
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Chas Williams <3chas3@gmail.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Tested-by: Daria Brashear <shadow@your-file-system.com>
Linux 2.4 and older are no longer supported; there's no need to
keep the noise in this file.
Change-Id: Ia1a968e0cdc1180e66bdedd0aa0638eadfd897fb
Reviewed-on: http://gerrit.openafs.org/11805
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Chas Williams <3chas3@gmail.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Tested-by: Daria Brashear <shadow@your-file-system.com>
They are no longer triggerable now that linux 2.4 is desupported,
so make the code easier to read.
Change-Id: I77b48d30db66b635cfdc06e977f9884dd2825713
Reviewed-on: http://gerrit.openafs.org/11800
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Chas Williams <3chas3@gmail.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Tested-by: Daria Brashear <shadow@your-file-system.com>
The Linux 2.4 series (and older) will not be supported platforms
for OpenAFS 1.8 and later. Detect these systems at configure time
and direct users of those systems to the OpenAFS 1.6 series of releases.
These systems are believed to not be in common use with OpenAFS,
and retaining support for the LinuxThreads threading implementation
they require presents a maintenance burden that the project is
not equipped to deliver. The project will be able to move forward
more quickly by desupporting these systems.
Code conditional on these old systems can be removed in subsequent
commits.
Change-Id: I679fc2390b35851f3b0457a846047c812bc03dba
Reviewed-on: http://gerrit.openafs.org/11799
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Chas Williams <3chas3@gmail.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Tested-by: Daria Brashear <shadow@your-file-system.com>
hcrypto depends on roken, so roken must come after it.
Change-Id: Ic63de1f9095555a6c3e83f2f6d2f9d024ad00006
Reviewed-on: http://gerrit.openafs.org/11743
Reviewed-by: Chas Williams <3chas3@gmail.com>
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Introduce the vos remaddrs sub-command for removing multi-homed server
entries from the vldb. The remaddrs sub-command completes the listaddrs
and setaddrs command suite and allows vos changeaddr to be deprecated
completely.
Change-Id: I98e92e776a153591a617a5b04037c3b6139d4732
Reviewed-on: http://gerrit.openafs.org/11606
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Chas Williams <3chas3@gmail.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Partially revert commit e2e93aa892,
which added local declarations for des_init_mutex, des_random_mutex,
and rxkad_random_mutex to a number of files in libadmin, apparently
to fix the build on macos 10.3. That OS is long EoL-ed, and
more recent versions of OS X include toolchains that do not
need these extra declarations. In particular, the extra declarations
can be harmful when these files start to pull in more symbols
from our libraries (e.g., libafscp), since the details of the
linking process can cause that to generate duplicate symbol errors.
There is no longer any need to have local declarations of these
symbols for OS X, so just remove them.
Change-Id: Ie152387b4bd16b470054821fc8ddf852e1a4285c
Reviewed-on: http://gerrit.openafs.org/11798
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
When copying into the 'cell' array, use the size of cell, not
the size of some other array that is copied into a few lines previously.
Change-Id: Ib8b523901dd8008038c5a95a7c315b899cff8cee
Reviewed-on: http://gerrit.openafs.org/11807
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Commit b9fb9c62a6 refactored lpioctl()
so that LINUX would have its own implementation. This also simplified
the other lpioctl() implementations by removing superfluous variable
'rval'.
Unfortunately, 'rval' was actually required for both DARWIN and SUN511.
On both of these platforms, the address of 'errcode' is passed
to the respective ioctl_*() routine so its value may be passed back
to lpioctl(). Therefore, 'errcode' must not also be used for the
return value from these functions; doing so results in the return
value from the function overwriting the intended value of 'errcode' upon
return to lpioctl().
In the case of Solaris 11, ioctl_sun_afs_syscall() always returns zero
(as long as the ioctl device 'dev/afs' opened successfully).
So 'errcode' was always being set to zero, even if the pioctl had
actually failed. For example, without this fix, 'fs listcells'
loops forever on Solaris 11, listing an infinite number of "cells",
because it will never "see" the EDOM that informs it of the last defined
cell.
Partially revert b9fb9c62a6 by restoring
the 'rval' variable and logic for DARWIN and SUN511.
Change-Id: I4407af29d54813689cf8ccf2517bb2df4dd8eb25
Reviewed-on: http://gerrit.openafs.org/11734
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
We get linker conflicts if we try to statically link against the
system libkrb5 when it is heimdal, from both hcrypto and rfc3961.
While here, add an include guard to hcrypto's config.h.
Change-Id: Ib5fcd9291b295415325a4ed230bd35d496961367
Reviewed-on: http://gerrit.openafs.org/11740
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
abs(3) is a function of one variable of type int returning int.
labs(3) is a function of one variable of type long returning long.
labs(3) should be used when the input is of type long, as in
kaprocs.c.
Calling anything from the abs(3) family on a variable of unsigned
type is a bogus type pun, and a logical operation which is a no-op.
(Unsigned values are never negative and thus the absolute value
function is the identity over the entire range of values representable
in an unsigned type.) Just remove the use of abs() for unsigned
values, as in kaprocs.c, krb_udp.c, and vldb_check.c
While in kaprocs.c, wrap a long line that was touched for the
conversion to labs(3), spell the argument to time(3) as NULL
instead of 0, remove unneeded parentheses, and correct the spelling
of "reserved".
Change-Id: I0897b250fd885a1230d1622015eec9afe3450b46
Reviewed-on: http://gerrit.openafs.org/11745
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
clang 3.5 is more aggressive about these checks than the previous
FreeBSD system compiler, so new warnings (which became errors)
appeared on FreeBSD 11-CURRENT.
In afs_dcache.c, checking &tdc->f for NULL-ness has no effect.
The struct fcache f member of struct dcache is an ordinary structure
element; its address will be the value of tdc plus the offset of
f within struct dcache, which will not be NULL even if tdc is NULL.
In ubik_db_if.c, udbHandle is a file-scope global and thus has
allocated storage; the address of a member variable will never
be NULL. The 0 it was compared against was spelled RX_SECIDX_NULL,
which shows the intended check, which is for the value of the
uh_scIndex member variable, not its address.
In afscp_server.c, srv->conns can never be NULL since conns is a member
variable of struct afscp_server (of array type, containing pointers
to struct rx_connection). Comparing the array member variable against
NULL is comparing the address of the array, which is never NULL since
it is not allocated separately from struct afscp_server.
In fssync-debug.c, state.vop->partName is never NULL because
common_volop_prolog always allocates for state.vop, and the
partName member variable of struct fssync_state is of array type,
and thus is not separately allocated from the containing structure.
Change-Id: I03e1332d8a3320f1a4d303b444985648a207116e
Reviewed-on: http://gerrit.openafs.org/11739
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Update all remaining copies of CellServDB in the tree, and make the
Red Hat packaging use it by default too.
Change-Id: Id915a82b1364ef6e301921e4d3873c7203aef91c
Reviewed-on: http://gerrit.openafs.org/11764
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Fix a long standing bug in the ChangeAddr RPC which damages the vldb,
When vos changeaddr is run with -oldaddr and -newaddr, and the -oldaddr
is present in an multi-homed entry, instead of changing the address in
the mh entry, the server slot is "downgraded" to a single homed entry
and the mh entry is orphaned in the vldb.
Instead, if the -oldaddr is in a multi-home entry, refuse to change the
address with a VL entry not found error and log the event.
Multi-homed addresses can be changed manually using the vos setaddrs
command which calls the RegisterAddrs() RPC.
Change-Id: I20ba3e7fa5ffdb1b1abd2e2a716d8e4bb6594542
Reviewed-on: http://gerrit.openafs.org/11639
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
If a DISK_UpdateInterfaceAddr RPC is received the server that sent
it restarted. Force the urecovery code to verify the server state.
Change-Id: I465863dc3a52d844b56d576bd55229435556cfd6
Reviewed-on: http://gerrit.openafs.org/11738
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Reviewed-by: Jeffrey Hutzelman <jhutz@cmu.edu>
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
When processing an DISK_Begin RPC verify that there is an active quorum
and that the local database is current. Otherwise, fail the RPC with
a UNOQUORUM error.
The returned error must be UNOQUORUM instead of USYNC becase the returned
error code will be returned by the coordinator's ContactQuorum_iterate()
to the client that triggered the write transaction. Most ubik clients
will only retry if the error is UNOQUORUM.
FIXES 131997
Change-Id: Icaa30e6aca82e7e7d33e9171a4f023970aba61df
Reviewed-on: http://gerrit.openafs.org/11689
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Jeffrey Hutzelman <jhutz@cmu.edu>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
The backing_dev_info is only stored in the super_block now.
Change-Id: I57e147100bd47a8d1f5e97224ceb3322ea102a48
Reviewed-on: http://gerrit.openafs.org/11756
Reviewed-by: Marc Dionne <marc.c.dionne@gmail.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
In an age where Linux version numbers are determined by Google+ polls,
it’s clear that they aren’t going to be very useful for marking major
API compatibility boundaries like they were in the days of 2.2/2.4.
Change-Id: I56e0e88eb178573c3eb280d5a5a01d8b8a20a363
Reviewed-on: http://gerrit.openafs.org/11755
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Reviewed-by: Marc Dionne <marc.c.dionne@gmail.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Even though we hope no one will actually ever need to use it, explain
this configure switch in the same place as the others.
Change-Id: Ib59f963b9000c3e66dc84c7b18eb220f0e108bd5
Reviewed-on: http://gerrit.openafs.org/11749
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Preserve the volume clone and backup ids in the volume header when
restoring over an existing volume, instead of always setting the clone
and backup ids to zero.
For example, before this change, restoring over a volume resets the
ROnly and Backup ids reported in the volume header section of vos
examine.
$ vos examine xyzzy
xyzzy 536871023 RW 3 K On-line
myhost /vicepa
RWrite 536871023 ROnly 536871024 Backup 536871025
...
RWrite: 536871023 ROnly: 536871024 Backup: 536871025
number of sites -> 2
server myhost partition /vicepa RW Site
server myhost partition /vicepa RO Site
$ cat /tmp/xyzzy.dump | vos restore myhost a xyzzy -overwrite incremental
Restoring volume xyzzy Id 536871023 on server myhost partition /vicepa .. done
Restored volume xyzzy on myhost /vicepa
$ vos examine xyzzy
xyzzy 536871023 RW 3 K On-line
myhost /vicepa
RWrite 536871023 ROnly 0 Backup 0
...
RWrite: 536871023 ROnly: 536871024 Backup: 536871025
number of sites -> 2
server myhost partition /vicepa RW Site
server myhost partition /vicepa RO Site
Change-Id: If7ffcf84a983046c10d9d215d672a6a1ba1f9400
Reviewed-on: http://gerrit.openafs.org/11516
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
In older versions of OpenAFS (prior to 2001), the backupDate was
never set. Try to provide somewhat more reasonable behavior in
this case, by using a different date in that case.
Change-Id: Id74ce003c6a2317b06e78ba64d6891229c16421a
Reviewed-on: http://gerrit.openafs.org/11627
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
You're not supposed to write the length of the submitted data on the
split rx stream for a StoreData operation; the fileserver knows how
much data to read from the "Length" parameter of the StoreData RPC.
For a FetchData, putting the data length over the split rx stream is
required, since we can't get the "OUT" arguments before reading the
file data. But for a StoreData, this is unnecessary, since the length
is right there in the arguments.
So just get rid of this commented-out code; it's clearly wrong and
this commit explains why.
Change-Id: Idde0f9079e90da75d71a142f4a9f36a84ce79776
Reviewed-on: http://gerrit.openafs.org/10786
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Currently, if a server RPC hangs forever, the client call will error
out with RX_CALL_TIMEOUT (if idle/dead timeouts are configured). If we
later try to make a new call on that conn, the server will respond
with BUSY packets, and we'll have to wait until we RX_CALL_TIMEOUT
again. After that we'll set lastBusy and avoid the call channel, but
that extra delay with the BUSY packets is avoidable.
So, avoid this extra delay by setting lastBusy when we kill a call
with RX_CALL_TIMEOUT, so a future rx_NewCall will avoid the call
channel. It makes sense to set lastBusy here, since the call channel
is more likely to be busy than the other call channels.
Change-Id: Iadf77f52ae418491e3108a4b0b5388361f2424aa
Reviewed-on: http://gerrit.openafs.org/10785
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Commit 23d6287f7f introduced the
behavior where a client can immediately retry a call if it receives a
"busy" packet from the server (meaning, the call channel is already in
use). This happened via Rx returning the error code RX_CALL_BUSY, and
the caller was supposed to immediately retry the call, so Rx could
reissue the RPC on a different call channel.
However, this behavior makes it more likely for the server to process
an RPC that the client thinks has not been processed. Say the client
issues an RPC, the server replies with a "busy" packet, and the client
resends the original packet before it sees the "busy" packet. In
this case, the server will get the resent packet for the RPC request
and process it, but the client will think the call has failed (and
presumably will retry the call on a new channel). For calls that are
non-idempotent (e.g. MakeDir), this can result in incorrect errors
(e.g. EEXIST) as well as incorrect cache state in the client.
There may be some ways to mitigate at least some of the problems here,
but this kind of "instant" retry behavior is often not really that
helpful. Calls that take a very long time to run on the server are
very rare (and usually indicate some other problem), while the
occasional short-lived "busy" packet is relatively common (sometimes
the server just hasn't cleaned up the call by the time we issue a new
call). So just get rid of the retrying behavior to ensure we don't
continue to encounter any problems like this.
To get rid of this behavior, we remove the RX_CALL_BUSY code, and all
code dealing with processing it. This means removing the RX_CALL_BUSY
handling from the client, as well as removing
rx_SetBusyChannelError(). This effectively reverts most of
23d6287f7f, and a few other commits
related to RX_CALL_BUSY.
With this change, if all we get from the server are BUSY packets when
we try to issue an RPC, the call will eventually error out with
RX_CALL_TIMEOUT (or hang forever, if no timeouts are configured). This
can be thought of intuitively as similar to "idle dead" behavior,
since we are just waiting for the server to proceed with processing
the call. So, if "idle dead" is configured, we still timeout after the
"idle dead" timeout. And if no idle or hard dead timeout is
configured, we will hang forever; just like if the server started
processing the call but then hangs forever.
Note that not all of 23d6287f7f is
reverted. Namely, the logic to have rx_NewCall try to pick the "least
busy" channel is retained.
Thanks to Simon Wilkinson for bringing up and discussing this issue in
this thread:
<http://thread.gmane.org/gmane.comp.file-systems.openafs.devel/10931>
<https://lists.openafs.org/pipermail/openafs-devel/2013-April/019297.html>
Change-Id: I272e51f252356aa14bc4b8a3b7c594700deb432c
Reviewed-on: http://gerrit.openafs.org/10784
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
After change Ie0497d24f1bf4ad7d30ab59061f96c3298f47d17, RX_CALL_IDLE
is not generated by Rx anymore; "idle dead" timeouts just cause
RX_CALL_TIMEOUT errors. Any code dealing with it is thus now dead code
(this value was deliberately never sent over the wire), so remove the
dead code.
Change-Id: I2b38327f77ffc8168712b83506afa1da3eea1224
Reviewed-on: http://gerrit.openafs.org/10783
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
After change Ie0497d24f1bf4ad7d30ab59061f96c3298f47d17,
testing for idleDeadDetection is equivalent to testing if idleDeadTime
is non-zero. The idleDeadDetection field is thus redundant, so remove
it.
Change-Id: Id11f2829167f4de1505cee286dcc7c56b431a5a6
Reviewed-on: http://gerrit.openafs.org/10782
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
This commit removes the functionality introduced in
c26dc0e6aa (which is also modified by a
few later commits), as well as
05f3a0d1e0. Instead we modify the
startWait check in rxi_CheckCall to apply to both "reading" and
"writing" to enforce "idle dead" timeouts.
Why do this? First, let's start out with the following:
If an Rx call gets permanently "stuck", what happens? What should
happen?
Here, "stuck" means that either the server or client hangs while
processing the call. The server or client is waiting for something to
complete before it issues the next rx_Read() or rx_Write() call. In
various situations over the years, this has happened because the
server or client is waiting for a lock, waiting for local disk I/O to
complete, or waiting for some other arbitrary event to occur.
Currently, what happens with such a "hanging" call is a little
complex, and has several different results in different situations.
The behavior of a call in this "stuck" situation is handled by the
"idle dead" timeout of an Rx call/connection. This timeout is enforced
in rxi_CheckCall, in two different conditionals (if an "idle dead"
timeout is configured):
if (call->startWait && ((call->startWait + idleDeadTime) < now) &&
(call->flags & RX_CALL_READER_WAIT)) {
if (call->state == RX_STATE_ACTIVE) {
cerror = RX_CALL_TIMEOUT;
goto mtuout;
}
}
and
if (call->lastSendData && ((call->lastSendData + idleDeadTime) < now)) {
if (call->state == RX_STATE_ACTIVE) {
cerror = conn->service ? conn->service->idleDeadErr : RX_CALL_IDLE;
idle_timeout = 1;
goto mtuout;
}
}
The first of these handles the case where we are waiting to rx_Read()
from a call for too long (the other side of the call needs to give us
more data). The second handles the case where we are waiting to
rx_Write() for too long (the other side of the call needs to read some
of the data we sent previously).
This second case was added by commit
c26dc0e6aa, but it has the general
problem that this check does not check if anyone is actually trying to
write to the call, and just tries to keep track of the last time we
wrote to the call. So, we may have written some data to the call
successfully, and then we went off to do something else. We can then
kill the call later for taking too long to write to, even though
nobody is trying to write to it. This results in a few problems:
(1) When the fileserver is writing to the client, it may need to wait
for various locks and it may need to wait for local disk I/O to
complete. If this takes too long for any reason, the fileserver
will kill the call (currently with VNOSERVICE), but the thread
for servicing the call will still keep running until whatever the
fileserver was waiting for finishes.
(2) lastSendData is set whenever we send any ACK besides an
RX_ACK_PING_RESPONSE (as of commit
658d2f4728). If we are the server,
and we send any such ACK (in particular, RX_ACK_REQUESTED is
common), the "idle dead" timer starts. This means the server can
easily kill a call for idleness even if the server has never sent
the client anything, and even if the server is still actively
reading from the client.
(3) When a client tries to issue an RPC for the server, the "idle
dead" timeout effectively becomes a hard dead timeout, since we
will write the RPC arguments to the Rx stream, and then wait for
the server to respond with the output arguments. During this
time, our 'lastSendData' is the last time we sent our arguments
to the server, and so the call must finish before
'call->lastSendData + idleDeadTime' is in the past.
In addition to this "idle dead" processing, commit
05f3a0d1e0 appears to attempt to provide
"idle dead"-like behavior by disabling Rx keepalives at certain points
(when we're waiting for disk I/O), controlled by the application
process (currently only the fileserver). The idea is that if
keepalives are disabled, the server will just appear unreachable to
the client, and so if disk I/O takes too long, the client will just
kill the call because it looks like the server is gone. However, this
also has some problems:
(A) Clients send their own keepalives, and the server will still
respond to them. So, the server will not appear to be
inaccessible anyway. But even if it did work:
(B) This approach only accounts for delays in disk I/O, and not
anywhere else (we could hang for a wide variety of reasons). It
also requires the fileserver to decide when it's okay for a call
to be killed due to "idle dead" and when it's not, which
currently seems to be decided somewhat arbitrarily.
(C) This doesn't really let the client dictate its own "idle dead"
timeout for idleness specifically; it just looks like the server
went away.
(D) The fileserver would appear to be unreachable in this situation,
but it's not actually unreachable. This can be confusing to
clients, since distinguishing between a server that is completely
down vs just taking too long is an important distinction.
(E) As noted in (1) above, the fileserver thread will still keep
waiting for whatever it has been waiting for, even though the
call has been killed and is thus useless.
So instead of all of this stuff, just modify the rxi_CheckCall "idle
dead" check to depend on the call->startWait parameter instead. This
parameter will be set whenever anyone is waiting for something to
proceed in the call, whether that is waiting to read data or write
data. This should make "idle dead" processing much simpler, as it is
reduced to effectively: if we've been waiting for longer than N
seconds, kill the call.
This involves ripping out much of the code related to lastSendData and
rx_KeepAlive*. This means removing the call->lastSendData field and
the rx_SetServerIdleDeadErr function, since those were only used for
the behavior in c26dc0e6aa. This also
means removing rx_KeepAliveOn and rx_KeepAliveOff, since those were
only used for the behavior in
05f3a0d1e0. This commit also removes the
only known use of the VNOSERVICE error code, so add some comments
saying what this code was used for (especially since it is quite
different from other V* error codes).
Note that the behavior in (1) could actually be desirable in some
situations. In environments that have clients without "idle dead"
functionality, and those clients cannot be upgraded or reconfigured,
this commit means those clients may hang forever if the server hangs
forever. Some sites may want the fileserver to be able to kill such
hanging calls, so the client will not hang (even if it doesn't free up
the fileserver thread). However, such behavior should really be a
special case for such sites, and not be the default behavior (or only
behavior) for all sites. The fileserver should just be concerned with
maintaining its own threads and availability, and clients should
manage their own timeouts and handle hanging servers.
Thanks to Markus Koeberl, who originally brought attention to some of
the problematic behavior here, and helped investigate what was going
on in the fileserver.
Change-Id: Ie0497d24f1bf4ad7d30ab59061f96c3298f47d17
Reviewed-on: http://gerrit.openafs.org/10773
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
When enumerating a directory if status info for an entry cannot be
obtained, fake it. Do not return STATUS_ACCESS_DENIED to the redirector
as that will be interpreted as the directory not being listable.
Change-Id: I488f5d8d244c363135e00e156a685cd56fd060c8
Reviewed-on: http://gerrit.openafs.org/11736
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
In the case where an explicit mount point to a .backup volume is
resolved from a .backup volume the cache manager refuses to evaluate
the mount point target. This is meant to address unwanted recursion
in the directory tree searches.
Change the error code to ERROR_TOO_MANY_SYMLINKS and propagate that
error to the AFS redirector. That will result in the application
receiving STATUS_ACCESS_DENIED instead of
STATUS_REPARSE_POINT_NOT_RESOLVED.
The STATUS_REPARSE_POINT_NOT_RESOLVED error causes cmd.exe and
powershell.exe to terminate recursive directory searches.
Change-Id: I5dfdd835e8696b823af45a8e5c33a5ca6320cf31
Reviewed-on: http://gerrit.openafs.org/11693
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
In various places where we intentionally ignore the return values of system
calls and standard library routines, this changes the way in which we do so,
to avoid compiler warnings when building on Ubuntu 12.10, with gcc 4.7.2 and
eglibc 2.15-0ubuntu20.1.
Change-Id: I41f806a686d68b02aec2847886bd5d787cbff3d3
Reviewed-on: http://gerrit.openafs.org/9980
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
We use some uio structures that were allocated on the stack, but we
only initialize them by initializing individual fields. On some
platforms (Solaris is one known example, but probably not the only
one), there are additional fields we do not initialize. Since we
cannot be certain of what any additional fields there may be, just
zero the whole thing.
This is basically the same change as
I0eae0b49a70aee19f3a9ec118b03cfb3a6bd03a3, but in the rx subtree.
Change-Id: I400144143bb1f47409eccb931daacc8a5058e074
Reviewed-on: http://gerrit.openafs.org/11711
Tested-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Although there are still servers deployed on AIX systems,
there may not be any clients in use, and it is unlikely that
there will be new deployments which require this documentation.
Change-Id: Id6554e120cb01c5d4de5c7de67e74e802b7ea217
Reviewed-on: http://gerrit.openafs.org/11592
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Get the rest of them all at once.
Change-Id: Idb33746d43a4a1a9f41e21d7f6d81360ecdd952e
Reviewed-on: http://gerrit.openafs.org/11591
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
it shoudn't happen that we aren't setting mvid on root vnodes,
so assert so we notice if the invariant is violated
Change-Id: I32c8aa4dced8751d11817d74508b87ff44261837
Reviewed-on: http://gerrit.openafs.org/11669
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
