Re: [Qemu-devel] [PATCH v1 00/21] Multiple fixes & improves to QIOChannel & Win32

[Paolo Bonzini <pbonzini@redhat.com>]

On 09/03/2016 18:28, Daniel P. Berrange wrote:
> This series started out as an attempt to fix the Win32 problems
> identified by Andrew Baumann
> 
>    https://lists.gnu.org/archive/html/qemu-devel/2016-03/msg01343.html
> 
> It turned into a significantly larger cleanup of some chardev
> and osdep win32 portability code.
> 
> 
> Patch 1 addresses Andrew's 2nd stated problem - handling of
> getpeername() failures, by fixing errno handling on Win32.
> 
> Patches 2-7 do some fixes in the test-io-channel-socket test
> case so that it is able to run on Win32.
> 
> Patch 8 is a simple error message fix.
> 
> Patch 9 adds missing EWOULDBLOCK handling to QIOChannelSocket
> 
> Patches 10-12 change QIOChannelSocket so that it uses a Win32
> specific GSource implementation for creating watches. This is
> the key fix for Andrew's 1st stated problem.
> 
> Patch 13 fixes a further problem identified with chardev on
> Win32 - the 'wait' option behaves the same as 'nowait'.
> 
> At this point tests/test-io-channel-socket passes and
> 
>   qemu-system-x86_64.exe  -serial tcp:127.0.0.1:9000,server,nowait -device sga -display non
> 
> works on win32 once more.
> 
> 
> Patches 14-16 are some cleanups to the chardev code to improve
> its clarity. They are not required for fixing any real problem
> 
> Patches 17-20 change the way we provide Win32 portability for
> sockets APIs inside QEMU. These do fix a number of bugs in the
> QEMU code related to mistaken use of errno instead of
> socket_error(), or close() instead fo closesocket() and
> accept() instead of qemu_accept(). None of these bugs appear
> to be critical issues.
> 
> Patch 21 improves error reporting with error_abort
> 
> Based on this, I'm proposing 1-13 for QEMU 2.6 release as they
> fix critical win32 bugs.
> 
> Patches 14-21 can either be included in 2.6 or 2.7 - I'm
> ambivalent on which, since they're cleanups / minor fixes.
> 
> Daniel P. Berrange (18):
>   osdep: fix socket_error() to work with Mingw64
>   io: use bind() to check for IPv4/6 availability
>   io: initialize sockets in test program
>   io: bind to socket before creating QIOChannelSocket
>   io: wait for incoming client in socket test
>   io: set correct error object in background reader test thread
>   io: assert errors before asserting content in I/O test
>   io: fix copy+paste mistake in socket error message
>   io: add missing EWOULDBLOCK checks in Win32 I/O code paths
>   char: ensure listener socket is in blocking mode when waiting
>   char: remove qemu_chr_finish_socket_connection method
>   char: remove socket_try_connect method
>   char: remove qemu_chr_open_socket_fd method
>   osdep: add wrappers for socket functions
>   osdep: remove use of Win32 specific closesocket/ioctlsocket
>   osdep: remove use of socket_error() from all code
>   osdep: remove direct use of qemu_socket & qemu_accept
>   error: ensure errno detail is printed with error_abort
> 
> Paolo Bonzini (3):
>   io: pass HANDLE to g_source_add_poll on Win32
>   io: introduce qio_channel_create_socket_watch
>   io: implement socket watch for win32 using WSAEventSelect+select
> 
>  Makefile                                |   4 +-
>  block/sheepdog.c                        |  41 +++--
>  contrib/ivshmem-server/ivshmem-server.c |   4 +-
>  include/io/channel-watch.h              |  20 ++-
>  include/io/channel.h                    |   3 +
>  include/qemu/sockets.h                  |  19 --
>  include/sysemu/os-posix.h               |  17 ++
>  include/sysemu/os-win32.h               | 106 ++++++++---
>  io/channel-socket.c                     |  63 ++++---
>  io/channel-watch.c                      | 162 ++++++++++++++++-
>  io/channel.c                            |  14 ++
>  migration/qemu-file-unix.c              |  16 +-
>  migration/tcp.c                         |  13 +-
>  migration/unix.c                        |   2 +-
>  net/socket.c                            |  49 +++--
>  qemu-char.c                             |  97 ++++------
>  qga/channel-posix.c                     |   4 +-
>  slirp/ip_icmp.c                         |   4 +-
>  slirp/misc.c                            |   8 +-
>  slirp/slirp.h                           |   2 -
>  slirp/socket.c                          |   4 +-
>  slirp/tcp_input.c                       |   4 -
>  slirp/tcp_subr.c                        |   8 +-
>  slirp/udp.c                             |   6 +-
>  tests/io-channel-helpers.c              |   6 +-
>  tests/test-io-channel-socket.c          |  92 +++++-----
>  util/error.c                            |  40 ++---
>  util/osdep.c                            |  42 -----
>  util/oslib-posix.c                      |  39 ++++
>  util/oslib-win32.c                      | 304 +++++++++++++++++++++++++++++++-
>  util/qemu-coroutine-io.c                |   6 +-
>  util/qemu-sockets.c                     |  32 ++--
>  32 files changed, 867 insertions(+), 364 deletions(-)
> 

For everything I didn't comment on:

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