[Qemu-devel] [PATCH for-1.5 0/2] main-loop: fix slirp on win32

[Qemu-devel] [PATCH for-1.5 0/2] main-loop: fix slirp on win32

[Stefan Hajnoczi]

User networking is broken on win32.  These patches resolve the issues.

TeLeMan and therock247uk: Please apply these patches and confirm that they fix
the bug.

Paolo: Please let me know if you want to take Author:.

Stefan Hajnoczi (2):
  main-loop: narrow win32 pollfds_fill() event bitmasks
  main-loop: partial revert of 5e3bc73

 main-loop.c | 40 ++++++++++++++++++----------------------
 1 file changed, 18 insertions(+), 22 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH for-1.5 1/2] main-loop: narrow win32 pollfds_fill() event bitmasks

[Stefan Hajnoczi]

pollfds_fill() and pollfds_poll() translate GPollFD to rfds/wfds/xfds
for sockets on win32.  select(2) is the underlying system call which is
used to monitor sockets for activity.

Currently file descriptors that monitor G_IO_ERR will be included in
both rfds and wfds.  As a result, select(2) will report writability on
file descriptors where we only really wanted to monitor readability
(with errors).

slirp_pollfds_poll() hit this issue: UDP sockets are blocking sockets so
we hang in sorecvfrom() when G_IO_ERR is set due to the socket being
writable (we only wanted to check for readability).

This patch fixes the slirp_pollfds_poll() hang.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 main-loop.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/main-loop.c b/main-loop.c
index f46aece..2b8eed7 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -333,11 +333,11 @@ static int pollfds_fill(GArray *pollfds, fd_set *rfds, fd_set *wfds,
         GPollFD *pfd = &g_array_index(pollfds, GPollFD, i);
         int fd = pfd->fd;
         int events = pfd->events;
-        if (events & (G_IO_IN | G_IO_HUP | G_IO_ERR)) {
+        if (events & G_IO_IN) {
             FD_SET(fd, rfds);
             nfds = MAX(nfds, fd);
         }
-        if (events & (G_IO_OUT | G_IO_ERR)) {
+        if (events & G_IO_OUT) {
             FD_SET(fd, wfds);
             nfds = MAX(nfds, fd);
         }
@@ -360,10 +360,10 @@ static void pollfds_poll(GArray *pollfds, int nfds, fd_set *rfds,
         int revents = 0;
 
         if (FD_ISSET(fd, rfds)) {
-            revents |= G_IO_IN | G_IO_HUP | G_IO_ERR;
+            revents |= G_IO_IN;
         }
         if (FD_ISSET(fd, wfds)) {
-            revents |= G_IO_OUT | G_IO_ERR;
+            revents |= G_IO_OUT;
         }
         if (FD_ISSET(fd, xfds)) {
             revents |= G_IO_PRI;
-- 
1.8.1.4

[Qemu-devel] [PATCH for-1.5 2/2] main-loop: partial revert of 5e3bc73

[Stefan Hajnoczi]

This patch reverts part of 5e3bc735d93dd23f074b5116fd11e1ad8cd4962f.

Paolo Bonzini wrote this patch and commented:

"WSAEventSelect is edge-triggered and the event will not be signaled if
the socket handler does not consume all the data in the socket buffer."

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 main-loop.c | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/main-loop.c b/main-loop.c
index 2b8eed7..cf36645 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -394,6 +394,20 @@ static int os_host_main_loop_wait(uint32_t timeout)
         return ret;
     }
 
+    FD_ZERO(&rfds);
+    FD_ZERO(&wfds);
+    FD_ZERO(&xfds);
+    nfds = pollfds_fill(gpollfds, &rfds, &wfds, &xfds);
+    if (nfds >= 0) {
+        select_ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv0);
+        if (select_ret != 0) {
+            timeout = 0;
+        }
+        if (select_ret > 0) {
+            pollfds_poll(gpollfds, nfds, &rfds, &wfds, &xfds);
+        }
+    }
+
     g_main_context_prepare(context, &max_priority);
     n_poll_fds = g_main_context_query(context, max_priority, &poll_timeout,
                                       poll_fds, ARRAY_SIZE(poll_fds));
@@ -426,24 +440,6 @@ static int os_host_main_loop_wait(uint32_t timeout)
         g_main_context_dispatch(context);
     }
 
-    /* Call select after g_poll to avoid a useless iteration and therefore
-     * improve socket latency.
-     */
-
-    FD_ZERO(&rfds);
-    FD_ZERO(&wfds);
-    FD_ZERO(&xfds);
-    nfds = pollfds_fill(gpollfds, &rfds, &wfds, &xfds);
-    if (nfds >= 0) {
-        select_ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv0);
-        if (select_ret != 0) {
-            timeout = 0;
-        }
-        if (select_ret > 0) {
-            pollfds_poll(gpollfds, nfds, &rfds, &wfds, &xfds);
-        }
-    }
-
     return select_ret || g_poll_ret;
 }
 #endif
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH for-1.5 2/2] main-loop: partial revert of 5e3bc73

