ih_sync_thread currently syncs files flagged as needing
synchronization in the background every 10 seconds. This practice has
caused severe data corruption on more than one occasion over the past
few years (124359, 131530). It has also been argued repeatedly that it
provides no meaningful additional on-disk consistency, so there is no
reason for it to exist even if it were error-free.
Syncing files in the background provides no guarantee on the
consistency of the file contents, since the files are not synced in
any order with respect to each other, or with respect to what
filesystem operations may be occurring in the application.
Additionally, journalling filesystems common on fileserver backends
will typically ensure some consistency after a certain amount of time
(by default, 5 seconds on ZFS and ext3+), so doing this sync ourselves
is often redundant or even counterproductive.
So, to avoid current and future issues with ih_sync_thread interacting
with other ihandle users, just get rid of it. Files flagged as needing
sync are still synced (not in the background) during IH_REALLYCLOSE.
FIXES 131530
Change-Id: I29571c82c5b7454cd834b339fd48baeb9963a87b
Reviewed-on: http://gerrit.openafs.org/8797
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
We don't need to zero the result code two lines before we assign
a new value to it - just remove the pointless assignment
Caught by clang-analyzer
Change-Id: I8fefdc839ef9980d7f7703add3b11b089d524d38
Reviewed-on: http://gerrit.openafs.org/9167
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
ni is unused on the true side of the if expression in nuke() - so
just don't bother populating it.
Caught by clang-analyzer
Change-Id: Idfd088f999f4c4bcdabc03b8b86636aa02541343
Reviewed-on: http://gerrit.openafs.org/9166
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
The type that an enum is promoted to for arithmetic is compiler
dependent. We can't assume that it's an int, or a short.
Explicitly cast the volume state enum where clang gets upset.
Change-Id: Iab285d3a04dac698797ab0df2337aee75e3876b2
Reviewed-on: http://gerrit.openafs.org/9137
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
The comment for the code handling the -showsuid option was incorrect.
Change-Id: I0a63211cf5dd8a3b2636d4e4a2cc45a55c3bf62a
Reviewed-on: http://gerrit.openafs.org/9109
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
In rare circumstances an FSYNC_VOL_ON operation may fail silently,
leaving the volume in its previous state. The only clue is a FileLog
message "volume <nnnn> not in quiescent state".
This is caused by a race condition in the volume package: an
FSYNC_VOL_ON operation is attempting to preattach a volume
(in VPreAttachVolumeByVp_r()) at the same time a fileserver RPC
(e.g. FetchStatus) is detaching the volume (in VReleaseVolumeHandles_r())
at the conclusion of attach2() logic.
The fix calls VWaitExclusiveState_r() before calling
VPreAttachVolumeByVp_r().
Change-Id: Ib66859381d29311fda3e08984dcb740eadafb340
Reviewed-on: http://gerrit.openafs.org/8814
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
Fix the trailing whitespace and leading spaces
before tabs in the readme files.
Change-Id: If20e528ddb28f82e4d3d1b1f03dec8670f914afc
Reviewed-on: http://gerrit.openafs.org/8877
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Ken Dreyer <ktdreyer@ktdreyer.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Avoid out-of-bounds array accesses by first checking that the index
is in-bounds, and then dereferencing; not the other way around.
Change-Id: Ib36a28ca6181a7a7fd602de45cb5a15fad099a44
Reviewed-on: http://gerrit.openafs.org/8879
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Two places in the vol package performed IH_CONDSYNC(vp->linkHandle)
only if AFS_NT40_ENV. This was correct when the namei implementation
was windows only; however, this ifdef was apparently overlooked
when namei was implemented for UNIX.
Change-Id: I0cbe2c5c0a65ece0485b8c2d5a5f92eeb53725fe
Reviewed-on: http://gerrit.openafs.org/8815
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
use one type for volumeid, not 3. kill VolId, and stop
using afs_uint32 (and a few times, afs_int32)
Change-Id: Ibcbd09b5a24d8720b02a02f926e6f59dc0f529aa
Reviewed-on: http://gerrit.openafs.org/8845
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
a couple direct references to diskstuff have crept in. push them
back out
Change-Id: I66c7a157aeb326dee17012d551c0f499bae35bc7
Reviewed-on: http://gerrit.openafs.org/8807
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Add the FDH_ISUNLINKED functionality to ihandle. This lets the caller
know if the file for the underlying file descriptor has been deleted
out from under us. This is useful for sanity checks in some callers.
Change-Id: I59d4fffba853cfa1b900a08b7b43134c29bbf844
Reviewed-on: http://gerrit.openafs.org/8838
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
According to the mesoamerican long count calendar, ifdef ladders will
destroy the world in 2012.
Change-Id: Ie9b16d600e4743eba3d934f8c3cf4d299abbf7e3
Reviewed-on: http://gerrit.openafs.org/8837
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
Before DAFS, volumes that were not inService/blessed were not
accessible by normal clients, but were still allowed to attach. That
is, access to clients was prevented at VGetVolume-time, rather than at
attach-time. Commit 939382c5 tried to short-circuit this by detecting
this volume state at attach-time. However, volume utilities (e.g.
volserver) can give us back a volume over FSSYNC when they are done
with the volume, and for non-DAFS, we then try to attach the volume.
So, with 939382c5 that attachment will fail when volserver gives us
back a volume that is not inService/blessed (which can happen for some
normal volume operations).
This situation is not terrible, since either way the volume is not
usable by clients (since the volume didn't attach), and the volume is
still usable by volserver (since volserver is allowed to check out
nonexistent volumes). But it is a deviation from pre-DAFS behavior and
it can result in confusing error messages, so revert the 939382c5
behavior for non-DAFS.
For DAFS, this behavior is fine, since the fileserver does not attach
a volume unless it is trying to service a client request. So, leave it
for DAFS.
FIXES 131505
Change-Id: I8cf47640ceb068762eb42f1b70eb88aa9c4a060a
Reviewed-on: http://gerrit.openafs.org/8557
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
Allow non-fileserver, non-dafs, programs to attach volumes with the
V_READONLY mode again. This was lost during the code changes for
dafs.
The caller sends a fssync request to the fileserver, which updates the
on-disk contents of the volume headers, before the caller reads the
volume headers, allowing the caller to have the most recent info about
the volume. The fileserver still has the volume in use.
Later in the attachment process, the inUse check is skipped for the case
of a non-fileserver process which is attaching the volume using the
V_READONLY mode, otherwise the attachment would incorrectly mark the
volume as needing to be salvaged.
Note: The mode checks in VMustCheckOutVolume() are correct. We must
checkout the volume when attaching with the V_READONLY mode. This
fix updates the VShouldCheckInUse(), in which an additional
exception was added to cover the case for V_READONLY mode from a non-
fileserver process.
Note: A check is added in the dafs version of attach to avoid overwriting the
inUse field when a volume utility is attaching a volume in V_READONLY mode.
Currently, V_READONLY is not used by dafs, but this was done to avoid future
errors.
Change-Id: I09d3af1e74087d0627399392e662e5075d41cdeb
Reviewed-on: http://gerrit.openafs.org/8339
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
RX mutexes have two mechanisms for asserting ownership of a mutex:
MUTEX_ISMINE, which returns true if the caller is the owner of the
mutex in question, and osirx_AssertMutex which fires an assertion if
the calling thread doesn't own a specified mutex.
Neither of these work with pthread mutexes on all platforms, and the
kernel support for MUTEX_ISMINE is dubious in places. Because in some
implementations, MUTEX_ISMINE tries to lock the mutex in question, a
failing call to MUTEX_ISMINE can lead to the process holding an
additional, unexpected, lock.
This patch reworks all of this, and uses the new opr mutex framework
so that we can do mutex assertions in userspace, too. We remove the
unsafe MUTEX_ISMINE macro, and replace it with MUTEX_ASSERT which
simply asserts if the specified mutex is not held by the current
thread. osirx_AssertMutex is removed as it is now redundant.
MUTEX_ASSERT will always work in kernel code.
For userspace, we provide opr_mutex_assert, which is implemented using
POSIX error checking mutexes. As error checking mutexes have a runtime
overhead, this is only available when configured with
--enable-debug-locks, the rest of the time calls to opr_mutex_assert are
no-ops.
Change-Id: I285ee2b558389fa3d63b786e4bc4420fa2126c80
Reviewed-on: http://gerrit.openafs.org/8282
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
The MUTEX_* and CV_* macros leaked from RX a while ago - they mean
that most of the pthreaded tree has a dependency on RX, as well as
further confusing the difference between userspace and kernel.
Tidy all of this up so that we have opr_mutex_* and opr_cv_* macros
to handle portable locking, and use these throughout the userspace
tree. Only kernel code should now use MUTEX_* and CV_*.
Provide opr/lockstub.h as a header that can be used by non-pthreaded
code to easily stub out these functions.
Change-Id: I24be525c7667641134d50561ce7f1e2d752cdf1f
Reviewed-on: http://gerrit.openafs.org/8280
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
right now some not-commonly-defined code fails to compile with warnings.
fix it so it uses a format string like everything else.
Change-Id: Ib35445db9800444f3ee74a43f17aeb6f2959645a
Reviewed-on: http://gerrit.openafs.org/8236
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Modify RX so that it uses opr queues throughout, rather than the older,
non-type-safe rx_queue structure and macros. Attempt to clarify which
items in a structure are queue headers, and which are linkage pointers.
This has the knock on effect that including an RX header doesn't
automatically give you rx_queue.h in your application's namespace.
Change-Id: I1b3fbcd8c03f8153a557bd4532710bcebfe45818
Reviewed-on: http://gerrit.openafs.org/8232
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
One caller of VRequestSalvage_r in attach2 was not passing the
VOL_SALVAGE_NO_OFFLINE flag. This really should be passed for every
place that manually sets vp->nUsers = 0, since then the VPutVolume_r
handlers will never fire.
Change-Id: I088d93a6c598a4d636224f4cf4c0d89454efcca8
Reviewed-on: http://gerrit.openafs.org/8203
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
Casting NULL shouldn't be necessary in most cases. Eliminate this
in favor of shorter lines and consistency. Function pointers, variadic
arguments should still be cast.
Change-Id: Ibcd67378556e45c2b24f7aa85a4d73c72cd88e02
Reviewed-on: http://gerrit.openafs.org/8151
Reviewed-by: Garrett Wollman <wollman@csail.mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
This patch futher improves our usage of assert() and friends. The
intention is to bring clarity to which forms of assert are used in
which situations, and to solve the problem of assert(X), or
osi_Assert(X) being used in a situation where X has side-effects.
It introduces two new assertion macros opr_Assert() and opr_Verify(),
and clarifies the usage of osi_Assert() and assert().
*) opr_Assert is a direct equivalent of assert(), with the exception
that its output can be redirected to a log file when used in server
code. It is the preferred version of assert for libraries, and server
side code. Note that whilst opr_Assert doesn't currently become a
no-op when NDEBUG is defined, the intention is that it will do so at
some point in the future.
*) opr_Verify(X) asserts if the value of X is false. Unlike assert()
it will always run X, regardless of whether the value is checked or
not. The eventual intention is that when NDEBUG is defined,
opr_Verify(X) => X
*) osi_Assert is an assertion macro intended for use in kernel code,
or in mixed kernel/userland code. When code is built for userspace,
osi_Assert(X) => opr_Assert(X)
*) assert is the system's own assert macro. It should only be used
in client code. Whilst a header (opr_assert.h) is provided to map
assert() to opr_Assert(), its use is discouraged
Change-Id: Ie6d61305686bdc7193cc8690e6f4fbe363211faf
Reviewed-on: http://gerrit.openafs.org/5395
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
Rather than using our own renamefile() implementation on Windows,
pull in rk_rename() from libroken wherever a rename which unlinks
the destination file is required.
Change-Id: Ied8ac26b90a0983283b02704328da123bdd4f4b2
Reviewed-on: http://gerrit.openafs.org/8026
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
given that we can mark something invalid for future use, ever,
once we have done so for all fds, we ih_reallyclose is done.
don't persist the setting to the detriment of new fds
Change-Id: If82368dad79841a5d68f45a608b2645b32f951e7
Reviewed-on: http://gerrit.openafs.org/6101
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Alistair Ferguson <alistair.ferguson@mac.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
If an RW is already present on disk on the target server (any partition),
'vos convertROtoRW' will still convert the RO, creating a second RW on the server.
Detect this and refuse to convert the RO by returning EXDEV (invalid cross-device link).
Change-Id: Ide15a7c39f2a975fd8597e497094b6a67b448e4f
Reviewed-on: http://gerrit.openafs.org/7934
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
There are several odd-looking but stylized loops involving GetNextVol()
which can be radically simplified if only GetNextVol() would return
a meaningful value. Move all of the code that skips non-volume-header
files in the directory into GetNextVol and have it return a truth value
(instead of always returning zero) that indicates whether it saw
something that looks like a volume header. Then all the odd while
loops and strcmps just collapse into while(GetNextVol(...)).
GetNextVol() had external scope, but there are no callers in the
tree that use it outside of volprocs.c, and it's not part of a
public library interface, so make it static.
While here, don't strcmp() past the end of a filename that begins with
'V' but is too short to be a valid volume name.
Change-Id: I214b33c46714959d700608b3d3718c79d3792878
Reviewed-on: http://gerrit.openafs.org/7893
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
Set the correct bit after the bitmap is expanded with VGrowBitmap
so we don't allocate it twice and trigger a salvage.
Change-Id: Iee307a0913d4402c2d5a32d493ff489b35778811
Reviewed-on: http://gerrit.openafs.org/7793
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
The C standard allows NULL to be defined as a bare "0", which will
be passed to variadic functions as an int. If the function expects
a pointer type, demons fly out of your nose. strcompose() is such
a function, so make sure that all of its callers cast NULL appropriately.
(None of them did.) This may be an opportune time to change all of
the callers to spell it opr_strcompose() as well, and avoid using a
reserved identifier, but this change does not do so.
Change-Id: Ia9007a48102da4d0a85a48b41a44e83102304b49
Reviewed-on: http://gerrit.openafs.org/7805
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
Actually, all of the initializers in this small function are dead.
Change-Id: If89fe3222ec39b34233f92f3aafd458f5723b0c5
Reviewed-on: http://gerrit.openafs.org/7771
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
Make all of the functions in partition.c that are only used in that
file static.
Change-Id: I12e0980578b79459f50b6f4a46d2c423847e18a9
Reviewed-on: http://gerrit.openafs.org/7586
Reviewed-by: Jeffrey Altman <jaltman@secure-endpoints.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
Avoid unnecessary volume attach retry loops if a volume
salvage is unable to be scheduled due to a hard error.
Change-Id: If1962b83e03f634044a55b03787661b8a7695008
Reviewed-on: http://gerrit.openafs.org/7566
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Tom Keiser <tkeiser@sinenomine.net>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
Since commit 5fc2365f, a VNOVOL error early in attach2 meant that we
skipped changing the volume state to anything, and just returned
instead. When we do this, the volume is in VOL_STATE_ATTACHING for
DAFS, and so if we return, the volume will forever be in
VOL_STATE_ATTACHING. The next thing that tries to access the volume
will wait forever for the volume to come out of that state.
So, revert half of 5fc2365f, and transition to ERROR state instead.
This code path should not be hit during normal usage, since a
nonexistant volume access for the fileserver will be detected earlier.
If the volume does not appear to exist at this stage of attachment,
something is wrong with the volume, so this warrants the ERROR state.
For the volserver and other volume utilities, we may hit this when a
request just plain references a nonexistant volume for whatever
reason, but in that case the vp should go away soon. For non-DAFS,
this commit does not change much, since the difference between
error_notbroken and unlocked_error is very small.
The other half of 5fc2365f is not changed, since it is correct. For
VOFFLINE errors at this point, the volume has already been
transitioned to VOL_STATE_UNATTACHED, so it is okay to return. Add a
comment to help make this more explicit.
Change-Id: I63853f6e1721020cd204639c6e3ce4b59de08f75
Reviewed-on: http://gerrit.openafs.org/7516
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
FSYNC_Drop currently attaches volumes that were checked out by the
dropped fssync handler, but not checked back in, in order to make the
volume available again. For DAFS, however, a full attachment is
unnecessary; just preattach instead.
Change-Id: I2c9d0b8cf0ddc7f4182347706b2c26d86f69725e
Reviewed-on: http://gerrit.openafs.org/7515
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
Normally, volume locks acquired by an exiting salvaging process would
be automatically given up when the process exits, since our FDs are
closed. However, if we exit by calling Exit() or Abort(), we
gracefully shutdown our SYNC channels before exiting. For FSSYNC, this
can result in the fileserver trying to online the volumes we had
checked out but had not yet checked back in, so the fileserver may try
to online a volume we have locked, before the locks have been
released.
To avoid this, unlock all volume locks for all partitions before we
shutdown SYNC channels on exit.
Change-Id: Idcda5fcf4639da7d4b951bea36606ea8daf68f3c
Reviewed-on: http://gerrit.openafs.org/7514
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
Tested-by: Derrick Brashear <shadow@dementix.org>
malloc() returns a (void *) on all of our current platforms. So,
don't bother casting the return value before assigning it - it's
unecessary noise.
Change-Id: I9d41f2f1aee03d9317b4c5780fb00e13d30b0821
Reviewed-on: http://gerrit.openafs.org/7461
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
calloc returns a (void *) argument. So we don't need to cast its
output before storing it.
Change-Id: Ibdfc61a8a2cf9779a0bfb17ff8ebf1f3bef3ea0e
Reviewed-on: http://gerrit.openafs.org/7456
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
realloc takes, and returns a (void *) argument. So we don't need to
cast these parameters into, or out of, realloc. Doing so is just
noise, so don't bother.
Change-Id: I64e721703536515b2e315e3b033eb2be14f7e18c
Reviewed-on: http://gerrit.openafs.org/7455
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
Rather than doing
a = malloc(sizeof(me));
memset(a, 0, sizeof(me));
Just use
a = calloc(1, sizeof(me));
This is simpler, shorter, and removes the potential for the size of
the memset not matching the size of the malloc (or the target of the
memset being wrong!)
Where the size is of the form (n * sizeof(me)), we also use
calloc(n, sizeof(me));
Change-Id: Ia0f75665c1031fd2982eee0e1d8c8ebe23d7fbc0
Reviewed-on: http://gerrit.openafs.org/7454
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
free(3) is defined as:
void free(void *ptr);
so there is no need to cast the pointer argument being past to it to
(char *), (void *), or whatever other randomness happened to be floating
through the author's mind. Remove all such casts, as they are just
noise.
Change-Id: Ib7c8c259d53e9ce74fc486cb0997360044795395
Reviewed-on: http://gerrit.openafs.org/7453
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
Where we have
newStr = malloc(strlen(oldStr)+1);
strcpy(newStr, oldStr);
replace these with
newStr = strdup(oldStr);
It's shorter, clearer, and gets rid of a load of occurences of strcpy,
which some compilers are now warning is unsafe (although it isn't in
this context)
Get rid of a number of custom duplicate string functions and replace
them with strdup where the behaviour is identical
Change-Id: If800343a7d13b1ba6362d4570a2a324fa3525250
Reviewed-on: http://gerrit.openafs.org/7450
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
In attach_volume_header, make sure we free the volume's header if
we're returning an error. We take care of the locks and i/o handles in
the immediately preceding block, but for an actual error we don't get
rid of the header. Do so.
Noticed by Tom Keiser.
Change-Id: I97e61700f644066ac1a0e6b45a67dd62c5397034
Reviewed-on: http://gerrit.openafs.org/7325
Reviewed-by: Tom Keiser <tkeiser@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
VAttachVolumeByVp_r (specifically attach_check_vop in attach2) already
handles checking for conflicting vol ops, and gives us VOFFLINE
appropriately. We don't need to check again in GetVolume.
Change-Id: Ibb93d423d3c856dd957a2569412a85698180ff8e
Reviewed-on: http://gerrit.openafs.org/7304
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Tom Keiser <tkeiser@sinenomine.net>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
attach2/VAttachVolumeByVp_r do not alter the yielded error code
according to specialStatus. So, pay attention to specialStatus after
receiving an error from VAttachVolumeByVp_r, to ensure we respond with
the correct error code.
Change-Id: I59e977dd1f0949f8fe5670c7a52429acbfb7d7e9
Reviewed-on: http://gerrit.openafs.org/7303
Reviewed-by: Tom Keiser <tkeiser@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
Currently, if GetVolume() finds that the volume we're trying to attach
has a vol op that leaves the volume offline, we do the
VBUSY/VRESTARTING trick as described in CheckVnode(). This doesn't
make any sense for a couple of reasons.
For one, VBUSY/VRESTARTING is not the correct error code to return to
the client when an offline vol op is in progress and vp->specialStatus
is not set everywhere else we yield VOFFLINE.
Additionally, this block of code is only hit once for a particular vol
op. Once we reach this section, the volume is in UNATTACHED state, and
so on the next iteration of GetVolume we will immediately return
VOFFLINE (or specialStatus). So the CheckVnode-like situation is not
applicable, since we are not returning VBUSY to the same client for 15
minutes; we would return VBUSY once and then return VOFFLINE.
Change-Id: I0e8376df7937fd6bd01f9998371b9289c4ad2618
Reviewed-on: http://gerrit.openafs.org/7302
Reviewed-by: Tom Keiser <tkeiser@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
When invoked with -header option, print the header inode
number, and namei filename, even if the header file cannot
be opened.
Change-Id: Id2a2e63f07e12cd817718a9da63d24250718ae06
Reviewed-on: http://gerrit.openafs.org/7190
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Tom Keiser <tkeiser@sinenomine.net>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
Currently, GetVolume treats a volume in the VOL_STATE_GOING_OFFLINE
state the same as VOL_STATE_SHUTTING_DOWN, and so returns VNOVOL for a
GOING_OFFLINE volume, but these states are very different.
GOING_OFFLINE indicates that a volume should soon be in the UNATTACHED
state, so we should treat GOING_OFFLINE the same as UNATTACHED for
returning errors to the user. For UNATTACHED, we return specialStatus
if it's set, or VOFFLINE otherwise; so, just do the same for
GOING_OFFLINE.
Change-Id: Ia9500d2cfd90b8c15782a06025f39298810d0a99
Reviewed-on: http://gerrit.openafs.org/7290
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
