vhost-user-backend: fetch 'used' index from guest #180
Conversation
This file contains 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
germag
requested review from
eryugey,
jiangliu,
sboeuf,
slp and
stefano-garzarella
as code owners
sboeuf
reviewed
Aug 8, 2023
germag
force-pushed
the
fetch_used_index
branch
from
c3f481e
to
4132759
Compare
|
v2:
|
commit 958cdec stopped GET_VRING_BASE from resetting the vring. This was required because when the VM is stopped/resumed the index will be 0 instead of the correct value. However, after the guest driver changes, for instance, after reboot, the 'used' index should be reset to 0. The bug fixed in that commit had the unintended side effect of setting the 'used' index to 0 after a driver change. QEMU's vhost-user library sets the 'used' index when receiving SET_VRING_ADDR, _probably_ to make sure that the VQ is configured. Perhaps the appropriate place is when receiving the first kick. A better solution would be to send the 'used' index in SET_VRING_BASE, as is done when using packed VQs. To keep compatibility with QEMU and just in case, any implementation expects the 'used' index to be set when receiving SET_VRING_ADDR let's fetch the 'used' index from the guest when receiving that message. Signed-off-by: Hanna Czenczek <hreitz@redhat.com> Signed-off-by: German Maglione <gmaglione@redhat.com>
germag
force-pushed
the
fetch_used_index
branch
from
4132759
to
a27046d
Compare
|
v3:
|
stefano-garzarella
approved these changes
Aug 9, 2023
sboeuf
approved these changes
Aug 9, 2023
|
FYI PR opened here with the release: #183 We can discuss there. |
stefano-garzarella
added a commit
to stefano-garzarella/vhost-device
that referenced
this pull request
Aug 22, 2023
vhost-user-backend v0.10.0 introduced an issue that affects all vhost-user backends. I easily reproduced the problem with vhost-device-vsock: just restart the guest kernel and the device no longer works. vhost-user-backend v0.10.1 includes the fix [1] for that issue. [1] rust-vmm/vhost#180 Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
vireshk
pushed a commit
to rust-vmm/vhost-device
that referenced
this pull request
Aug 22, 2023
vhost-user-backend v0.10.0 introduced an issue that affects all vhost-user backends. I easily reproduced the problem with vhost-device-vsock: just restart the guest kernel and the device no longer works. vhost-user-backend v0.10.1 includes the fix [1] for that issue. [1] rust-vmm/vhost#180 Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
birktj
pushed a commit
to birktj/virtiofsd
that referenced
this pull request
Jan 5, 2024
The new version of vhost-user-backend fetches the used idx from the guest[0], without this fix the guest hangs after a reboot, because the used idx is not reset. This error was introduced when we fixed GET_VRING_BASE to incorrectly reset the vring[1], which left the vring in an inconsistent state after a stop/cont cycle. Resetting the VQ when receiving GET_VRING_BASE had the unintended side effect of setting the 'used' index to 0 after a driver change (e.g., after a reboot). [0] rust-vmm/vhost#180 (commit: 8a4ba9d0c5666075aa7f7e2e32ceeaf8d827f9da) [1] rust-vmm/vhost#161 (commit: 958cdec2b8741af77b7f90214ec9c97040bf5a8a)
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.
Summary of the PR
commit 958cdec stopped GET_VRING_BASE from resetting the vring. This was required because when the VM is stopped/resumed the index will be 0 instead of the correct value.
However, after the guest driver changes, for instance, after reboot, the 'used' index should be 0. The bug fixed in that commit had the unintended side effect of setting the 'used' index to 0 after a driver change.
QEMU sets the 'used' index when receiving SET_VRING_ADDR, probably to make sure that the VQ is configured. Perhaps the appropriate place is when receiving the first kick. A better solution would be to send the used index in SET_VRING_BASE, as is done when using packed VQs.
To keep compatibility with QEMU and just in case, any implementation expects the 'used' index to be set when receiving SET_VRING_ADDR let's fetch the 'used' index from the guest when receiving that message.