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

Jump to bottom

lib/posix-fdio: Move non-trivial libc syscall wrappers over from vfscore #1392

Merged
merged 3 commits into from May 23, 2024
Merged

lib/posix-fdio: Move non-trivial libc syscall wrappers over from vfscore #1392

merged 3 commits into from May 23, 2024

Conversation

andreittr
Copy link
Contributor

@andreittr andreittr commented Apr 24, 2024 •
edited

Description of changes

This changeset moves the implementations of non-trivial libc wrapper functions for file-related syscalls from vfscore into posix-fdio, where these syscalls are actually implemented, fixing an oversight of the original posix-fdio work.
In addition, two improvements are made to the handling of these functions:

  • pread/pwrite(64) aliasing is now simpler
  • VA args are correctly fetched for every known fcntl command, whether supported or not

Prerequisite checklist

  • Read the contribution guidelines regarding submitting new changes to the project;
  • Tested your changes against relevant architectures and platforms;
  • Ran the checkpatch.uk on your commit series before opening this PR;
  • Updated relevant documentation.

Base target

  • Architecture(s): [e.g. x86_64 or N/A]
  • Platform(s): [e.g. kvm, xen or N/A]
  • Application(s): [e.g. app-python3 or N/A]

Additional configuration

N/A; this change should be transparent to user code

@andreittr
lib/posix-fdio: Move over libc funcs from vfscore
d7abc4e 
This change moves the implementations of non-trivial libc wrapper
functions for file-related syscalls from vfscore into posix-fdio, where
these syscalls are actually implemented.
This was an oversight of the original posix-fdio work.

Signed-off-by: Andrei Tatar <andrei@unikraft.io>
@andreittr
lib/posix-fdio: Clean up pread/pwrite aliasing
e7a7957 
This change reworks the libc function aliasing for pread(64) and
pwrite(64), simplifying it.

Signed-off-by: Andrei Tatar <andrei@unikraft.io>
@andreittr
lib/posix-fdio: Support VA args for all fcntl cmds
e5dbcb2 
This change adds support to the `fcntl` libc wrapper for fetching the
optional argument for all known fcntl cmd values.

Signed-off-by: Andrei Tatar <andrei@unikraft.io>
@andreittr andreittr requested review from a team as code owners April 24, 2024 10:27
@github-actions github-actions bot added area/lib Internal Unikraft Microlibrary lang/c Issues or PRs to do with C/C++ lib/vfscore VFS Core Interface labels Apr 24, 2024
@razvand razvand removed request for a team April 28, 2024 05:33
@razvand razvand added this to the v0.17.0 (Calypso) milestone Apr 28, 2024
robertZamfir601
robertZamfir601 approved these changes May 1, 2024
View reviewed changes
Copy link

@robertZamfir601 robertZamfir601 left a comment

Choose a reason for hiding this comment

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

Looks good to me

Reviewed-by: Robert Zamfir georobi.016@gmail.com

StefanJum
StefanJum reviewed May 17, 2024
View reviewed changes
Copy link
Member

@StefanJum StefanJum left a comment

Choose a reason for hiding this comment

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

Any reason open and openat were not moved to posix-fdio?
All looks good besides that.

@andreittr
Copy link
Contributor Author

Any reason open and openat were not moved to posix-fdio?

Both of these syscalls are centered on paths rather than open files. Paths are inherently linked to the concept of a "filesystem" and do not make sense without it, e.g., in an environment where all your files are pseudo-files (ttys, sockets, pipes, etc.).
As part of the great vfscore refactoring, libraries are given much narrower areas of responsibility; posix-fdio is tasked only with providing input/output & control syscalls for file descriptors. The mechanisms by which these file descriptors are created/opened are left to other libraries.
At the moment all filesystem- (and thus path-related) functionality remains in vfscore, so it remains responsible for open.

This will change soon though, when open, openat, and several dozen other VFS-related syscalls will before long be given a new home under posix-vfs. Stay tuned!

StefanJum
StefanJum approved these changes May 21, 2024
View reviewed changes
Copy link
Member

@StefanJum StefanJum left a comment

Choose a reason for hiding this comment

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

Ah, makes sense, thanks.
Reviewed-by: Stefan Jumarea stefanjumarea02@gmail.com

skuenzer
skuenzer approved these changes May 22, 2024
View reviewed changes
Copy link
Member

@skuenzer skuenzer left a comment

Choose a reason for hiding this comment

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

Thanks!

Approved-by: Simon Kuenzer simon@unikraft.io

@razvand razvand changed the base branch from staging to staging-1392 May 23, 2024 04:26
@razvand razvand merged commit 9468928 into unikraft:staging-1392 May 23, 2024
12 of 14 checks passed
razvand pushed a commit that referenced this pull request May 23, 2024
@andreittr @razvand
lib/posix-fdio: Move over libc funcs from vfscore
5e3f482 
This change moves the implementations of non-trivial libc wrapper
functions for file-related syscalls from vfscore into posix-fdio, where
these syscalls are actually implemented.
This was an oversight of the original posix-fdio work.

Signed-off-by: Andrei Tatar <andrei@unikraft.io>
Reviewed-by: Robert Zamfir <georobi.016@gmail.com>
Reviewed-by: Stefan Jumarea <stefanjumarea02@gmail.com>
Approved-by: Simon Kuenzer <simon@unikraft.io>
GitHub-Closes: #1392
razvand pushed a commit that referenced this pull request May 23, 2024
@andreittr @razvand
lib/posix-fdio: Clean up pread/pwrite aliasing
59a799d 
This change reworks the libc function aliasing for pread(64) and
pwrite(64), simplifying it.

Signed-off-by: Andrei Tatar <andrei@unikraft.io>
Reviewed-by: Robert Zamfir <georobi.016@gmail.com>
Reviewed-by: Stefan Jumarea <stefanjumarea02@gmail.com>
Approved-by: Simon Kuenzer <simon@unikraft.io>
GitHub-Closes: #1392
razvand pushed a commit that referenced this pull request May 23, 2024
@andreittr @razvand
lib/posix-fdio: Support VA args for all fcntl cmds
b4ae6bc 
This change adds support to the `fcntl` libc wrapper for fetching the
optional argument for all known fcntl cmd values.

Signed-off-by: Andrei Tatar <andrei@unikraft.io>
Reviewed-by: Robert Zamfir <georobi.016@gmail.com>
Reviewed-by: Stefan Jumarea <stefanjumarea02@gmail.com>
Approved-by: Simon Kuenzer <simon@unikraft.io>
GitHub-Closes: #1392
@andreittr andreittr deleted the ttr/fdio-libcalls branch May 24, 2024 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skuenzer skuenzer skuenzer approved these changes

@StefanJum StefanJum StefanJum approved these changes

@robertZamfir601 robertZamfir601 robertZamfir601 approved these changes

Assignees

@skuenzer skuenzer

Labels
area/lib Internal Unikraft Microlibrary lang/c Issues or PRs to do with C/C++ lib/vfscore VFS Core Interface
Projects
Unikraft Core Roadmap
Status: ✅ Done!
Milestone
v0.17.0 (Calypso)
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@andreittr @skuenzer @StefanJum @robertZamfir601 @razvand