Conversation
Created using jj-spr 0.1.0
| /// This MUST be called when reseting the virtio-socket device so | ||
| /// that we don't use stale `GuestAddr`s across device resets. | ||
| pub(crate) fn clear_rx_chain(&mut self) { | ||
| self.rx_chain = None; |
There was a problem hiding this comment.
In the future I would like to eliminate this RxPermit by possibly extending the VirtQueue to allow peek() or a method that checks if the chain has a descriptor available.
Created using jj-spr 0.1.0
Created using jj-spr 0.1.0
Created using jj-spr 0.1.0
iximeow
left a comment
iximeow
left a comment
There was a problem hiding this comment.
nice, took me a second to fully process the relationships here but I think this all makes sense. a few notes about how we might want to structure this but I'm morally +1
lib/propolis/src/vsock/poller.rs
Outdated
|
|
||
| self.state.set_stopped(); | ||
| return; |
There was a problem hiding this comment.
comment about parceling out functions motivated in part by "oh we'd totally forget to set_stopped() if we had another exit condition huh" :D
|
|
||
| pub fn reset(&self) { | ||
| let (tx, rx) = mpsc::sync_channel(0); | ||
| self.pause_tx.send(PausedCmd::Reset { oneshot: tx }).unwrap(); |
There was a problem hiding this comment.
meta note on the unwraps related to pause_rx and tx/rs on the channel itself: since the event loop thread lives and dies entirely based on these pause_tx transmissions (or port_getn getting told the FD is gone) I'm actually pretty comfortable about these unwraps.
I think I'd move the self.pause_tx.send(_).unwrap() to a function that notes the relationship between the poller thread and these control interfaces though?
in particular, unwraps the other way (the poller thread writing to tx when Propolis has torn down VM objects and dropped the sender) doesn't seem like a risk as long as we don't do something obviously wrong like try to resume() a poller we've halt()'d. I don't have any thoughts about writing that up more concisely, but on the whole I don't see a way we get to these unwraps 🙏
Created using jj-spr 0.1.0
Created using jj-spr 0.1.0
3 participants
This PR extends the
VsockPollerevent loop so that it complies with theLifecycletrait within propolis.The virtio device now requests a pause to the event-loop by sending a
VsockEvent::Pauseviaport_send(3C).When the event loop receives this event it finishes processing any tasks and then waits on a mpsc channel. This channel is responsible for processing follow up events driven by the virtio device lifecycle such as resume, reset, and halt. The reset event takes care of cleaning up all state internal to
VsockPollerin addition to removing the cached descriptor chain inVsockVq. It's important we drop this cached descriptor as using it across device resets would cause propolis to scribble data into a random location in guest memory depending on where theGuestAddrpoints.Finally with this work in place it should make solving #1065 trivial.