Fix #580, improve FS_BASED mounts on VxWorks #709
Conversation
|
Ping @johnphamngc -- please review/confirm if this is adequate to address the issue described in #580 |
The mount/unmount implementation was not really checking for and handling this mapping type. To be consistent with POSIX it should also create a directory if it does not already exist.
jphickey
force-pushed
the
fix-580-vxworks-fsbased-mount
branch
from
8168c01
to
e175530
Compare
jphickey
added
the
CCB:Ready
| } | ||
| else | ||
| { | ||
| if (mkdir(local->system_mountpt, 0775) == 0) |
There was a problem hiding this comment.
VxWorks 6.9's mkdir doesn't take a mode; throws a lint error.
There was a problem hiding this comment.
Sorry - I should have mentioned, if you want to build you'll need to also pull the fix/workaround for VxWorks 7 compatibility in #706 (mkdir was fixed to be POSIX compliant in VxWorks 7).
There was a problem hiding this comment.
Hm, this commit 25cd5df seems to have broken vxWorks when attempting to load cFE - "Warning: module 0x56d75b0 holds reference to undefined symbol OS_WaitForStateChange_Impl." when trying to build and run the branch. I'll see if I can backport the fix to OSAL 5.1.0-rc1+dev16 and see if it fixes the issue.
There was a problem hiding this comment.
Yeah there was a bunch of PRs recently, I thought #706 + this one would work in isolation but maybe not...
At any rate, with this change I was able to test/confirm that you can do:
OS_FileSysAddFixedMap(&fs_id, "/", "/root");
Which succeeds, then followed by:
OS_TranslatePath("/root/myfile.txt", myfile_path)
Which in turn sets myfile_path to //myfile.txt
Note - yes it translates to a double leading slash but that is technically OK on UNIX and VxWorks seems to accept this and open it fine as well...
There was a problem hiding this comment.
Can you confirm that the case of OS_FileSysAddFixedMap(&fs_id, "/cf", "/cf"); works as well? Otherwise I'll get around to backporting the fix and checking it sometime tomorrow.
I think I'll make another ticket for the NFS directory case, since it's unrelated besides both affecting OS_FileSysMountVolume_Impl and is a change in the POSIX OSAL instead. That one is a lower priority since I can just tell our developers to not run cFS off of an NFS mounted directory.
There was a problem hiding this comment.
Looks like it works for that case as well. I'll wait till we rebase w/ the latest CFE to see if the NFS issue manifests again before making another ticket.
There was a problem hiding this comment.
Thanks, and yes please submit a separate ticket for the NFS issue. In the meantime I am looking into getting one of my test targets booted using an NFS root, and I'll check if anything looks amiss on it.
|
Added dependency label - if/when merging note that this also requires the compatibility fix in #706 |
astrogeco
added
CCB-20210106
and removed
CCB:Ready
|
CCB 2021-01-06 APPROVED |
Describe the contribution
The mount/unmount implementation was not adequately checking for and handling the
FS_BASED(pass -through) mapping type - which should be mostly a no-op. But to be consistent with POSIX it should also create a mount point directory if it does not already exist when using this mapping type.Adds a documentation note to
OS_FileSysAddFixedMap()regarding the limitation that the virtual mount point cannot be empty - soOS_FileSysAddFixedMap(.., "/", "/")does not work - never did. HoweverOS_FileSysAddFixedMap(.., "/", "/root")does work and allows one to open files in the root as "/root/" from OSAL applications.Fixes #580
Testing performed
Build and run all unit tests on native
Sanity check CFE
Confirm behavior of
OS_FileSysAddFixedMap()+OS_TranslatePath()when mapping to root FS.Expected behavior changes
Mount point directories do not need to be already existing when using
OS_FileSysAddFixedMapSystem(s) tested on
Ubuntu 20.04 (native)
VxWorks 6.9 (mcp750)
Additional context
Auto-creating the mount point dir is relevant to unit tests - this simplifies running the unit tests by not requiring the user to pre-create this directory. Otherwise without this one gets unit test failures if the directory doesn't already exist.
Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.