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.

Sign up for GitHub

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

Fix #580, improve FS_BASED mounts on VxWorks #709

Merged
merged 1 commit into from
Jan 12, 2021
Merged

Fix #580, improve FS_BASED mounts on VxWorks #709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/os/inc/osapi-filesys.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,16 @@ typedef struct
* This mimics the behavior of a "FS_BASED" entry in the VolumeTable but is registered
* at runtime. It is intended to be called by the PSP/BSP prior to starting the application.
*
* @note OSAL virtual mount points are required to be a single, non-empty top-level directory
* name. Virtual path names always follow the form /<virt_mount_point>/<relative_path>/<file>.
* Only the relative path may be omitted/empty (i.e. /<virt_mount_point>/<file>) but the
* virtual mount point must be present and not an empty string. In particular this means
* it is not possible to directly refer to files in the "root" of the native file system
* from OSAL. However it is possible to create a virtual map to the root, such as by calling:
*
* OS_FileSysAddFixedMap(&fs_id, "/", "/root");
*
*
* @param[out] filesys_id A non-zero OSAL ID reflecting the file system
* @param[in] phys_path The native system directory (an existing mount point)
* @param[in] virt_path The virtual mount point of this filesystem
Expand Down
83 changes: 66 additions & 17 deletions src/os/vxworks/src/os-impl-filesys.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "os-vxworks.h"

#include "os-impl-filesys.h"
#include "os-impl-dirs.h"
#include "os-shared-filesys.h"
#include "os-shared-idmap.h"

Expand Down Expand Up @@ -295,22 +296,62 @@ int32 OS_FileSysMountVolume_Impl(const OS_object_token_t *token)
OS_filesys_internal_record_t *local;
int32 status;
int fd;
struct stat stat_buf;

local = OS_OBJECT_TABLE_GET(OS_filesys_table, *token);

/*
* Calling open() on the physical device path
* mounts the device.
* For FS-based mounts, these are just a map to a some other
* directory in the filesystem.
*
* If it does exist then make sure it is actually a directory.
* If it does not exist then attempt to create it.
*/
fd = open(local->system_mountpt, O_RDONLY, 0644);
if (fd < 0)
if (local->fstype == OS_FILESYS_TYPE_FS_BASED)
{
status = OS_ERROR;
if (stat(local->system_mountpt, &stat_buf) == 0)
{
if (S_ISDIR(stat_buf.st_mode))
{
/* mount point exists */
status = OS_SUCCESS;
}
else
{
OS_DEBUG("%s is not a directory\n", local->system_mountpt);
status = OS_FS_ERR_PATH_INVALID;
}
}
else
{
if (mkdir(local->system_mountpt, 0775) == 0)
Copy link

@johnphamngc johnphamngc Jan 4, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VxWorks 6.9's mkdir doesn't take a mode; throws a lint error.

https://docs.windriver.com/bundle/VxWorks_Kernel_API_Reference_6.9_1/page/1190833.html#b89517da-114c-41d9-a160-575213a358c5__mkdir

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

{
/* directory created OK */
status = OS_SUCCESS;
}
else
{
OS_DEBUG("mkdir(%s): errno=%d\n", local->system_mountpt, errnoGet());
status = OS_FS_ERR_DRIVE_NOT_CREATED;
}
}
}
else
{
status = OS_SUCCESS;
close(fd);
/*
* For all other (non-FS_BASED) filesystem types,
* Calling open() on the physical device path mounts the device.
*/
fd = open(local->system_mountpt, O_RDONLY, 0644);
if (fd < 0)
{
status = OS_ERROR;
}
else
{
status = OS_SUCCESS;
close(fd);
}
}

return status;
Expand All @@ -333,26 +374,34 @@ int32 OS_FileSysUnmountVolume_Impl(const OS_object_token_t *token)

local = OS_OBJECT_TABLE_GET(OS_filesys_table, *token);

/*
** vxWorks uses an ioctl to unmount
*/
fd = open(local->system_mountpt, O_RDONLY, 0644);
if (fd < 0)
if (local->fstype == OS_FILESYS_TYPE_FS_BASED)
{
status = OS_ERROR;
/* unmount is a no-op on FS-based mounts - it is just a directory map */
status = OS_SUCCESS;
}
else
{
if (ioctl(fd, FIOUNMOUNT, 0) < 0)
/*
** vxWorks uses an ioctl to unmount
*/
fd = open(local->system_mountpt, O_RDONLY, 0644);
if (fd < 0)
{
status = OS_ERROR;
}
else
{
status = OS_SUCCESS;
}
if (ioctl(fd, FIOUNMOUNT, 0) < 0)
{
status = OS_ERROR;
}
else
{
status = OS_SUCCESS;
}

close(fd);
close(fd);
}
}

