Merged
Conversation
dancrossnyc
approved these changes
Apr 30, 2026
Contributor
dancrossnyc
left a comment
There was a problem hiding this comment.
The sound you hear is me, banging my head against the table and groaning loudly.
lgfa29
approved these changes
May 1, 2026
05044d4 to
fee6a9f
Compare
When we're going to get virtio-nic device running after initialization or import, part of `queues_restart()` involves synchronizing the state of viona rings with the Propolis-side state for corresponding virtqueues; we're about to hand off operation of those queues to viona after all. When we stop a virtio-nic device, `queues_sync()` synchronizes viona ring state back into Propolis via VNA_IOC_RING_GET_STATE, and when exported we correctly retain all queue state. On import we had omitted to set vrs_avail_idx and vrs_used_idx. The guest-side state for the wring then is written on top of the previously-exported memory, clobbering correct state with the now reset-to-initial queue indices. Resuming the guest at this point is a bit fraught, as there's no reason to believe that the descriptors at such indices are valid. In the lucky case you may catch a Linux guest logging something like: > virtio_net virtio0: input.0:id 0 is not a head! at which point it gives up on the very broken device.
f8063e2 to
bc07cd3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When we're going to get virtio-nic device running after initialization or import, part of
queues_restart()involves synchronizing the state of viona rings with the Propolis-side state for corresponding virtqueues; we're about to hand off operation of those queues to viona after all. When we stop a virtio-nic device,queues_sync()synchronizes viona ring state back into Propolis via VNA_IOC_RING_GET_STATE, and when exported we correctly retain all queue state.On import we had omitted to set vrs_avail_idx and vrs_used_idx. The guest-side state for the wring then is written on top of the previously-exported memory, clobbering correct state with the now reset-to-initial queue indices. Resuming the guest at this point is a bit fraught, as there's no reason to believe that the descriptors at such indices are valid. In the lucky case you may catch a Linux guest logging something like:
at which point it gives up on the very broken device.
... I could probably write a test for this, by setting
vrs_qaddr_avail(and avail) to something and then migrating the device. there's no reason we need to actually send packets anywhere or rx interrupts etc...anyhow, at this point
pingfrom a host elsewhere on my network to the VM, andpingfrom the VM to an external host, both keep working across migration again. yay!!