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

vfs: file system abstraction #5616

Merged
merged 16 commits into from
Mar 9, 2017
Merged

vfs: file system abstraction #5616

merged 16 commits into from
Mar 9, 2017

Conversation

jnohlgard
Copy link
Member

@jnohlgard jnohlgard commented Jul 8, 2016

This PR provides a basic virtual file system (VFS) layer for RIOT. The intention is to provide a much simpler layer than what was proposed in #2474 and #1265.

The overall design goals are:

  • Provide implementations for all newlib "file" syscalls
  • Keep it simple, do not add every possible file operation from Linux VFS.
  • Easy to map existing file system implementations for resource constrained systems onto the VFS layer API
  • Avoid keeping a central enum of all file system drivers that has to be kept up to date with external packages etc.
  • Use POSIX <errno.h> numbers as negative return codes for errors, avoid the global errno variable.
  • Only absolute paths to files (no per-process working directory)
  • No dynamic memory allocation

The file systems for resource constrained systems that I have been looking at (so far) are Contiki CFS and SPIFFS.
I have also been looking at Linux kernel headers and books/articles on writing Linux device drivers for more ideas for the design.

The API should be easy to understand for users who are familiar with the POSIX file functions (open, close, read, write, fstat, lseek).

The VFS layer keeps track of mounted file systems and open files, the vfs_open function searches the array of mounted file systems and dispatches the call to the file system instance with the longest matching mount point prefix.
Subsequent calls to vfs_read, vfs_write, etc will do a look up in the table of open files and dispatch the call to the correct file system driver for handling.

vfs_mount takes a string containing the mount point, a file system driver specification (struct file_system), and an opaque pointer that only the FS driver knows how to use, which can be used to keep driver parameters in order to allow dynamic handling of multiple devices.

Also included is a simple file system implementation of a static const file system called ConstFS. This was created as a tool to assist in unit testing of the VFS layer, and as an exercise in implementing file system drivers, and to provide an example for developers who wish to write their own file system driver to hook into the VFS layer.

TODO:

  • STDIN, STDOUT, STDERR through vfs_read/write
  • Write the newlib syscall wrappers
  • Elaborate the design thoughts in the Doxygen documentation
  • Explain clearly how to use the private_data members when implementing a file system driver
  • auto_init stuff?
  • Shell commands (mount will not be supported due to the dynamic nature of the vfs_mount function)
    • read file
    • write file
    • list mounted file systems and space utilization (vfs df)
    • list files in a directory
  • Improve constness to allow for keeping large structures like vfs_file_ops_t in flash.
  • ConstFS: Implement fstat, lseek, update read to use the file offset
  • Mutex for the open files table
  • fcntl - change flags on already open FDs
  • ioctl - communicate with special devices - not needed since kernel is rebuilt every time, easy to use device driver directly
  • stat (may wrap open+fstat+close sequence as a fall back)
  • statvfs, fstatvfs - information about a mounted file system
  • More unit tests
    • vfs_open
    • vfs_close
    • vfs_read
    • vfs_write
    • vfs_lseek
    • vfs_fstat
    • vfs_fcntl
    • vfs_mount
    • vfs_umount
    • vfs_rename
    • vfs_unlink
    • vfs_mkdir
    • vfs_rmdir
    • vfs_bind
    • vfs_opendir
    • vfs_closedir
    • vfs_readdir
    • vfs_normalize_path
    • vfs_iterate_mounts
    • vfs_stat
    • vfs_statvfs
    • vfs_fstatvfs
    • newlib syscall wrappers
  • DevFS device file system
    • basic functionality
    • auto_init for mounting
  • DevFS support in drivers
  • import xxd-like dumper from other branch as shell command
  • Directory support
    • opendir
    • readdir
    • closedir
    • unit tests for directories
  • vfs_rename

Not implemented:

  • Simultaneous access to the same file.

How to try it

Right now the only place to run the code is through a newly added embunit unit test batch:

make -C tests/unittests tests-vfs term BOARD=native

(also successfully tested on BOARD=mulle)

Open questions

  • Does it make sense to add an "inode" layer to separate the file name handling from the file opening?
  • Should mode be given as mode_t or int? (newlib uses int, POSIX uses mode_t which is unsigned long on 32 bit platforms)
  • Should mounted file systems be changed into a linked list? (originally was statically allocated array with fixed maximum mount point length) - implemented

@jnohlgard jnohlgard added State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Type: new feature The issue requests / The PR implemements a new feature for RIOT Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR labels Jul 8, 2016
@jnohlgard jnohlgard mentioned this pull request Jul 8, 2016
6 tasks
@kaspar030
Copy link
Contributor

Nice! Thank you for tackling this. Looks very nice already!

Some thoughts that came up while skimming the diff:

  • did you consider extending structs by embedding instead of using pointers, (like netdev2 does)?
  • any reason you're using "struct xxx..." within structs, instead of the typedefs?

@kaspar030 kaspar030 self-assigned this Jul 9, 2016
@jnohlgard
Copy link
Member Author

jnohlgard commented Jul 9, 2016

@kaspar030

  • did you consider extending structs by embedding instead of using pointers, (like netdev2 does)?

