Conversation
|
Agree that portalcalls are a useful and portable out-of-band communication mechanism and could definitely help here. If we wanted to move away from virtio-vsock, we could do this whole thing using portalcalls. We shouldn't require a response from indiv_debug.py in all cases, though. Maybe we can add an argument to hear back a plugin or specific call number. |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a Rust module for making portal calls (a custom syscall interface) and integrates it into the main application initialization. The implementation uses sendto syscall with a magic number to communicate with what appears to be a debugging interface in a virtualized environment.
Key Changes:
- New
portalcall.rsmodule providing wrapper functions for syscall-based portal communication - Integration into main application startup to notify
indiv_debug.pyin the penguin environment - Warning logged if portal call initialization fails
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/portalcall.rs | Implements portal call syscall wrapper and convenience functions for 1-5 arguments |
| src/main.rs | Adds portal call module import and initialization check on startup |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| pub fn portal_call(user_magic: u64, argc: i32, args: &[u64]) -> i64 { | ||
| unsafe { | ||
| syscall( | ||
| SYS_sendto, | ||
| PORTAL_MAGIC, | ||
| user_magic, | ||
| argc, | ||
| args.as_ptr(), |
There was a problem hiding this comment.
The argc parameter is passed as an i32 but the function doesn't validate that args.len() matches argc. This could lead to buffer overruns or incorrect behavior if the caller passes mismatched values. Consider using args.len() directly or adding validation: assert_eq!(argc as usize, args.len()).
| @@ -0,0 +1,51 @@ | |||
| use libc::{syscall, SYS_sendto}; | |||
|
|
|||
There was a problem hiding this comment.
The PORTAL_MAGIC constant lacks documentation explaining its purpose and origin. Consider adding a comment describing what this magic number represents and why it's used for the portal call interface.
| // PORTAL_MAGIC is a magic number used to identify portal calls via the syscall interface. | |
| // The value 0xc1d1e1f1 is chosen to be unique and unlikely to conflict with other syscall usages. | |
| // It acts as a signature for the portal call protocol, ensuring that only intended calls are processed. |
| mod portalcall; | ||
|
|
||
| const BUF_SIZE: usize = 65536; | ||
| const CMD_TIMEOUT: Duration = Duration::from_secs(10); |
There was a problem hiding this comment.
The INDIV_DEBUG_PORTALCALL_MAGIC constant lacks documentation explaining its purpose and relationship to the indiv_debug.py interface mentioned in the PR description. Consider adding a comment documenting this magic number's meaning.
| const CMD_TIMEOUT: Duration = Duration::from_secs(10); | |
| const CMD_TIMEOUT: Duration = Duration::from_secs(10); | |
| /// Magic number used to initialize the portal call interface. | |
| /// This value must match the expected magic in the `indiv_debug.py` interface, | |
| /// and is used to identify and authenticate debug portal calls between Rust and Python. | |
| /// See PR description and `indiv_debug.py` for protocol details. |
This PR adds a module for portalcall to rust.
Further, it requires that portalcall make a notification to indiv_debug.py in penguin and that it respond correctly.
That second part might not be what you want. It's safer for some things, but it's worse for backwards compat and whatnot.
I do think the portalcall stuff could be more broadly useful for this project. Having a portalcall general use logger could help pass along useful information if something failed in the process. That way we could get penguin output from the process directly.