return status;
Expand Down
34 changes: 34 additions & 0 deletions src/unit-test-coverage/vxworks/src/coveragetest-filesys.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,38 @@ void Test_OS_FileSysMountVolume_Impl(void)
* int32 OS_FileSysMountVolume_Impl (uint32 filesys_id)
*/
OS_object_token_t token = UT_TOKEN_0;
struct OCS_stat statbuf;

memset(&OS_filesys_table[0], 0, sizeof(OS_filesys_table[0]));
OS_filesys_table[0].fstype = OS_FILESYS_TYPE_NORMAL_DISK;
strcpy(OS_filesys_table[0].system_mountpt, "/ut");

OSAPI_TEST_FUNCTION_RC(OS_FileSysMountVolume_Impl(&token), OS_SUCCESS);

UT_SetDefaultReturnValue(UT_KEY(OCS_open), -1);
OSAPI_TEST_FUNCTION_RC(OS_FileSysMountVolume_Impl(&token), OS_ERROR);
UT_ClearForceFail(UT_KEY(OCS_open));

/* Additional cases for the FS_BASED handling */
OS_filesys_table[0].fstype = OS_FILESYS_TYPE_FS_BASED;

/* Mount dir does not exist but can be created */
UT_SetDefaultReturnValue(UT_KEY(OCS_stat), -1);
OSAPI_TEST_FUNCTION_RC(OS_FileSysMountVolume_Impl(&token), OS_SUCCESS);

/* Mount dir does not exist and cannot be created */
UT_SetDeferredRetcode(UT_KEY(OCS_mkdir), 1, -1);
OSAPI_TEST_FUNCTION_RC(OS_FileSysMountVolume_Impl(&token), OS_FS_ERR_DRIVE_NOT_CREATED);

/* Mount dir does exist but not a directory */
UT_ClearForceFail(UT_KEY(OCS_stat));
OSAPI_TEST_FUNCTION_RC(OS_FileSysMountVolume_Impl(&token), OS_FS_ERR_PATH_INVALID);

/* Mount dir does exist and is a directory */
memset(&statbuf, 0, sizeof(statbuf));
statbuf.st_mode = OCS_S_IFDIR;
UT_SetDataBuffer(UT_KEY(OCS_stat), &statbuf, sizeof(statbuf), false);
OSAPI_TEST_FUNCTION_RC(OS_FileSysMountVolume_Impl(&token), OS_SUCCESS);
}

void Test_OS_FileSysUnmountVolume_Impl(void)
Expand All @@ -145,6 +171,10 @@ void Test_OS_FileSysUnmountVolume_Impl(void)
*/
OS_object_token_t token = UT_TOKEN_0;

memset(&OS_filesys_table[0], 0, sizeof(OS_filesys_table[0]));
OS_filesys_table[0].fstype = OS_FILESYS_TYPE_NORMAL_DISK;
strcpy(OS_filesys_table[0].system_mountpt, "/ut");

OSAPI_TEST_FUNCTION_RC(OS_FileSysUnmountVolume_Impl(&token), OS_SUCCESS);

UT_SetDefaultReturnValue(UT_KEY(OCS_open), -1);
Expand All @@ -154,6 +184,10 @@ void Test_OS_FileSysUnmountVolume_Impl(void)
UT_SetDefaultReturnValue(UT_KEY(OCS_ioctl), -1);
OSAPI_TEST_FUNCTION_RC(OS_FileSysUnmountVolume_Impl(&token), OS_ERROR);
UT_ClearForceFail(UT_KEY(OCS_ioctl));

/* Additional cases for the FS_BASED handling (no op on unmount) */
OS_filesys_table[0].fstype = OS_FILESYS_TYPE_FS_BASED;
OSAPI_TEST_FUNCTION_RC(OS_FileSysUnmountVolume_Impl(&token), OS_SUCCESS);
}

void Test_OS_FileSysStatVolume_Impl(void)
Expand Down