On Mon, 2026-06-22 at 21:06 +0300, Kostiantyn Kostiuk wrote:
> On Mon, Jun 22, 2026 at 5:17 PM Polina Vishneva <
> [email protected]> wrote:
> 
> > On Mon, 2026-06-22 at 12:00 +0300, Kostiantyn Kostiuk wrote:
> > > Hi Polina,
> > > 
> > > First of all, thanks for your patches.
> > > 
> > > Before reviewing this patch, I want to ask: why is it needed?
> > > 
> > > I understand the patch "qemu-sockets: Enable AF_VSOCK on Windows",
> > > but AF_VSOCK has the same behavior on Linux and Windows.
> > > I expected that we could totally reuse the POSIX part, so just move it to
> > > common with
> > > some defines like:
> > > ```
> > > #ifdef G_OS_WIN32
> > >         in_ch = g_io_channel_win32_new_fd(in_fd);
> > > #else
> > >         in_ch = g_io_channel_unix_new(in_fd);
> > > #endif
> > > ```
> > > 
> > > We tested our driver with SSH over VSOCK, and standard Linux SSH works
> > > well.
> > > So, what is the difference in this case that you add win32-specific code
> > > for VSOCK?
> > 
> > 1. There's no shared GIOChannel path on Windows to #ifdef into.
> > channel-posix.c is built on GIOChannels + g_io_add_watch();
> > channel-win32.c is built on overlapped I/O over a Win32 HANDLE with a
> > reader thread and a custom GSource. There's no in_ch = g_io_channel_*()
> > line to wrap. vsock is the first GIOChannel-based channel on Windows.
> > 
> > 
> I know that currently windows virtio-sock driver does not
> support overlapped I/O.
> But what if we add it and reuse the same logic with overlapped I/O over a
> Win32 HANDLE?
> What do you think?
> 
> In general, my wish is to simplify new source code and reuse existing one.

That would indeed add uniformity and allow to call platform-specific
functions from the common (not vice versa), but at the cost of making
the win32 code even more complicated.

Sockets get GIOChannel for free, so we just reuse
read/write/listen/accept with channel-posix.c via channel-common.c. The
only win32-specific part left is plugging the socket into the main loop,
which GLib does via WSAEventSelect.

Wrapping a socket with existing overlapped I/O code would only add more
logic and reduce code reuse.

The serial path can't do vsock-listen: it opens one CreateFile HANDLE
and reads/writes it. vsock needs bind/listen/accept and re-arming after
each client. Those exist only for sockets, not for a HANDLE, and
overlapped I/O doesn't add them.

I'd honestly rather wrap virtio-serial overlapped I/O with GIOFuncs to
use it as a GIOChannel (so that both vsock and virtio-serial are
GIOChannels) rather than downgrade the sockets to a manual GSource.

> 
> 
> > 2. g_io_channel_win32_new_fd() is the wrong constructor, as it's for CRT
> > fds, not sockets. GLib wants g_io_channel_win32_new_socket() (it sets up
> > WSAEventSelect). And qemu_accept()/socket_listen() return a CRT fd
> > wrapping a SOCKET via _open_osfhandle(), so we recover it with
> > _get_osfhandle(), which is also why teardown needs the explicit
> > qemu_close_socket_osfhandle() ordering, to avoid a double closesocket().
> > 
> > 3. Winsock disconnect detection differs: GLib passes FD_READ|FD_CLOSE as
> > one G_IO_IN and Winsock reports FD_CLOSE only once, so a peer that
> > writes-then-closes never yields G_IO_HUP and the listener hangs
> > half-open. That's why the MSG_PEEK probe logic is required on Windows,
> > while on POSIX poll()/epoll() reports POLLHUP reliably and doesn't need
> > it.
> > 
> > Best regards,
> > Polina.
> > 
> > > 
> > > Best Regards,
> > > Kostiantyn Kostiuk.
> > > 
> > > 
> > > On Fri, Jun 19, 2026 at 9:03 PM Polina Vishneva <
> > > [email protected]> wrote:
> > > 
> > > > Add vsock-listen channel mode to the Windows guest agent.
> > > > 
> > > > The listen/accept path uses the standard QEMU socket helpers
> > > > (socket_parse(), socket_listen(), qemu_accept()) and plugs the
> > resulting
> > > > SOCKET into the GMainLoop via g_io_channel_win32_new_socket().
> > > > 
> > > > Signed-off-by: Polina Vishneva <[email protected]>
> > > > ---
> > > >  docs/interop/qemu-ga.rst |   4 +
> > > >  qga/channel-win32.c      | 257 ++++++++++++++++++++++++++++++++-------
> > > >  qga/main.c               |   6 +
> > > >  3 files changed, 222 insertions(+), 45 deletions(-)
> > > > 
> > 

Reply via email to