viona: multiqueue device should stay multiqueue across migration#1121
viona: multiqueue device should stay multiqueue across migration#1121
Conversation
|
between 70662f7 and 0b0566f I noticed an exciting issue very much like #1045: rebooting a guest would cause the PCI device to disappear. since this would have been, if everything else was in a happy state, a bug introduced by #1047... |
| delete_vnic(&vnic_name); | ||
| create_vnic(&underlying_nic, &vnic_name); |
There was a problem hiding this comment.
this is incidental but seems worth keeping: I was trying to chase out any sources of anything potentially sticking around across the "migrated" VMs, and the test vnic itself "could" stick around but really shouldn't.
the dladm commands won't (shouldn't?) block or be blocked by test operations, so I don't super love blocking the runtime like this but I'm also not worried about it..
lgfa29
left a comment
There was a problem hiding this comment.
Giving mostly a cerimonial ✅ since I'm not too familiar with this part of the code, but the general logic makes sense to me.
We correctly export and import the propolis-side state given a multiqueue VirtIO device, but we did not communicate that through to viona. On import the virtio-nic device has only told viona it will use one queue pair; we've skipped the "normal" set_features() in favor of setting features on the handle directly. Setting an imported multi-queue device running at this point will reset the many queues, but viona rings are not in a resettable state, fail to reset, and set the device to NEEDS_RESET immediately. Communicating the correct number of queue pairs to viona is a clear improvement, but we're not quite out of bugs yet..
05044d4 to
fee6a9f
Compare
|
(the force push was to pick up the dropshot-api-manager bump) |
We correctly export and import the propolis-side state given a multiqueue VirtIO device, but we did not communicate that through to viona. On import the virtio-nic device has only told viona it will use one queue pair; we've skipped the "normal" set_features() in favor of setting features on the handle directly. Setting an imported multi-queue device running at this point will reset the many queues, but viona rings are not in a resettable state, fail to reset, and set the device to NEEDS_RESET immediately.
Communicating the correct number of queue pairs to viona is a clear improvement, but we're not quite out of bugs yet..