On 19/10/2022 09.09, Bin Meng wrote:
On Wed, Oct 19, 2022 at 12:47 AM Alex Bennée <alex.ben...@linaro.org> wrote:


Bin Meng <bmeng...@gmail.com> writes:

From: Xuzhou Cheng <xuzhou.ch...@windriver.com>

Socket communication in the libqtest and libqmp codes uses read()
and write() which work on any file descriptor on *nix, and sockets
in *nix are an example of a file descriptor.

However sockets on Windows do not use *nix-style file descriptors,
so read() and write() cannot be used on sockets on Windows.
Switch over to use send() and recv() instead which work on both
Windows and *nix.

Signed-off-by: Xuzhou Cheng <xuzhou.ch...@windriver.com>
Signed-off-by: Bin Meng <bin.m...@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
---

(no changes since v2)

Changes in v2:
- Introduce qemu_send_full() and use it

  include/qemu/sockets.h |  2 ++
  tests/qtest/libqmp.c   |  5 +++--
  tests/qtest/libqtest.c |  4 ++--
  util/osdep.c           | 33 +++++++++++++++++++++++++++++++++
  4 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 036745e586..adf2b21bd9 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -33,6 +33,8 @@ int qemu_socketpair(int domain, int type, int protocol, int 
sv[2]);
  #endif

  int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen);
+ssize_t qemu_send_full(int s, const void *buf, size_t count)
+    G_GNUC_WARN_UNUSED_RESULT;
  int socket_set_cork(int fd, int v);
  int socket_set_nodelay(int fd);
  void qemu_socket_set_block(int fd);
diff --git a/tests/qtest/libqmp.c b/tests/qtest/libqmp.c
index ade26c15f0..2b08382e5d 100644
--- a/tests/qtest/libqmp.c
+++ b/tests/qtest/libqmp.c
@@ -23,6 +23,7 @@
  #endif

  #include "qemu/cutils.h"
+#include "qemu/sockets.h"
  #include "qapi/error.h"
  #include "qapi/qmp/json-parser.h"
  #include "qapi/qmp/qjson.h"
@@ -36,7 +37,7 @@ typedef struct {

  static void socket_send(int fd, const char *buf, size_t size)
  {
-    size_t res = qemu_write_full(fd, buf, size);
+    ssize_t res = qemu_send_full(fd, buf, size);

      assert(res == size);
  }
@@ -69,7 +70,7 @@ QDict *qmp_fd_receive(int fd)
          ssize_t len;
          char c;

-        len = read(fd, &c, 1);
+        len = recv(fd, &c, 1, 0);
          if (len == -1 && errno == EINTR) {
              continue;
          }
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 4f4b2d6477..8228262938 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -436,7 +436,7 @@ void qtest_quit(QTestState *s)

  static void socket_send(int fd, const char *buf, size_t size)
  {
-    size_t res = qemu_write_full(fd, buf, size);
+    ssize_t res = qemu_send_full(fd, buf, size);

      assert(res == size);
  }
@@ -468,7 +468,7 @@ static GString *qtest_client_socket_recv_line(QTestState *s)
          ssize_t len;
          char buffer[1024];

-        len = read(s->fd, buffer, sizeof(buffer));
+        len = recv(s->fd, buffer, sizeof(buffer), 0);
          if (len == -1 && errno == EINTR) {
              continue;
          }
diff --git a/util/osdep.c b/util/osdep.c
index 60fcbbaebe..0342e754e1 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -502,6 +502,39 @@ int qemu_accept(int s, struct sockaddr *addr, socklen_t 
*addrlen)
      return ret;
  }

+/*
+ * A variant of send(2) which handles partial send.
+ *
+ * Return the number of bytes transferred over the socket.
+ * Set errno if fewer than `count' bytes are sent.
+ *
+ * This function don't work with non-blocking socket's.
+ * Any of the possibilities with non-blocking socket's is bad:
+ *   - return a short write (then name is wrong)
+ *   - busy wait adding (errno == EAGAIN) to the loop
+ */
+ssize_t qemu_send_full(int s, const void *buf, size_t count)
+{
+    ssize_t ret = 0;
+    ssize_t total = 0;
+
+    while (count) {
+        ret = send(s, buf, count, 0);
+        if (ret < 0) {
+            if (errno == EINTR) {
+                continue;
+            }
+            break;
+        }
+
+        count -= ret;
+        buf += ret;
+        total += ret;
+    }
+
+    return total;
+}
+
  void qemu_set_hw_version(const char *version)
  {
      hw_version = version;

Hmm something goes wrong here:

FAILED: tests/qtest/libqos/libqos.fa.p/.._libqtest.c.o
cc -m64 -mcx16 -Itests/qtest/libqos/libqos.fa.p -Itests/qtest/libqos 
-I../../tests/qtest/libqos -I. -Iqapi -Itrace -Iui -Iui/shader 
-I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include 
-fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem 
/home/alex/lsrc/qemu.git/linux-headers -isystem linux-headers -iquote . -iquote 
/home/alex/lsrc/qemu.git -iquote /home/alex/lsrc/qemu.git/include -iquote 
/home/alex/lsrc/qemu.git/tcg/i386 -pthread -U_FORTIFY_SOURCE 
-D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE 
-Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite-strings 
-Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv 
-Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security 
-Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs 
-Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 
-Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi 
-fstack-protector-strong -fPIE -MD -MQ 
tests/qtest/libqos/libqos.fa.p/.._libqtest.c.o -MF 
tests/qtest/libqos/libqos.fa.p/.._libqtest.c.o.d -o 
tests/qtest/libqos/libqos.fa.p/.._libqtest.c.o -c ../../tests/qtest/libqtest.c
../../tests/qtest/libqtest.c: In function ‘socket_send’:
../../tests/qtest/libqtest.c:431:19: error: implicit declaration of function 
‘qemu_send_full’; did you mean ‘qemu_write_full’? 
[-Werror=implicit-function-declaration]
   431 |     ssize_t res = qemu_send_full(fd, buf, size);
       |                   ^~~~~~~~~~~~~~
       |                   qemu_write_full
../../tests/qtest/libqtest.c:431:19: error: nested extern declaration of 
‘qemu_send_full’ [-Werror=nested-externs]
cc1: all warnings being treated as errors


dropping this patch.


This is weird. It does not reproduce on my side, neither did the QEMU CI.

I think it needs an #include statement from an earlier patch?

 Thomas


Reply via email to