On 09.09.25 01:16, Peter Xu wrote:
On Wed, Sep 03, 2025 at 12:44:04PM +0300, Vladimir Sementsov-Ogievskiy wrote:
Use common qemu_set_blocking() instead.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>

Posix's qemu_socket_set_nonblock() asserts the retval.. While Windows's one
doesn't.  IIUC that's the only reason you provided the generic error
path in all callers, just in case some of them might fail on Windows?

Honestly, I thought that checking error on Linux is good too.. It may fail,
why not to check, where possible?


I wished Windows's one has an assertion from the start too.  Maybe none of
the failure path would really trigger.. Not a big deal:

Reviewed-by: Peter Xu <pet...@redhat.com>

One nitpick below:

---
  contrib/ivshmem-server/ivshmem-server.c |  5 ++++-
  hw/hyperv/syndbg.c                      |  4 +++-
  hw/virtio/vhost-user.c                  |  5 ++++-
  include/qemu/sockets.h                  |  1 -
  io/channel-socket.c                     |  7 +++----
  net/dgram.c                             | 16 +++++++++++++---
  net/l2tpv3.c                            |  5 +++--
  net/socket.c                            | 20 ++++++++++++++++----
  qga/channel-posix.c                     |  7 ++++++-
  tests/unit/socket-helpers.c             |  5 ++++-
  tests/unit/test-crypto-tlssession.c     |  8 ++++----
  util/oslib-posix.c                      |  7 -------
  util/oslib-win32.c                      |  5 -----
  util/vhost-user-server.c                |  4 ++--
  14 files changed, 62 insertions(+), 37 deletions(-)

diff --git a/contrib/ivshmem-server/ivshmem-server.c 
b/contrib/ivshmem-server/ivshmem-server.c
index 2f3c7320a6..9ccd436ee4 100644
--- a/contrib/ivshmem-server/ivshmem-server.c
+++ b/contrib/ivshmem-server/ivshmem-server.c
@@ -146,7 +146,10 @@ ivshmem_server_handle_new_conn(IvshmemServer *server)
          return -1;
      }
- qemu_socket_set_nonblock(newfd);
+    if (!qemu_set_blocking(newfd, false, NULL)) {

Better if with a IVSHMEM_SERVER_DEBUG(), which follows the original lines.

Will add


+        close(newfd);
+        return -1;
+    }
      IVSHMEM_SERVER_DEBUG(server, "accept()=%d\n", newfd);
/* allocate new structure for this peer */


Thanks for reviewing my patches!


--
Best regards,
Vladimir

Reply via email to