[PATCH v3 0/3] vhost-user: warn when guest RAM is not shared

Stefan Hajnoczi posted 3 patches 2 years, 9 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210714092946.569516-1-stefanha@redhat.com
include/exec/memory.h         | 11 +++++++++++
hw/virtio/vhost-user.c        | 20 ++++++++++++++++----
softmmu/memory.c              |  6 ++++++
tests/qtest/vhost-user-test.c |  2 +-
4 files changed, 34 insertions(+), 5 deletions(-)
[PATCH v3 0/3] vhost-user: warn when guest RAM is not shared
Posted by Stefan Hajnoczi 2 years, 9 months ago
v3:
 * Rebased
v2:
 * Clarify that object-memory-memfd enables share=on by default [Marc-André]
 * Rebased

vhost-user requires -object memory-backend-*,share=on option so that QEMU uses
mmap(MAP_SHARED) on guest RAM that is shared with the vhost-user device backend
process. This is needed so the QEMU process sees changes made by the vhost-user
device backend process, and vice versa.

Today QEMU and the vhost-user device process will start up and then fail with a
confusing error message if the user forgot to specify share=on.

This patch series adds a warning letting the user know that share=on is
required.

Stefan Hajnoczi (3):
  tests/qtest/vhost-user-test: use share=on with memfd
  memory: add memory_region_is_mapped_shared()
  vhost-user: warn when guest RAM is not shared

 include/exec/memory.h         | 11 +++++++++++
 hw/virtio/vhost-user.c        | 20 ++++++++++++++++----
 softmmu/memory.c              |  6 ++++++
 tests/qtest/vhost-user-test.c |  2 +-
 4 files changed, 34 insertions(+), 5 deletions(-)

-- 
2.31.1

Re: [PATCH v3 0/3] vhost-user: warn when guest RAM is not shared
Posted by Kevin Wolf 2 years, 9 months ago
Am 14.07.2021 um 11:29 hat Stefan Hajnoczi geschrieben:
> v3:
>  * Rebased
> v2:
>  * Clarify that object-memory-memfd enables share=on by default [Marc-André]
>  * Rebased
> 
> vhost-user requires -object memory-backend-*,share=on option so that QEMU uses
> mmap(MAP_SHARED) on guest RAM that is shared with the vhost-user device backend
> process. This is needed so the QEMU process sees changes made by the vhost-user
> device backend process, and vice versa.
> 
> Today QEMU and the vhost-user device process will start up and then fail with a
> confusing error message if the user forgot to specify share=on.
> 
> This patch series adds a warning letting the user know that share=on is
> required.

Hmm... This seems to work when I use shared=off for the memory backend,
even though the error message is easy to miss because more error
messages follow:

    $ LANG=C build/qemu-system-x86_64 -object memory-backend-memfd,id=mem,size=1G,share=off -m 1G -M memory-backend=mem -chardev socket,path=/tmp/vhost.sock,id=vhost1 -device vhost-user-blk-pci,chardev=vhost1
    qemu-system-x86_64: -device vhost-user-blk-pci,chardev=vhost1: warning: Found vhost-user memory region without MAP_SHARED (did you forget -object memory-*,share=on?)
    qemu-system-x86_64: Unexpected end-of-file before all data were read
    qemu-system-x86_64: Failed to set msg fds.
    qemu-system-x86_64: vhost VQ 0 ring restore failed: -1: Resource temporarily unavailable (11)

However, if I don't create the memory backend at all (which is probably
the most common error users will make), then the new message isn't
printed:

    $ LANG=C build/qemu-system-x86_64 -m 1G -chardev socket,path=/tmp/vhost.sock,id=vhost1 -device vhost-user-blk-pci,chardev=vhost1
    qemu-system-x86_64: Unexpected end-of-file before all data were read
    qemu-system-x86_64: Failed to set msg fds.
    qemu-system-x86_64: vhost VQ 0 ring restore failed: -1: Resource temporarily unavailable (11)

Wouldn't we want a warning for the second case, too?

Kevin


Re: [PATCH v3 0/3] vhost-user: warn when guest RAM is not shared
Posted by Stefan Hajnoczi 2 years, 9 months ago
On Tue, Jul 20, 2021 at 01:32:43PM +0200, Kevin Wolf wrote:
> Am 14.07.2021 um 11:29 hat Stefan Hajnoczi geschrieben:
> > v3:
> >  * Rebased
> > v2:
> >  * Clarify that object-memory-memfd enables share=on by default [Marc-André]
> >  * Rebased
> > 
> > vhost-user requires -object memory-backend-*,share=on option so that QEMU uses
> > mmap(MAP_SHARED) on guest RAM that is shared with the vhost-user device backend
> > process. This is needed so the QEMU process sees changes made by the vhost-user
> > device backend process, and vice versa.
> > 
> > Today QEMU and the vhost-user device process will start up and then fail with a
> > confusing error message if the user forgot to specify share=on.
> > 
> > This patch series adds a warning letting the user know that share=on is
> > required.
> 
> Hmm... This seems to work when I use shared=off for the memory backend,
> even though the error message is easy to miss because more error
> messages follow:
> 
>     $ LANG=C build/qemu-system-x86_64 -object memory-backend-memfd,id=mem,size=1G,share=off -m 1G -M memory-backend=mem -chardev socket,path=/tmp/vhost.sock,id=vhost1 -device vhost-user-blk-pci,chardev=vhost1
>     qemu-system-x86_64: -device vhost-user-blk-pci,chardev=vhost1: warning: Found vhost-user memory region without MAP_SHARED (did you forget -object memory-*,share=on?)
>     qemu-system-x86_64: Unexpected end-of-file before all data were read
>     qemu-system-x86_64: Failed to set msg fds.
>     qemu-system-x86_64: vhost VQ 0 ring restore failed: -1: Resource temporarily unavailable (11)
> 
> However, if I don't create the memory backend at all (which is probably
> the most common error users will make), then the new message isn't
> printed:
> 
>     $ LANG=C build/qemu-system-x86_64 -m 1G -chardev socket,path=/tmp/vhost.sock,id=vhost1 -device vhost-user-blk-pci,chardev=vhost1
>     qemu-system-x86_64: Unexpected end-of-file before all data were read
>     qemu-system-x86_64: Failed to set msg fds.
>     qemu-system-x86_64: vhost VQ 0 ring restore failed: -1: Resource temporarily unavailable (11)
> 
> Wouldn't we want a warning for the second case, too?

Yes. I think so. I'll investigate.

Stefan