genodelabs / genode

Genode OS Framework
https://genode.org/
Other
1.07k stars 254 forks source link

9P network filesystem client #1478

Closed ehmry closed 9 years ago

ehmry commented 9 years ago

http://9p.cat-v.org/

Its like lx_fs, but for non-Linux platforms.

Tested with u9fs QEMU, NOVA, and the Libc VFS test. Run file is in progress. I've been using u9fs server-side, another option would be Diod. http://9p.cat-v.org/implementations https://github.com/chaos/diod

The message marshalling should be refactored, but the transaction flow is there and it works.

ehmry commented 9 years ago

I've not been able to get a run script working, I've been using a custom test harness, but if anyone wants to give it a shot theres what I've got.

cnuke commented 9 years ago

Awesome! I only skimmed the implementation, so I cannot comment on it yet but as a first remark, the server should be moved from os to the gems repository because it depends on the libc and therefore should not be part of the os repo.

Regarding the run script, I would probably go for a scenario which uses u9fs to provide the files, i.e., the contents of the build directory. That would fit well into our automated testing facility.

ehmry commented 9 years ago

Will do

ehmry commented 9 years ago

Looks like this client isn't compatible with the 9P2000.L protocol that diod and v9fs uses, so its u9fs or a Plan9 fileserver. I personally prefer u9fs because its simplier.

nfeske commented 9 years ago

Very nice!

When looking over the patch 557ebbaad0d0aba7d471beabe0b8c6f33e449f30, the following questions popped up in my mind:

  1. Some files are missing license headers. In the '3p.h' case, the top comment states that this file is "Adapted from plan9front/sys/include/fcall.h". Would it be appropriate to add a Genode license header of would this violate the license of the original code? If the latter is the case, do you have a good idea how to go about it?
  2. You apparently took the ram_fs as blue print, which is absolutely fine. But is also results in a few code duplications that I would like to avoid. Maybe, it would help to move the utilities that are commonly useful for implementing file-system servers (like util.h, node_handle_registry.h) from os/src/server/ram_fs/ to the public location os/include/file_system/? Of course this should be a separate commit preceding the 9p commit.
  3. Apart from the copied headers, there is also quite a bit of overlap found in other files like main.cc. I think that we should investigate a way how to untangle the Genode-specific server code from file-system-specific code.

For merging the patch into mainline, the first and second point should be addressed whereas the third point can be moved to a dedicated issue. What are your thoughts?

ehmry commented 9 years ago

The licensing I skipped because I was lazy, that I can fix without any issues.

I would like to move the node_handle_registry out, because I didn't need to review any of that to get it to work. I'd be happy to adapter the other fs servers at the same time.

I didn't think the duplication in main was a problem, I would prefer a little duplication if it makes it easier to understand how the file system servers work.

ehmry commented 9 years ago

Also, the Listener class should be moved out of node.h and into os/include/file_system, I believe that is the same in the other servers. https://github.com/genodelabs/genode/commit/557ebbaad0d0aba7d471beabe0b8c6f33e449f30#diff-ef69159da5ad732bcae4502c2b3017c0R20

ehmry commented 9 years ago

Sadly a run scenario seems more distant as the u9fs server needs to run as root because it fails if it can't change users.

ehmry commented 9 years ago

Added a README and renamed to 9p_fs. The name is going to be confusing anyway you put it. Also, the directory is named ninep_fs so it can be imported by things that don't like identifiers to start with numerals.

ehmry commented 9 years ago

I have a run script that works with qemu and an already running instance of u9fs.

I have u9fs packaged for Gentoo

emerge -u layman
layman -L
layman -a emery

emerge -u u9fs
nano /etc/xinetd.d/u9fs
/etc/init.d/xinetd start

or something like that.

ehmry commented 9 years ago

Moved to my personal feature repo

https://github.com/ehmry/genode-emery