From: "Cole Helbling" <cole.e.helbling@outlook.com>
To: "Alyssa Ross" <hi@alyssa.is>, <devel@spectrum-os.org>
Cc: Cole Helbling <cole.e.helbling@outlook.com>
Subject: Re: [PATCH wayland v3] Support virtio_wl display sockets
Date: Mon, 27 Jul 2020 11:37:42 -0700 [thread overview]
Message-ID: <SJ0PR03MB55818EFC06F0E5D44E324224B3720@SJ0PR03MB5581.namprd03.prod.outlook.com> (raw)
In-Reply-To: <20200726060358.24277-1-hi@alyssa.is>
On Sat Jul 25, 2020 at 11:03 PM PDT, Alyssa Ross wrote:
> This patch adds libvirtio_wl, which exposes reimplementations of the
> sendmsg(2) and recvmsg(2) for virtio_wl socket fds. Tests for as much
> libvirtio_wl functionality as is reasonably possible to test without
> requiring a working virtio_wl connection are included. (Testing
> further would require running tests in a VM so that they could talk to
> the virtio_wl kernel driver, which would be prohibitively complex.
>
> virtio_wl socket fds do not actually point to sockets, but to special
> virtio_wl files. Whenever a display socket operation calls sendmsg()
> or recvmsg() with a file descriptor and receives an ENOTSOCK error, it
> retries the operation with the equivalent libvirtio_wl function, in
> case the file descriptor is a virtio_wl. This is the least invasive
> way to implement virtio_wl support -- if a normal socket is being
> used, there will be no change in behaviour.
>
> Because virtio_wl doesn't implement every socket feature, some
> workarounds are currently required to accomodate everything Wayland
> expects of sockets:
>
> * virtio_wl_recvmsg implements the MSG_DONTWAIT flag by setting
> O_NONBLOCK on the fd, attempting the VIRTWL_IOCTL_RECV operation,
> and then restoring the fd's original flags. This is obviously
> race-prone, but there is no reasonable alternative at present.
>
> * virtio_wl_sendmsg requires MSG_NOSIGNAL to be set, and ignores it.
> This is because I think from looking at the code that virtio_wl does
> not generate SIGPIPE signals, nor does it ever return EPIPE. I
> could be wrong about this, though.
>
> * virtio_wl does not support credential passing -- what would it even
> mean, considering the other end of the connection is on
> another (virtual) machine? So wl_client's ucred member will have
> pid, uid, and gid all set to -1 for a client connected over
> virtio_wl.
>
> * virtio_wl sockets do not support accept(2), so a fallback is used.
> A proxy program on the host accept(2) on a host socket. When it
> receives a connection, it attaches the connection socket to the VM,
> then sends the name of the connected socket over the Wayland display
> socket. Wayland then receives this name, looks up the connection
> socket, and uses that as the client connection socket.
>
> Additionally, virtio_wl memfd-like file descriptors don't support
> mremap(2), so for virtio_wl sockets Wayland will munmap(2) the memfd,
> and then mmap(2) it again. This should be at least mostly okay
> because Wayland only ever calls mremap with MREMAP_MAYMOVE, but it is
> still race-prone. To be able to do this, memfds are no longer closed
> after being mmaped, but are kept around in the wl_shm_pool struct, so
> that they can be passed to mmap() if required.
>
> This patch appears to be enough to reliably run Alacritty on the host
> system,
> with a Wayland compositor running in a VM. It is not capable of running
> Firefox, which most of the time fails to start, and occassionally will
> partially
> start (sometimes even getting far enough to draw a window) before
> freezing.
> ---
> Cole pointed out on IRC that previous versions of this patch
> unnecessarily included string.h and stdio.h in src/wayland.os.c.
> These includes have now been removed.
>
> src/connection.c | 4 +
> src/meson.build | 16 +-
> src/virtio_wl.c | 344 +++++++++++++++++++++++
> src/virtio_wl.h | 23 ++
> src/wayland-os.c | 21 ++
> src/wayland-os.h | 3 +
> src/wayland-server.c | 20 +-
> src/wayland-shm.c | 24 +-
> tests/meson.build | 1 +
> tests/virtio_wl-test.c | 615 +++++++++++++++++++++++++++++++++++++++++
> 10 files changed, 1062 insertions(+), 9 deletions(-)
> create mode 100644 src/virtio_wl.c
> create mode 100644 src/virtio_wl.h
> create mode 100644 tests/virtio_wl-test.c
Style and friends LGTM. As before, I'm not that great at grokking C, so
I'll just hope you know what you are doing. :P
Reviewed-by: Cole Helbling <cole.e.helbling@outlook.com>
prev parent reply other threads:[~2020-07-27 18:39 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-08 15:16 [PATCH wayland] " Alyssa Ross
2020-07-08 15:43 ` Cole Helbling
2020-07-08 21:07 ` [PATCH wayland v2] " Alyssa Ross
2020-07-26 6:03 ` [PATCH wayland v3] " Alyssa Ross
2020-07-27 18:37 ` Cole Helbling [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=SJ0PR03MB55818EFC06F0E5D44E324224B3720@SJ0PR03MB5581.namprd03.prod.outlook.com \
--to=cole.e.helbling@outlook.com \
--cc=devel@spectrum-os.org \
--cc=hi@alyssa.is \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://spectrum-os.org/git/crosvm
https://spectrum-os.org/git/doc
https://spectrum-os.org/git/mktuntap
https://spectrum-os.org/git/nixpkgs
https://spectrum-os.org/git/spectrum
https://spectrum-os.org/git/ucspi-vsock
https://spectrum-os.org/git/www
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).