vol: Re-evaluate conditons for cond vars Most users of cond vars follow this general pattern when waiting for a condition: while (!condition) { CV_WAIT(cv, mutex); } But a few places in src/vol do this: if (!condition) { CV_WAIT(cv, mutex); } It is important to always re-check for the relevant condition after waiting for a CV, even if it seems like we only need to wait exactly once, because pthread_cond_wait() is allowed to wake up its caller spuriously even the CV hasn't been signalled. On Solaris, this can actually happen if the calling thread is interrupted by a signal. In VInitPreAttachVolumes() for DAFS, currently this can cause a segfault if CV_WAIT returns while 'vq' is empty. We will try to queue_Remove() the head of the queue itself, resulting in vq.head.next being set to NULL, which will segfault when we try to pull the next item off of the queue. We generally cannot be interrupted by a signal when using opr's softsig, because signals are only delivered to the softsig thread and blocked in all other threads. It is technically possible to trigger this situation on Solaris by sending the (unblockable) SIGCANCEL signal, though this would be very unusual. To make sure issues like this cannot happen and to avoid weird corner cases, adjust all of our CV waiters to wait for a CV using a while() loop or similar pattern. Spurious wakeups may be impossible with LWP, but just try to make all code use a similar structure to be safe. Thanks for mvitale@sinenomine.net for finding and investigating the relevant issue. Change-Id: Ib188708b909661c28fa1a709306a02b01d3cd6d3 Reviewed-on: https://gerrit.openafs.org/15327 Reviewed-by: Cheyenne Wills <cwills@sinenomine.net> Reviewed-by: Mark Vitale <mvitale@sinenomine.net> Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net> Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Tested-by: BuildBot <buildbot@rampaginggeek.com>
vol: Don't leak volume bitmaps Since the original IBM code import, attach2 has set the volume's index bitmaps to NULL in preparation for allocating and initalizing new bitmaps. However, the volume may already have bitmaps from previous operations, and this is much more likely with DAFS. In this case, the old bitmaps are leaked. Instead, free any existing bitmap before allocating a new one. Discovered via Solaris libumem.so.1. Change-Id: I77e42ccd36cedead060974a4fe99e37b4f262093 Reviewed-on: https://gerrit.openafs.org/15428 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Andrew Deason <adeason@sinenomine.net> Reviewed-by: Cheyenne Wills <cwills@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
vol: Remove vestigial DVINC code Since the original IBM code import, DoCloneIndex has contained conditional code for DVINC. But DVINC has never been defined, so this has been dead code for a very long time. Remove the vestigial code. No functional change is incurred by this commit. Change-Id: I1d5c067cbd901f0510139bf8b7750d1575bd2adc Reviewed-on: https://gerrit.openafs.org/15332 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Cheyenne Wills <cwills@sinenomine.net> Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
clang-16: Fix conditionally unused-but-set variables clang-16 is flagging unused-but-set variables which result in build errors when --enable-warning is turned on. remote.c:485:15: error: variable 'pass' set but not used [-Werror,-Wunused-but-set-variable] afs_int32 pass; ^ These variables are actually used in specific cases depending on build configuration (e.g. AFS_PTHREAD_ENV, etc.). Relocate these variables so they are fully defined or referenced within preprocessor '#if' blocks. Change-Id: I900a192ec781be4ab4453769e1d8f0cc48fe5757 Reviewed-on: https://gerrit.openafs.org/15177 Reviewed-by: Andrew Deason <adeason@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Tested-by: BuildBot <buildbot@rampaginggeek.com>
vol: Use asprintf in _namei_examine_special GCC-12 is flagging an snprintf statement with a format truncation warning: namei_ops.c: In function ‘namei_ListAFSSubDirs’: namei_ops.c:2029:22: error: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size between 0 and 511 [-Werror=format-truncation=] Change code to use asprintf instead of snprintf. Return an error if a memory allocation fails. Change-Id: I1f617ab22dbec4c2497ec482115cd20c9af0ecfa Reviewed-on: https://gerrit.openafs.org/14955 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Fix PrintInode() mismatched array parameter warnings The PrintInode() prototypes do not match the function definitions. When AFS_64BIT_IOPS_ENV is defined (which is the common case and is required for namei), the buffer parameter is declared as a bounded character array (afs_ino_str_t) in the prototype, but is defined as an unbounded character pointer. When AFS_64BIT_IOPS_ENV is not defined (for legacy 32-bit inode vice partitions), PrintInode() is declared with no specified parameters. A static buffer is used to hold the formatted string when a NULL is passed as the first argument to PrintInode(). However, this method is only used by the volinfo and iopen utility programs. Fix the mismatch function prototypes and definitions to use the bounded char array (afs_ino_str_t) in all cases. Remove the deprecated function declaration with no specified parameters. Update vol-info and iopen to pass an afs_ino_str_t buffer and remove the now unused static buffer. Update the duplicated PrintInode() function definition in namei_ops.c. (This duplicated code could be removed in a future commit.) Change-Id: I5c0128bb0d572dab0df637289daad0e648ad8a8f Reviewed-on: https://gerrit.openafs.org/14770 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Cheyenne Wills <cwills@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
vol: Remove unused ihandle macros All of these macros are not used anywhere in the tree; get rid of them. Mostly this is just removing cruft, but removing FDH_READ/FDH_WRITE/FDH_SEEK also helps make sure we don't accidentally reintroduce code paths that use non-positional i/o. No functional change should be incurred by this commit. Change-Id: I9f6e885845e814cdb22d7c963e4ee6826cad8cea Reviewed-on: https://gerrit.openafs.org/14761 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
vol: Introduce and use FDH_BLOCKSIZE A couple of places in src/volser currently have some logic to get the size and blocksize of a file. The existing logic is nontrivial due to platform-specific quirks, and ignores afs_fstat errors. To fix these issue and consolidate the code into one place, introduce a new function, FDH_BLOCKSIZE, which gets the file size and blksize. Update the places in src/volser to use the new function. Change-Id: I4daeec84c8fdb5756a8d6a7f477d0045a19a8fe9 Reviewed-on: https://gerrit.openafs.org/14662 Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Tested-by: BuildBot <buildbot@rampaginggeek.com>
Change AFS*_LINUXnn_ENV to AFS*_LINUX_ENV The minimum Linux kernel that is now supported is linux-2.6.18. The Linux versioned preprocessor macros AFS_*LINUXnn_ENV are no longer needed to distinguish the different levels of Linux and can be merged into just a single set of macros. Perform a global change of _LINUX\d+_ENV to _LINUX_ENV. e.g. AFS_LINUX24_ENV -> AFS_LINUX_ENV AFS_USR_LINUX24_ENV -> AFS_USR_LINUX_ENV AFS_AMD64_LINUX20_ENV -> AFS_AMD64_LINUX_ENV Replace the multiple definitions for the versioned 'AFS*_LINUXnn_ENV' with just single non-version definitions 'AFS*_LINUX_ENV'. Apart from replacing the now-redundant #define directives and tidying up a few comments at the close of a preprocessor block to match their current form, this commit was done using a mechanical change of the variable names and did not reduce preprocessor statements that could now be combined or eliminated. Nor does this commit remove dead code. A follow-up commit (Cleanup AFS_*LINUX_ENV usage) will handle these changes. The updates should have no functional changes. Change-Id: I71e32ca9818d28f82b4f23679868d1b9a62c44bd Reviewed-on: https://gerrit.openafs.org/14387 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
vol: Use FDH_SIZE more consistently Several callers use OS_SIZE(fdP->fd_fd) instead of just calling FDH_SIZE(fdP). Right now these calls are equivalent, but change all of these callers to use FDH_SIZE, to make it easier to possibly change the behavior of FDH_SIZE in the future. Change-Id: If846c160c71f11935de8c0e4634afa2c7f7ee53f Reviewed-on: https://gerrit.openafs.org/14657 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
vol: Blank fake linkHandles A couple of places in namei_ops.c use "fake" linkHandle FdHandle_t's. Make sure these are initialized, so we don't accidentally use uninitialized fields in the FdHandle_t. Currently, the code never uses any fields that are not explicitly set in this same code path, but this seems rather fragile, and future commits may change the fields in an FdHandle_t. Change-Id: I9372f03a4d3b68afd3ab24a27ae669cba92e0abe Reviewed-on: https://gerrit.openafs.org/14656 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
ihandle: Remove FDH_READV/WRITEV references FDH_READV and FDH_WRITEV have been unused since commit 335ccb40 (ihandle positional read and write). Remove a couple of lingering references to them. Change-Id: I640d411782440958a823ef0e2aa810bddd8ceee2 Reviewed-on: https://gerrit.openafs.org/14655 Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Tested-by: BuildBot <buildbot@rampaginggeek.com>
namei: Calculate offset before use in GetFreeTag We pass 'offset' to FDH_LOCKFILE here, but 'offset' is calculated a few lines below. The compiler doesn't catch this, since FDH_LOCKFILE is a macro that ignores the second argument (except on WINNT). Just move the calculation a few lines above, to ensure 'offset' is set before it's actually used, to avoid issues if and when FDH_LOCKFILE is changed to use the second argument. Change-Id: If858005cb83370ca905f93f9d3d7eed2941ddf99 Reviewed-on: https://gerrit.openafs.org/14654 Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Tested-by: BuildBot <buildbot@rampaginggeek.com>
Remove a few unused opr/util components A few pieces of opr and util have no callers, have never been used since they were added, and are not expected to be used in the near future. Remove the unused code; if we ever want to use any of this again, it can be re-added when we actually need it. Specifically, this removes: - 'stoupper' in src/opr/casestrcpy.c, added in e117599f (fileserver-hates-pruclient-20060626) - 'opr_ffsll' and 'opr_flsll' in src/opr/ffs.h, added in d4369917 (opr: implement the BSD ffs() functions) - src/util/thread_pool.c and related headers, added in 4ca57f3f (Provide an abstract thread pool object) Change-Id: Id098cb86318ac9e2412937f4b63b5d99e35be568 Reviewed-on: https://gerrit.openafs.org/14547 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
vol: ensure ih package defaults are set for salvage Like most OpenAFS components that work with ihandles, salvager relies on implicit invocation of ih_PkgDefaults via the one-shot in the first call to IH_INIT. Unfortunately, there is at least one reachable code path in salvager that asserts (panics) because vol_io_params has not yet been initialized. This is when salvaging a volume group that does not have a link table; the salvager then panics while attempting to create a new link table: SalvageFileSys -> SalvageFileSys1 -> DoSalvageVolumeGroup -> CreateLinkTable -> IH_CREATE -> namei_icreate -> icreate -> namei_SetLinkCount -> FDH_SYNC -> ih_fdsync -> osi_Assert(0) This path was discovered while testing the non-dafs salvager, but it has also been observed in the field with the DAFS salvageserver. It is possible that there are additional undiscovered paths where vol_io_params are required but uninitialized. Add an implicit ih_PkgDefaults call to icreate to avoid triggering the assert via the code path above. Change-Id: If348d7f8ab2d34d55727b5a6d78db6e1c8e14cdf Reviewed-on: https://gerrit.openafs.org/14378 Reviewed-by: Andrew Deason <adeason@sinenomine.net> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
vol: move ih_PkgDefaultsSet check inside ih_PkgDefaults Instead of repeating the oneshot check in each caller of ih_PkgDefaults, move the oneshot check into ih_PkgDefaults itself. While here, ensure that ih_PkgDefaults does its work under IH_LOCK. Finally, make ih_PkgDefaultsSet a local static variable (no longer exported). Change-Id: I66717de12cb5cc70de0507a768f968f6afbd38f8 Reviewed-on: https://gerrit.openafs.org/14383 Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Reviewed-by: Mark Vitale <mvitale@sinenomine.net> Tested-by: BuildBot <buildbot@rampaginggeek.com>
Resolve missing printf args A handful of printf's requested more args than they were given. The missing args are now provided. (via cppcheck) Change-Id: I3d2bfd1b68a3518ee4c8a65f02446a2bae85d926 Reviewed-on: https://gerrit.openafs.org/13155 Reviewed-by: Andrew Deason <adeason@sinenomine.net> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
vol: prevent salvage segfault for orphaned vnode with out-of-range parent While salvaging a RW volume, salvager may segfault if it encounters an orphaned directory with a parent vnode that does not exist. For example, if the large vnode index contains a maximum vnode of 2901, any parent vnode encountered that is larger than 2901 will result in an out-of-bounds reference to our vnode essence array, leading to a segfault or undefined behavior. Modify the logic to check for out-of-bounds parent vnodes, and log them rather than segfaulting. Change-Id: I49f53935830fbb428fe0bff04c33248d3806a4b2 Reviewed-on: https://gerrit.openafs.org/14385 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Andrew Deason <adeason@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
vol: always build vol-bless utility In order to avoid future bit-rot, always build vol-bless. Also add it to the clean rule. However, continue to leave it undistributed and uninstalled by default. Change-Id: I3d2dc94c28a7feeb20167223655e97538e807ce6 Reviewed-on: https://gerrit.openafs.org/14464 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de> Reviewed-by: Cheyenne Wills <cwills@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
vol: add vol-bless to .gitignore No functional change is incurred by this commit. Change-Id: If84ba946d43d67eb6c253462f5826f9a45a2df46 Reviewed-on: https://gerrit.openafs.org/14463 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Cheyenne Wills <cwills@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>