My thoughts were that some of these structs should be constant and static, e.g. the file system driver, and should not consume valuable RAM, while other parts are variable. Right now I think all of it is kept in RAM, but the goal is to keep the static parts in flash instead and using pointers from the RAM parts to the flash parts.
-- updated: file operations and file system operations are const and kept in flash

  • any reason you're using "struct xxx..." within structs, instead of the typedefs?

No particular reason, I started using them in one place to get a pointer to the same type of struct, and then it just kind of kept on. I'll see if it still compiles if I modify it to use the typedefs instead.
-- updated: modified to use structname_t instead of struct structname

off += filp->pos;
break;
case SEEK_END:
off += fp->size;
Copy link
Contributor

Choose a reason for hiding this comment

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

break

Copy link
Member Author

Choose a reason for hiding this comment

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

addressed

@jnohlgard
Copy link
Member Author

updated todo-list with some more ideas.

@vincent-d
Copy link
Member

This looks very nice.

I was working on the same topic, but my work is much less advanced than yours. I'm trying to get SPIFFS working and we worked on a low level flash interface as well. I'll push some work soon and will try to rebase my SPIFFS port on top of your vfs.

Thanks for that work

@jnohlgard
Copy link
Member Author

@vincent-d Nice! I have had plans to add SPIFFS but I did not feel that I knew where to begin, so I started with this instead, it's good to see that others are working towards the same goal.
I am currently under way to add a device file system, DevFS, similar to /dev on Linux, in order to be able to access low level devices from the shell, such as NVRAM. Maybe we can fit the generic flash driver onto that as well? (see TODO in top post)

@jnohlgard
Copy link
Member Author

squashed, WIP commit list was getting out of hand.

@jnohlgard jnohlgard force-pushed the pr/vfs branch 2 times, most recently from 518d49b to 7549a62 Compare July 12, 2016 08:05
@kaspar030
Copy link
Contributor

How do we deal with directories?

@jnohlgard
Copy link
Member Author

Updated with more work on DevFS, including:

  • DevFS auto_init mounts /dev
  • Shell command vfs to access the VFS mounted file systems from the shell
  • Newlib fixes
  • basic fcntl implementation
  • VFS file operations implementation for /drivers/nvram to access the contents of NVRAM devices via VFS.
  • Configured NVRAM VFS on DevFS for Mulle (/dev/mulle-fram).
  • improved unit tests

@jnohlgard
Copy link
Member Author

jnohlgard commented Jul 12, 2016

@kaspar030 not sure. CFS and SPIFFS both implement specific opendir, closedir, readdir. Maybe those functions should be added as part of vfs_file_system_ops_t, what is your opinion?

@jnohlgard jnohlgard added this to the Release 2016.10 milestone Jul 12, 2016
sys/vfs/vfs.c Outdated
return -ENOENT;
}
/* Increment open files counter for this mount */
atomic_inc(&_vfs_mounts[md].open_files);
Copy link
Member

Choose a reason for hiding this comment

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

This should not be done here but only in vfs_open, otherwise the unlink also increments the counter and umount fails afterwards.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, this is on purpose as a kind of semaphore to avoid races between umount and open/unlink. There were a few missing decrements of open_files in vfs_unlink though that I have fixed now.

@jnohlgard
Copy link
Member Author

jnohlgard commented Jul 12, 2016

I'll do an update tomorrow morning with opendir, readdir, closedir support for devfs and constfs (and the VFS support code of course)

@jnohlgard
Copy link
Member Author

Updated with the directory interface. No unit tests yet or ConstFS, DevFS support for directories.

@miri64
Copy link
Member

miri64 commented Jul 13, 2016

I know FAT is not the most ideal FS for our purposes with all its layers of backwards compatibility, but we also should consider porting the FatFS library as a package using this interface. After all: FAT is still the file system type most SD cards are shipped with. I saw this library used on systems that are way more constraint than the ones RIOT targets (the CCC's r0ket badge e.g.), so space shouldn't be an issue.

@miri64
Copy link
Member

miri64 commented Mar 8, 2017

Does this need a re-ACK?

@jnohlgard
Copy link
Member Author

@miri64 Needs at least someone to approve the workaround for the mips board in the beginning of fcntl.h and statvfs.h

@jnohlgard
Copy link
Member Author

@vincent-d Does your ACK hold?

@vincent-d
Copy link
Member

Yes! ACK

@miri64
Copy link
Member

miri64 commented Mar 9, 2017

qemu error on Jarvis seems unrelated and Murdock (1) is still the imperative CI, so merge at will.

@jnohlgard
Copy link
Member Author

Yay! :D

@jnohlgard jnohlgard merged commit 887cc72 into RIOT-OS:master Mar 9, 2017
@jnohlgard jnohlgard deleted the pr/vfs branch March 9, 2017 08:45
@miri64
Copy link
Member

miri64 commented Mar 9, 2017

Congratz \o/

@vincent-d
Copy link
Member

Great!!

@kaspar030
Copy link
Contributor

Congrats and a lot of thanks to @gebart! Good job!

@emmanuelsearch
Copy link
Member

great!!

@kYc0o
Copy link
Contributor

kYc0o commented Mar 9, 2017

Super! Thanks @gebart !

@jnohlgard jnohlgard added the Area: fs Area: File systems label Feb 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: fs Area: File systems CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.