Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible OSAL bug in OS_FileSysAddFixedMap not allowing FS_BASED mapping on VxWorks #580

Closed
johnphamngc opened this issue Aug 26, 2020 · 8 comments · Fixed by #709 or #750
Closed
Assignees
Milestone

Comments

@johnphamngc
Copy link

johnphamngc commented Aug 26, 2020

Describe the bug
Unable to add a arbitrary filesystem path mappings with OS_FileSysAddFixedMap

To Reproduce
Call OS_FileSysAddFixedMap w/ parameters &fsid,"/", "/", and it'll fail

Expected behavior
Entry added to OS_filesys_table

Code snips
It looks like the error code from line 516 is propagated improperly to line 530 instead of just being used to set flags on 524, causing the finalization step to fail.

return_code = OS_FileSysStartVolume_Impl(local_id);
if (return_code == OS_SUCCESS)
{
local->flags |= OS_FILESYS_FLAG_IS_READY;
return_code = OS_FileSysMountVolume_Impl(local_id);
}
if (return_code == OS_SUCCESS)
{
/*
* mark the entry that it is a fixed disk
*/
local->flags |=
OS_FILESYS_FLAG_IS_MOUNTED_SYSTEM |
OS_FILESYS_FLAG_IS_MOUNTED_VIRTUAL;
}
/* Check result, finalize record, and unlock global table. */
return_code = OS_ObjectIdFinalizeNew(return_code, global, filesys_id);

System observed on:

  • SP0-s
  • OS: VxWorks 6.9
  • CFE 6.7.0+dev292, OSAL 5.0.0+dev247, PSP 1.4.0+dev76 (w/ additions for SP0-s)

Additional context
Attempting to add a mapping to / for netbooting off of FTP on SP0-s PSP. Mapping /cf/ to /cf/ used to work earlier this year. In addition, OS_FileSys_FindVirtMountPoint appears to fail to map to "/" if it's in the table (although I suppose it's not desirable to map / in production anyway, since it'd override all other mappings).

Reporter Info
John N Pham, Northrop Grumman

@johnphamngc
Copy link
Author

johnphamngc commented Aug 26, 2020

Hm, after a code change, it still fails to read /cf/cfe_es_startup.scr due to the OS_FILESYS_FLAG_IS_MOUNTED_VIRTUAL flag not being set (checked in OS_FileSys_FindVirtMountPoint). Ultimately I had to work around this by forcing OS_FileSysMountVolume_Impl to always return success and mapping '/cf' to '/cf' instead of '/' to '/'.

@johnphamngc
Copy link
Author

johnphamngc commented Aug 27, 2020

It looks like there's a related problem w/ running this on Linux too off of an NFS mounted home directory:
CFE_PSP: OS_FileSysAddFixedMap() failure: -106
(after CTRL-C):
OS_FileSysMountVolume_Impl():254:ERROR: Cannot create mount point ./cf: File exists.

@skliper skliper added this to the 6.0.0 milestone Nov 4, 2020
@skliper skliper added the bug label Nov 4, 2020
@jphickey
Copy link
Contributor

jphickey commented Nov 5, 2020

Starting to look into this ....

First and foremost - the design of the OSAL virtual file system is rather simplistic in nature (always has been). The virtual mount points are all expected to be single names immediately off the root VFS (e.g. /cf or /eeprom etc). Anything more complex has not been tested and is unlikely to work because OS_TranslatePath looks for a very specific pattern keyed off the second slash (/) character in the virtual file name (i.e. stuff before the slash is considered the VFS mount point).

Is there a reason that you can't make all the virtual mount dirs fit the expected pattern in your system?

Perhaps this is a documentation issue - maybe it should be clearer what the virtual mount points are supposed to be?

@jphickey
Copy link
Contributor

jphickey commented Nov 5, 2020

NOTE: it is possible to map the real root fs - just not hte virtual root fs. For instance:

Status = OS_FileSysAddFixedMap(&fs_id, "/", "/root");

should map the virtual /root dir to be the real root directory on the target. In CFE you'd have to open files as /root/<my_file> instead of just /<my_file>. Would something like this be workable?

@johnphamngc
Copy link
Author

The intent was to map /cf to /cf,in order to get the CFE to locate all the stuff on /cf with the source mount point not necessarily being a separate filesystem, as the linux PSP does. Use case is netbooting the SP0 w/ the root FS being the FTP directory. This used to work, but I think the current implementation of OS_FileSysMountVolume_Impl() assumes it's an actual filesystem before attempting this and this fails for vxWorks. We did try mapping / to / first since that seemed to make sense, so maybe a doc issue.

I think JSC just worked around all of this by copying everything to ramdisk and mapping that as /cf/, whereas we just patched the OS_FileSysMountVolume_Impl()to always return success. Might be cleanest just to do what JSC does if it's too much work to allow arbitrary mount sources.

Another related problem, perhaps separate issue, is OS_FileSysMountVolume_Impl() on Linux fails if cFS is run off of an NFS mount (see second comment) for some reason. Also worked around by making it always return success.

@jphickey
Copy link
Contributor

Dug into this again over the past week - and I can confirm a discrepancy here in how VxWorks and POSIX are handling FS_BASED mounts, with respect to the existence of the mount point directory.

In POSIX, if the mount point does not exist, it will be created as part of OS_FileSysMountVolume_Impl() but VxWorks does not do this - this implementation tries to open() the dir so it must pre-exist or the call will fail. Furthermore when un-mounting it also tries to issue the FIOUNMOUNT ioctl, so this will likely fail too - but unlikely one would ever try to unmount an FS-based mount to begin with.

At any rate, I will update the OS_FileSys(Unmount|Mount)Volume_Impl() on VxWorks to better handle the FS_BASED entries. Mount should create the dir if it doesn't exist (like POSIX does) and also unmount should be a no-op.

jphickey added a commit to jphickey/osal that referenced this issue Dec 28, 2020
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 added a commit to jphickey/osal that referenced this issue Dec 28, 2020
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.
astrogeco added a commit that referenced this issue Jan 12, 2021
@jhnphm
Copy link

jhnphm commented Jan 20, 2021

Adding a note that the fix doesn't resolve the case where the directory is on a netdrv filesystem (i.e. FTP), since stat() on netdrv directories fail, due to a vxWorks limitation, and don't know of a clean solution for that (so not reopening/creating another ticket). Workaround would be to have a boot script or usrAppInit copy the files to a directory on ramfs and map that. The default SP0 PSP maps /ram0/cf as /cf which sidesteps the issue.

@jphickey
Copy link
Contributor

One possibility that crossed my mind is to simply skip the "stat" altogether and just call mkdir() and ignore any returned error. There'd be less error feedback as a result - but in truth there is a hole between the stat() and mkdir() anyway. That is, something could get in the middle because they aren't atomic. At any rate, that might also work with network mounts too.

jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
Added CFE_OMIT_DEPRECATED_6_7 just to be consistent
Also fix nasa#552 - removes non-existent codes
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
…r-codes

Fix nasa#580, Deprecate CFE_OS_ abstracted error codes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants