PR for Rawposix(dispatcher.rs, fs_call.rs, misc.rs) #47
Conversation
robinyuan1002
requested review from
rennergade,
yzhang71,
Yaxuan-w and
qianxichen233
src/lib.rs
Outdated
| } | ||
| } | ||
|
|
||
| pub fn lind_fork(parent_cageid: u64, child_cageid: u64) -> i32 { |
There was a problem hiding this comment.
add comments to explain why those three are special
There was a problem hiding this comment.
I thought we were removing these?
There was a problem hiding this comment.
IMO all of this should just be consolidated through the dispatcher, and if not it needs a comment with a good reason.
There was a problem hiding this comment.
These functions in lib.rs (e.g. lind_fork, lind_exit, lind_exec) are used by multi-process in wasmtime for better readability. Since fork, exit and exec syscall is intercepted and handled internally within wasmtime, so I write a wrapper for these three syscalls. In this way, when I need to call the rawposix fork in clone syscall in wasmtime, I can use lind_fork instead of something like lind_syscall_api(cageid, 68, ..., child_cageid, NOTUSED, NOTUSED, ...), which looks a little bit redundent
There was a problem hiding this comment.
Don't we use lind_syscall_api everywhere else? I see no reason to do this or why the other functions were moved out of the dispatcher. The lib.rs file should look as it did before.
There was a problem hiding this comment.
lib.rs is now cleaned up
src/interface/comm.rs
Outdated
| @@ -641,4 +641,4 @@ pub fn kernel_select( | |||
| }; | |||
|
|
|||
| return result; | |||
| } | |||
| } | |||
|
I think besides some minutiae the biggest thing here is figuring out whats going on in lib.rs I'm still very confused about that. |
src/safeposix/dispatcher.rs
Outdated
| @@ -201,252 +176,655 @@ pub extern "C" fn rustposix_thread_init(cageid: u64, signalflag: u64) { | |||
| interface::signalflag_set(signalflag); | |||
| } | |||
|
|
|||
| /// The `lind_syscall_api` function acts as the central handler for executing various system calls | |||
There was a problem hiding this comment.
this isn't a very helpful comment, can you talk more about how this function works and less about Lind as a whole?
There was a problem hiding this comment.
I think the syscall api comment needs to be more informative. Perhaps @qianxichen233 can help. Other than that you need to fix the merge conflict then its good to go.
src/safeposix/dispatcher.rs
Outdated
| @@ -137,7 +142,6 @@ macro_rules! get_onearg { | |||
| } | |||
| }; | |||
| } | |||
|
|
|||
| //this macro takes in a syscall invocation name (i.e. cage.fork_syscall), and all of the arguments | |||
There was a problem hiding this comment.
do we not use these macros anymore? if not we should remove them
src/safeposix/dispatcher.rs
Outdated
| let buf = (start_address + arg2) as *const u8; | ||
| let count = arg3 as usize; | ||
| interface::check_cageid(cageid); | ||
| unsafe { |
There was a problem hiding this comment.
I didn't realize we were using unsafe to do all these calls, I think this is probably bad. Why do we do this?
modify dispatcher.rs and type.rs to support argument type conversion, mention we have three special syscall fork/exec/exit with used by wasmtime so it won't pass the regular dispatcher path.
add implementation of futex and nanosleep.