All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: Claudio Fontana <cfontana@suse.de>, qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [PATCH] build-sys: fix win32 compilation with --target-list=''
Date: Fri, 18 Dec 2020 14:21:40 +0100	[thread overview]
Message-ID: <4287c327-0e3e-5f35-b078-0360796e6103@redhat.com> (raw)
In-Reply-To: <CAMxuvawOQc7sHoVV+eaAChwDb5RVRBHWPqzZ85xoovp+_nqtdg@mail.gmail.com>

On 18/12/20 14:01, Marc-André Lureau wrote:
>> in aio_set_fd_handler.  I think we can remove the call to
>> qemu_fd_register from qemu_try_set_nonblock, and that should fix the
>> issue as well.
>
> That's tricky to say whether this won't introduce regression. For most 
> fds from qemu, if they use aio_set_fd_handler, that should be ok.
> 
> But what about other fds? For examples, the ones from slirp?

slirp already calls qemu_fd_register, see net_slirp_register_poll_fd

> In fact, I 
> don't understand how it could work today. We are passing socket() fd 
> directly to g_poll(). But according to the documentation:
> 
>   * On Win32, the fd in a GPollFD should be Win32 HANDLE (*not* a file
>   * descriptor as provided by the C runtime) that can be used by
>   * MsgWaitForMultipleObjects. This does *not* include file handles
>   * from CreateFile, SOCKETs, nor pipe handles. (But you can use
>   * WSAEventSelect to signal events when a SOCKET is readable).
> 
> And MsgWaitForMultipleObjects doesn't mention SOCKET as being valid 
> handles to wait for.

No, it's more complicated.  On Win32, gpollfds is only used for sockets 
(despite the name!), while poll_fds is used for prepare/query/g_poll/check.

What we do is basically the same that QIOChannel and aio-win32.c already 
do, just with more indirection to fit the SLIRP callback API:

- main_loop_wait calls net_slirp_poll_notify, which asks SLIRP to send 
back the list of file descriptors through the net_slirp_add_poll callback.

- the file descriptors are stored in the gpollfds global.

- os_host_main_loop_wait does a select on the sockets with 0 timeout

- if no socket is ready, g_poll is done with the original timeout 
(otherwise the timeout is zeroed)

- the sockets were registered with WSAEventSelect in 
net_slirp_register_poll_fd, so they interrupt the subsequent g_poll if 
data comes in.

I don't see any other use of MainLoopPoll, so all non-SLIRP sockets 
should be going through {qemu,aio}_set_fd_handler.  In particular this 
is the case for all of chardev/ (which uses QIOChannel), io/ and net/. 
These are all the other users of qemu_set_nonblock and 
qemu_try_set_nonblock.

Paolo

> But when I run qemu with slirp, with or without qemu_fd_register, I 
> don't see any error or regression.
> 
> Am I missing something?



  reply	other threads:[~2020-12-18 13:25 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-17 10:44 [PATCH] build-sys: fix win32 compilation with --target-list='' marcandre.lureau
2020-12-17 11:32 ` Claudio Fontana
2020-12-17 11:41   ` Marc-André Lureau
2020-12-17 11:46   ` Paolo Bonzini
2020-12-18 13:01     ` Marc-André Lureau
2020-12-18 13:21       ` Paolo Bonzini [this message]
2020-12-18 13:33         ` Marc-André Lureau
2020-12-18 13:23       ` Marc-André Lureau

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=4287c327-0e3e-5f35-b078-0360796e6103@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=cfontana@suse.de \
    --cc=marcandre.lureau@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.