[Paolo Bonzini]

Il 16/05/2013 17:36, Stefan Hajnoczi ha scritto:
> This patch reverts part of 5e3bc735d93dd23f074b5116fd11e1ad8cd4962f.
> 
> Paolo Bonzini wrote this patch and commented:
> 
> "WSAEventSelect is edge-triggered and the event will not be signaled if
> the socket handler does not consume all the data in the socket buffer."
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>


> ---
>  main-loop.c | 32 ++++++++++++++------------------
>  1 file changed, 14 insertions(+), 18 deletions(-)
> 
> diff --git a/main-loop.c b/main-loop.c
> index 2b8eed7..cf36645 100644
> --- a/main-loop.c
> +++ b/main-loop.c
> @@ -394,6 +394,20 @@ static int os_host_main_loop_wait(uint32_t timeout)
>          return ret;
>      }
>  
> +    FD_ZERO(&rfds);
> +    FD_ZERO(&wfds);
> +    FD_ZERO(&xfds);
> +    nfds = pollfds_fill(gpollfds, &rfds, &wfds, &xfds);
> +    if (nfds >= 0) {
> +        select_ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv0);
> +        if (select_ret != 0) {
> +            timeout = 0;
> +        }
> +        if (select_ret > 0) {
> +            pollfds_poll(gpollfds, nfds, &rfds, &wfds, &xfds);
> +        }
> +    }
> +
>      g_main_context_prepare(context, &max_priority);
>      n_poll_fds = g_main_context_query(context, max_priority, &poll_timeout,
>                                        poll_fds, ARRAY_SIZE(poll_fds));
> @@ -426,24 +440,6 @@ static int os_host_main_loop_wait(uint32_t timeout)
>          g_main_context_dispatch(context);
>      }
>  
> -    /* Call select after g_poll to avoid a useless iteration and therefore
> -     * improve socket latency.
> -     */
> -
> -    FD_ZERO(&rfds);
> -    FD_ZERO(&wfds);
> -    FD_ZERO(&xfds);
> -    nfds = pollfds_fill(gpollfds, &rfds, &wfds, &xfds);
> -    if (nfds >= 0) {
> -        select_ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv0);
> -        if (select_ret != 0) {
> -            timeout = 0;
> -        }
> -        if (select_ret > 0) {
> -            pollfds_poll(gpollfds, nfds, &rfds, &wfds, &xfds);
> -        }
> -    }
> -
>      return select_ret || g_poll_ret;
>  }
>  #endif
> 

Re: [Qemu-devel] [PATCH for-1.5 0/2] main-loop: fix slirp on win32

[Paolo Bonzini]

Il 16/05/2013 17:35, Stefan Hajnoczi ha scritto:
> User networking is broken on win32.  These patches resolve the issues.
> 
> TeLeMan and therock247uk: Please apply these patches and confirm that they fix
> the bug.
> 
> Paolo: Please let me know if you want to take Author:.

No worries, thanks.

Paolo

Re: [Qemu-devel] [PATCH for-1.5 0/2] main-loop: fix slirp on win32

[TeLeMan]

On Thu, May 16, 2013 at 11:35 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> User networking is broken on win32.  These patches resolve the issues.
>
> TeLeMan and therock247uk: Please apply these patches and confirm that they fix
> the bug.
>
> Paolo: Please let me know if you want to take Author:.
>
> Stefan Hajnoczi (2):
>   main-loop: narrow win32 pollfds_fill() event bitmasks
>   main-loop: partial revert of 5e3bc73
>
>  main-loop.c | 40 ++++++++++++++++++----------------------
>  1 file changed, 18 insertions(+), 22 deletions(-)
>
> --
> 1.8.1.4
>
Yeah, It works.

Tested-by: TeLeMan <geleman@gmail.com>

Re: [Qemu-devel] [PATCH for-1.5 0/2] main-loop: fix slirp on win32

[Paolo Bonzini]

Il 17/05/2013 03:25, TeLeMan ha scritto:
> On Thu, May 16, 2013 at 11:35 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>> User networking is broken on win32.  These patches resolve the issues.
>>
>> TeLeMan and therock247uk: Please apply these patches and confirm that they fix
>> the bug.
>>
>> Paolo: Please let me know if you want to take Author:.
>>
>> Stefan Hajnoczi (2):
>>   main-loop: narrow win32 pollfds_fill() event bitmasks
>>   main-loop: partial revert of 5e3bc73
>>
>>  main-loop.c | 40 ++++++++++++++++++----------------------
>>  1 file changed, 18 insertions(+), 22 deletions(-)
>>
>> --
>> 1.8.1.4
>
> Yeah, It works.
> 
> Tested-by: TeLeMan <geleman@gmail.com>

Thanks for your help!

Paolo

Re: [Qemu-devel] [PATCH for-1.5 0/2] main-loop: fix slirp on win32

[Anthony Liguori]

Applied.  Thanks.

Regards,

Anthony Liguori