[Qemu-devel] [PATCH 0/4] tests/pxe-test: add testcase using vhost-user-bridge

[Qemu-devel] [PATCH 0/4] tests/pxe-test: add testcase using vhost-user-bridge

[Jens Freimann]

This implements a testcase for pxe-test using the vhost-user interface. Spawn a
vhost-user-bridge process and connect it to the qemu process.

To make the testcase work we need to apply a few patches before the actual testcase:
- Patch 1 disables debug output of vhost-user-bridge
- Patch 2 fixes passing a file descriptor to -netdev.  This was broken since the
  mcast option was introduced. 
- Patch 3 makes sure we stop processing vhost-user messages when recvmsg returns 0.

regards,
Jens

Jens Freimann (4):
  tests/vhost-user-bridge: disable debug output by default
  net: fix -netdev socket,fd= for UDP sockets
  libvhost-user: quit when no more data received
  tests/pxe-test: add testcase using vhost-user-bridge

 contrib/libvhost-user/libvhost-user.c |   4 +-
 net/socket.c                          |  37 +++++----
 tests/Makefile.include                |   4 +-
 tests/pxe-test.c                      | 140 +++++++++++++++++++++++++++++++++-
 tests/vhost-user-bridge.c             |   2 +-
 5 files changed, 164 insertions(+), 23 deletions(-)

-- 
2.9.4

[Qemu-devel] [PATCH 1/4] tests/vhost-user-bridge: disable debug output by default

[Jens Freimann]

From: Jens Freimann <jfreiman@redhat.com>

vhost-user-bridge prints out a lot of information, including dumps
of all transmitted data. When called from a testcase this output
clutters the actual test results, so let's make the default no debug
output.

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: Jens Freimann <jfreiman@redhat.com>
---
 tests/vhost-user-bridge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c
index 1e5b5ca..93d9535 100644
--- a/tests/vhost-user-bridge.c
+++ b/tests/vhost-user-bridge.c
@@ -34,7 +34,7 @@
 #include "standard-headers/linux/virtio_net.h"
 #include "contrib/libvhost-user/libvhost-user.h"
 
-#define VHOST_USER_BRIDGE_DEBUG 1
+#define VHOST_USER_BRIDGE_DEBUG 0
 
 #define DPRINT(...) \
     do { \
-- 
2.9.4

[Qemu-devel] [PATCH 2/4] net: fix -netdev socket, fd= for UDP sockets

[Jens Freimann]

This patch fixes -netdev socket,fd= for UDP sockets
Currently -netdev socket,fd=<...> results in

  qemu: error: specified mcastaddr "127.0.0.1" (0x7f000001) does not
    contain a multicast address
  qemu-system-x86_64: -netdev
    socket,id=n1,fd=3: Device 'socket' could not be initialized

To fix these we need to allow specifying multicast and fd arguments
for the same netdev. With this the user can specify "-netdev
fd=3,mcast=<IP:port>"

Fixes: 3d830459b1eccdb61b75e2712fd364012ce5a115
Signed-off-by: Jens Freimann <jfreimann@redhat.com>
---
 net/socket.c | 37 ++++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index f85ef7d..18af2ab 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -320,11 +320,11 @@ static NetClientInfo net_dgram_socket_info = {
 static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer,
                                                 const char *model,
                                                 const char *name,
-                                                int fd, int is_connected)
+                                                int fd, int is_connected,
+                                                const char *mcast)
 {
     struct sockaddr_in saddr;
     int newfd;
-    socklen_t saddr_len = sizeof(saddr);
     NetClientState *nc;
     NetSocketState *s;
 
@@ -333,8 +333,13 @@ static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer,
      * by ONLY ONE process: we must "clone" this dgram socket --jjo
      */
 
-    if (is_connected) {
-        if (getsockname(fd, (struct sockaddr *) &saddr, &saddr_len) == 0) {
+    if (is_connected && mcast != NULL) {
+            if (parse_host_port(&saddr, mcast) < 0) {
+                fprintf(stderr,
+                        "qemu: error: init_dgram: fd=%d failed parse_host_port()\n",
+                        fd);
+                goto err;
+            }
             /* must be bound */
             if (saddr.sin_addr.s_addr == 0) {
                 fprintf(stderr, "qemu: error: init_dgram: fd=%d unbound, "
@@ -351,12 +356,6 @@ static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer,
             dup2(newfd, fd);
             close(newfd);
 
-        } else {
-            fprintf(stderr,
-                    "qemu: error: init_dgram: fd=%d failed getsockname(): %s\n",
-                    fd, strerror(errno));
-            goto err;
-        }
     }
 
     nc = qemu_new_net_client(&net_dgram_socket_info, peer, model, name);
@@ -432,7 +431,7 @@ static NetSocketState *net_socket_fd_init_stream(NetClientState *peer,
 
 static NetSocketState *net_socket_fd_init(NetClientState *peer,
                                           const char *model, const char *name,
-                                          int fd, int is_connected)
+                                          int fd, int is_connected, const char *mc)
 {
     int so_type = -1, optlen=sizeof(so_type);
 
@@ -445,7 +444,7 @@ static NetSocketState *net_socket_fd_init(NetClientState *peer,
     }
     switch(so_type) {
     case SOCK_DGRAM:
-        return net_socket_fd_init_dgram(peer, model, name, fd, is_connected);
+        return net_socket_fd_init_dgram(peer, model, name, fd, is_connected, mc);
     case SOCK_STREAM:
         return net_socket_fd_init_stream(peer, model, name, fd, is_connected);
     default:
@@ -567,7 +566,7 @@ static int net_socket_connect_init(NetClientState *peer,
             break;
         }
     }
-    s = net_socket_fd_init(peer, model, name, fd, connected);
+    s = net_socket_fd_init(peer, model, name, fd, connected, NULL);
     if (!s)
         return -1;
     snprintf(s->nc.info_str, sizeof(s->nc.info_str),
@@ -602,7 +601,7 @@ static int net_socket_mcast_init(NetClientState *peer,
     if (fd < 0)
         return -1;
 
-    s = net_socket_fd_init(peer, model, name, fd, 0);
+    s = net_socket_fd_init(peer, model, name, fd, 0, NULL);
     if (!s)
         return -1;
 
@@ -652,7 +651,7 @@ static int net_socket_udp_init(NetClientState *peer,
     }
     qemu_set_nonblock(fd);
 
-    s = net_socket_fd_init(peer, model, name, fd, 0);
+    s = net_socket_fd_init(peer, model, name, fd, 0, NULL);
     if (!s) {
         return -1;
     }
@@ -675,9 +674,9 @@ int net_init_socket(const Netdev *netdev, const char *name,
     assert(netdev->type == NET_CLIENT_DRIVER_SOCKET);
     sock = &netdev->u.socket;
 
-    if (sock->has_fd + sock->has_listen + sock->has_connect + sock->has_mcast +
-        sock->has_udp != 1) {
-        error_report("exactly one of fd=, listen=, connect=, mcast= or udp="
+    if (sock->has_listen + sock->has_connect + sock->has_mcast +
+        sock->has_udp > 1) {
+        error_report("exactly one of listen=, connect=, mcast= or udp="
                      " is required");
         return -1;
     }
@@ -696,7 +695,7 @@ int net_init_socket(const Netdev *netdev, const char *name,
             return -1;
         }
         qemu_set_nonblock(fd);
-        if (!net_socket_fd_init(peer, "socket", name, fd, 1)) {
+        if (!net_socket_fd_init(peer, "socket", name, fd, 1, sock->mcast)) {
             return -1;
         }
         return 0;
-- 
2.9.4

[Qemu-devel] [PATCH 3/4] libvhost-user: quit when no more data received

[Jens Freimann]

From: Jens Freimann <jfreiman@redhat.com>

End processing of messages when VHOST_USER_NO_MESSAGE
is received.

Without this we run into a vubr_panic() call and get
"PANIC: Unhandled request: 0"

Signed-off-by: Jens Freimann <jfreiman@redhat.com>
---
 contrib/libvhost-user/libvhost-user.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c
index 9efb9da..85523ca 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -161,7 +161,7 @@ vu_message_read(VuDev *dev, int conn_fd, VhostUserMsg *vmsg)
         rc = recvmsg(conn_fd, &msg, 0);
     } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
 
-    if (rc <= 0) {
+    if (rc < 0) {
         vu_panic(dev, "Error while recvmsg: %s", strerror(errno));
         return false;
     }
@@ -806,6 +806,8 @@ vu_process_message(VuDev *dev, VhostUserMsg *vmsg)
         return vu_get_queue_num_exec(dev, vmsg);
     case VHOST_USER_SET_VRING_ENABLE:
         return vu_set_vring_enable_exec(dev, vmsg);
+    case VHOST_USER_NONE:
+        return true;
     default:
         vmsg_close_fds(vmsg);
         vu_panic(dev, "Unhandled request: %d", vmsg->request);
-- 
2.9.4

[Qemu-devel] [PATCH 4/4] tests/pxe-test: add testcase using vhost-user-bridge

[Jens Freimann]

From: Jens Freimann <jfreiman@redhat.com>

Add a PXE testcase tunneling traffic through vhost-user-bridge process.
Create a vhost-user-bridge  process and connect it to qemu via a socket.

Signed-off-by: Jens Freimann <jfreimann@redhat.com>
---
 tests/Makefile.include |   4 +-
 tests/pxe-test.c       | 140 ++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 142 insertions(+), 2 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 7af278d..e40550c 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -705,7 +705,8 @@ tests/boot-order-test$(EXESUF): tests/boot-order-test.o $(libqos-obj-y)
 tests/boot-serial-test$(EXESUF): tests/boot-serial-test.o $(libqos-obj-y)
 tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o \
 	tests/boot-sector.o tests/acpi-utils.o $(libqos-obj-y)
-tests/pxe-test$(EXESUF): tests/pxe-test.o tests/boot-sector.o $(libqos-obj-y)
+tests/pxe-test$(EXESUF): tests/pxe-test.o tests/boot-sector.o \
+    tests/vhost-user-bridge$(EXESUF) $(libqos-obj-y)
 tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
 tests/ds1338-test$(EXESUF): tests/ds1338-test.o $(libqos-imx-obj-y)
 tests/m25p80-test$(EXESUF): tests/m25p80-test.o
@@ -834,6 +835,7 @@ $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
 	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
 	$(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
 		QTEST_QEMU_IMG=qemu-img$(EXESUF) \
+		QTEST_VUBR_BINARY=./tests/vhost-user-bridge$(EXESUF) \
 		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} \
 		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y) $(check-qtest-generic-y),"GTESTER","$@")
 	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y) $(gcov-files-generic-y); do \
diff --git a/tests/pxe-test.c b/tests/pxe-test.c
index 34282d3..d6379a8 100644
--- a/tests/pxe-test.c
+++ b/tests/pxe-test.c
@@ -5,7 +5,8 @@
  *
  * Authors:
  *  Michael S. Tsirkin <mst@redhat.com>,
- *  Victor Kaplansky <victork@redhat.com>
+ *  Victor Kaplansky <victork@redhat.com>,
+ *  Jens Freimann <jfreiman@redhat.com>
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
@@ -13,14 +14,150 @@
 
 #include "qemu/osdep.h"
 #include <glib/gstdio.h>
+#include <glib.h>
 #include "qemu-common.h"
 #include "libqtest.h"
 #include "boot-sector.h"
+#include <sys/vfs.h>
 
+#define LPORT 5555
+#define RPORT 4444
 #define NETNAME "net0"
+#define QEMU_CMD_MEM    "--enable-kvm -m %d "\
+                        "-object memory-backend-file,id=mem,size=%dM,"\
+                        "mem-path=%s,share=on -numa node,memdev=mem -mem-prealloc "
+#define QEMU_CMD_CHR    " -chardev socket,id=%s,path=%s"
+#define QEMU_CMD_NETDEV " -device virtio-net-pci,netdev=net0 "\
+                        " -netdev vhost-user,id=net0,chardev=%s,vhostforce "\
+                        " -netdev user,id=n0,tftp=./,bootfile=%s "\
+                        " -netdev socket,id=n1,fd=%d"
+#define QEMU_CMD_NET    " -device virtio-net-pci,netdev=n0 "\
+                        " -device virtio-net-pci,netdev=n1 "
+
+#define QEMU_CMD        QEMU_CMD_MEM QEMU_CMD_CHR \
+                        QEMU_CMD_NETDEV QEMU_CMD_NET
+
+#define HUGETLBFS_MAGIC       0x958458f6
+#define VUBR_SOCK "vubr.sock"
+#define MEMSZ 1024
 
 static char disk[] = "tests/pxe-test-disk-XXXXXX";
 
+static const char *init_hugepagefs(const char *path)
+{
+    struct statfs fs;
+    int ret;
+
+    if (access(path, R_OK | W_OK | X_OK)) {
+        g_test_message("access on path (%s): %s\n", path, strerror(errno));
+        return NULL;
+    }
+
+    do {
+        ret = statfs(path, &fs);
+    } while (ret != 0 && errno == EINTR);
+
+    if (ret != 0) {
+        g_test_message("statfs on path (%s): %s\n", path, strerror(errno));
+        return NULL;
+    }
+
+    if (fs.f_type != HUGETLBFS_MAGIC) {
+        g_test_message("Warning: path not on HugeTLBFS: %s\n", path);
+        return NULL;
+    }
+
+    return path;
+}
+
+static int vubr_create_socket(struct sockaddr_in *si_remote, int rport)
+{
+    int sock;
+
+    if (inet_aton("127.0.0.1", (struct in_addr *) &si_remote->sin_addr.s_addr)
+            == 0) {
+        g_test_message("inet_aton failed\n");
+        return -1;
+    }
+    sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
+    if (sock == -1) {
+        g_test_message("socket creation failed\n");
+        return -1;
+    }
+    if (connect(sock, (struct sockaddr *) si_remote, sizeof(*si_remote))) {
+        g_test_message("connect failed: %s", strerror(errno));
+        return -1;
+    }
+
+    return sock;
+}
+
+static void test_pxe_vhost_user(void)
+{
+    char template[] = "/tmp/vhost-user-bridge-XXXXXX";
+    char template2[] = "/tmp/hugepages-XXXXXX";
+    gchar * vubr_args[] = {NULL, NULL, NULL, NULL};
+    struct sockaddr_in si_remote = {
+        .sin_family = AF_INET,
+        .sin_port = htons(RPORT),
+    };
+    const char *hugefs = NULL;
+    const char *tmpfs2 = NULL;
+    const char *tmpfs = NULL;
+    const char *root = NULL;
+    GError *error = NULL;
+    char *vubr_binary;
+    char *qemu_args;
+    GPid vubr_pid;
+    int sock = -1;
+
+    tmpfs = mkdtemp(template);
+    if (!tmpfs) {
+        g_test_message("mkdtemp on path(%s): %s\n",
+                       template, strerror(errno));
+    }
+    vubr_binary = getenv("QTEST_VUBR_BINARY");
+    g_assert(vubr_binary);
+    vubr_args[0] = g_strdup_printf("%s", vubr_binary);
+    vubr_args[1] = g_strdup_printf("-u");
+    vubr_args[2] = g_strdup_printf("%s/%s", tmpfs, VUBR_SOCK);
+    g_spawn_async(NULL, vubr_args, NULL,
+                  G_SPAWN_SEARCH_PATH_FROM_ENVP |
+                  G_SPAWN_SEARCH_PATH,
+                  NULL, NULL, &vubr_pid, &error);
+    g_assert_no_error(error);
+
+    hugefs = getenv("QTEST_HUGETLBFS_PATH");
+    if (hugefs) {
+        root = init_hugepagefs(hugefs);
+        g_assert(root);
+    } else {
+        tmpfs2 = mkdtemp(template2);
+        g_assert(tmpfs2);
+        root = tmpfs2;
+    }
+
+    sock = vubr_create_socket(&si_remote, RPORT);
+    g_assert_cmpint(sock, !=, -1);
+
+    qemu_args = g_strdup_printf(QEMU_CMD, MEMSZ, MEMSZ, (root),
+                                "char0", vubr_args[2], "char0",
+                                disk, sock);
+    qtest_start(qemu_args);
+    boot_sector_test();
+    qtest_quit(global_qtest);
+    g_free(qemu_args);
+    g_free(vubr_args[0]);
+    g_free(vubr_args[1]);
+    g_free(vubr_args[2]);
+    g_assert_cmpint(g_remove(g_strdup_printf("%s/%s", tmpfs, VUBR_SOCK)),
+                    ==, 0);
+    if (tmpfs2) {
+        g_assert_cmpint(rmdir(tmpfs2), ==, 0);
+    }
+    g_assert_cmpint(g_rmdir(tmpfs), ==, 0);
+}
+
 static void test_pxe_one(const char *params, bool ipv6)
 {
     char *args;
@@ -65,6 +202,7 @@ int main(int argc, char *argv[])
     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
         qtest_add_func("pxe/e1000", test_pxe_e1000);
         qtest_add_func("pxe/virtio", test_pxe_virtio_pci);
+        qtest_add_func("pxe/vhost-user", test_pxe_vhost_user);
     } else if (strcmp(arch, "ppc64") == 0) {
         qtest_add_func("pxe/virtio", test_pxe_virtio_pci);
         qtest_add_func("pxe/spapr-vlan", test_pxe_spapr_vlan);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 3/4] libvhost-user: quit when no more data received

[Marc-André Lureau]

On Fri, Jul 21, 2017 at 11:58 AM Jens Freimann <jfreimann@redhat.com> wrote:

> From: Jens Freimann <jfreiman@redhat.com>
>
> End processing of messages when VHOST_USER_NO_MESSAGE
> is received.
>
>
What is VHOST_USER_NO_MESSAGE?


> Without this we run into a vubr_panic() call and get
> "PANIC: Unhandled request: 0"
>
> Signed-off-by: Jens Freimann <jfreiman@redhat.com>
>

---
>  contrib/libvhost-user/libvhost-user.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/libvhost-user/libvhost-user.c
> b/contrib/libvhost-user/libvhost-user.c
> index 9efb9da..85523ca 100644
> --- a/contrib/libvhost-user/libvhost-user.c
> +++ b/contrib/libvhost-user/libvhost-user.c
> @@ -161,7 +161,7 @@ vu_message_read(VuDev *dev, int conn_fd, VhostUserMsg
> *vmsg)
>          rc = recvmsg(conn_fd, &msg, 0);
>      } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
>
> -    if (rc <= 0) {
> +    if (rc < 0) {
>

that's looks fine


>          vu_panic(dev, "Error while recvmsg: %s", strerror(errno));
>          return false;
>      }
> @@ -806,6 +806,8 @@ vu_process_message(VuDev *dev, VhostUserMsg *vmsg)
>          return vu_get_queue_num_exec(dev, vmsg);
>      case VHOST_USER_SET_VRING_ENABLE:
>          return vu_set_vring_enable_exec(dev, vmsg);
> +    case VHOST_USER_NONE:
> +        return true;
>

return true means 'reply_requested', is that what you want so
vu_message_write() is called? If so, please explain in commit message.


>      default:
>          vmsg_close_fds(vmsg);
>          vu_panic(dev, "Unhandled request: %d", vmsg->request);
> --
> 2.9.4
>
>
> --
Marc-André Lureau

Re: [Qemu-devel] [PATCH 3/4] libvhost-user: quit when no more data received

[Jens Freimann]

On Fri, Jul 21, 2017 at 10:59:23AM +0000, Marc-André Lureau wrote:
>On Fri, Jul 21, 2017 at 11:58 AM Jens Freimann <jfreimann@redhat.com> wrote:
>
>> From: Jens Freimann <jfreiman@redhat.com>
>>
>> End processing of messages when VHOST_USER_NO_MESSAGE
>> is received.
>>
>>
>What is VHOST_USER_NO_MESSAGE?

I meant VHOST_USER_NONE. Will fix in next versionn. 
>
>> Without this we run into a vubr_panic() call and get
>> "PANIC: Unhandled request: 0"
>>
>> Signed-off-by: Jens Freimann <jfreiman@redhat.com>
>>
>
>---
>>  contrib/libvhost-user/libvhost-user.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/contrib/libvhost-user/libvhost-user.c
>> b/contrib/libvhost-user/libvhost-user.c
>> index 9efb9da..85523ca 100644
>> --- a/contrib/libvhost-user/libvhost-user.c
>> +++ b/contrib/libvhost-user/libvhost-user.c
>> @@ -161,7 +161,7 @@ vu_message_read(VuDev *dev, int conn_fd, VhostUserMsg
>> *vmsg)
>>          rc = recvmsg(conn_fd, &msg, 0);
>>      } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
>>
>> -    if (rc <= 0) {
>> +    if (rc < 0) {
>>
>
>that's looks fine
>
>
>>          vu_panic(dev, "Error while recvmsg: %s", strerror(errno));
>>          return false;
>>      }
>> @@ -806,6 +806,8 @@ vu_process_message(VuDev *dev, VhostUserMsg *vmsg)
>>          return vu_get_queue_num_exec(dev, vmsg);
>>      case VHOST_USER_SET_VRING_ENABLE:
>>          return vu_set_vring_enable_exec(dev, vmsg);
>> +    case VHOST_USER_NONE:
>> +        return true;
>>
>
>return true means 'reply_requested', is that what you want so
>vu_message_write() is called? If so, please explain in commit message.

No reply needed. I'll change it to return false. 

Thanks for the review!

regards,
Jens

Re: [Qemu-devel] [PATCH 4/4] tests/pxe-test: add testcase using vhost-user-bridge

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 3569 bytes --]

On Fri, Jul 21, 2017 at 11:55:53AM +0200, Jens Freimann wrote:
> +static const char *init_hugepagefs(const char *path)
> +{
> +    struct statfs fs;
> +    int ret;
> +
> +    if (access(path, R_OK | W_OK | X_OK)) {
> +        g_test_message("access on path (%s): %s\n", path, strerror(errno));
> +        return NULL;
> +    }
> +
> +    do {
> +        ret = statfs(path, &fs);

This system-call and HUGETLBFS_MAGIC are Linux-specific but I don't see
anything in the makefile or this test code that restricts it to Linux.
Will this break non-Linux hosts?

Perhaps #ifdef __linux__ is needed.

> +    } while (ret != 0 && errno == EINTR);
> +
> +    if (ret != 0) {
> +        g_test_message("statfs on path (%s): %s\n", path, strerror(errno));
> +        return NULL;
> +    }
> +
> +    if (fs.f_type != HUGETLBFS_MAGIC) {
> +        g_test_message("Warning: path not on HugeTLBFS: %s\n", path);
> +        return NULL;
> +    }
> +
> +    return path;
> +}
> +
> +static int vubr_create_socket(struct sockaddr_in *si_remote, int rport)
> +{
> +    int sock;
> +
> +    if (inet_aton("127.0.0.1", (struct in_addr *) &si_remote->sin_addr.s_addr)
> +            == 0) {
> +        g_test_message("inet_aton failed\n");
> +        return -1;
> +    }

Or:

  si_remote->sin_addr.s_addr = htonl(INADDR_LOOPBACK);

> +    sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
> +    if (sock == -1) {
> +        g_test_message("socket creation failed\n");
> +        return -1;
> +    }
> +    if (connect(sock, (struct sockaddr *) si_remote, sizeof(*si_remote))) {
> +        g_test_message("connect failed: %s", strerror(errno));
> +        return -1;
> +    }
> +
> +    return sock;
> +}
> +
> +static void test_pxe_vhost_user(void)
> +{
> +    char template[] = "/tmp/vhost-user-bridge-XXXXXX";
> +    char template2[] = "/tmp/hugepages-XXXXXX";
> +    gchar * vubr_args[] = {NULL, NULL, NULL, NULL};
> +    struct sockaddr_in si_remote = {
> +        .sin_family = AF_INET,
> +        .sin_port = htons(RPORT),
> +    };
> +    const char *hugefs = NULL;
> +    const char *tmpfs2 = NULL;
> +    const char *tmpfs = NULL;
> +    const char *root = NULL;
> +    GError *error = NULL;
> +    char *vubr_binary;
> +    char *qemu_args;
> +    GPid vubr_pid;
> +    int sock = -1;
> +
> +    tmpfs = mkdtemp(template);
> +    if (!tmpfs) {
> +        g_test_message("mkdtemp on path(%s): %s\n",
> +                       template, strerror(errno));
> +    }
> +    vubr_binary = getenv("QTEST_VUBR_BINARY");
> +    g_assert(vubr_binary);
> +    vubr_args[0] = g_strdup_printf("%s", vubr_binary);
> +    vubr_args[1] = g_strdup_printf("-u");
> +    vubr_args[2] = g_strdup_printf("%s/%s", tmpfs, VUBR_SOCK);
> +    g_spawn_async(NULL, vubr_args, NULL,
> +                  G_SPAWN_SEARCH_PATH_FROM_ENVP |
> +                  G_SPAWN_SEARCH_PATH,
> +                  NULL, NULL, &vubr_pid, &error);

I think a few things are missing for test cleanup (especially on
failure).

From https://developer.gnome.org/glib/stable/glib-Spawning-Processes.html#g-spawn-async-with-pipes:

  If child_pid is not NULL and an error does not occur then the returned
  process reference must be closed using g_spawn_close_pid().

G_SPAWN_DO_NOT_REAP_CHILD wasn't specified so glib will automatically
reap the child but the test case will not know when the vubr process has
terminated.

A qtest_add_abrt_handler() is also missing to kill the vubr child if the
test case terminates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH 4/4] tests/pxe-test: add testcase using vhost-user-bridge

[Michael S. Tsirkin]

On Fri, Jul 21, 2017 at 11:55:53AM +0200, Jens Freimann wrote:
> From: Jens Freimann <jfreiman@redhat.com>
> 
> Add a PXE testcase tunneling traffic through vhost-user-bridge process.
> Create a vhost-user-bridge  process and connect it to qemu via a socket.
> 
> Signed-off-by: Jens Freimann <jfreimann@redhat.com>
> ---
>  tests/Makefile.include |   4 +-
>  tests/pxe-test.c       | 140 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 142 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 7af278d..e40550c 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -705,7 +705,8 @@ tests/boot-order-test$(EXESUF): tests/boot-order-test.o $(libqos-obj-y)
>  tests/boot-serial-test$(EXESUF): tests/boot-serial-test.o $(libqos-obj-y)
>  tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o \
>  	tests/boot-sector.o tests/acpi-utils.o $(libqos-obj-y)
> -tests/pxe-test$(EXESUF): tests/pxe-test.o tests/boot-sector.o $(libqos-obj-y)
> +tests/pxe-test$(EXESUF): tests/pxe-test.o tests/boot-sector.o \
> +    tests/vhost-user-bridge$(EXESUF) $(libqos-obj-y)
>  tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
>  tests/ds1338-test$(EXESUF): tests/ds1338-test.o $(libqos-imx-obj-y)
>  tests/m25p80-test$(EXESUF): tests/m25p80-test.o
> @@ -834,6 +835,7 @@ $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
>  	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
>  	$(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
>  		QTEST_QEMU_IMG=qemu-img$(EXESUF) \
> +		QTEST_VUBR_BINARY=./tests/vhost-user-bridge$(EXESUF) \
>  		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} \
>  		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y) $(check-qtest-generic-y),"GTESTER","$@")
>  	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y) $(gcov-files-generic-y); do \
> diff --git a/tests/pxe-test.c b/tests/pxe-test.c
> index 34282d3..d6379a8 100644
> --- a/tests/pxe-test.c
> +++ b/tests/pxe-test.c
> @@ -5,7 +5,8 @@
>   *
>   * Authors:
>   *  Michael S. Tsirkin <mst@redhat.com>,
> - *  Victor Kaplansky <victork@redhat.com>
> + *  Victor Kaplansky <victork@redhat.com>,
> + *  Jens Freimann <jfreiman@redhat.com>
>   *
>   * This work is licensed under the terms of the GNU GPL, version 2 or later.
>   * See the COPYING file in the top-level directory.
> @@ -13,14 +14,150 @@
>  
>  #include "qemu/osdep.h"
>  #include <glib/gstdio.h>
> +#include <glib.h>
>  #include "qemu-common.h"
>  #include "libqtest.h"
>  #include "boot-sector.h"
> +#include <sys/vfs.h>
>  
> +#define LPORT 5555
> +#define RPORT 4444
>  #define NETNAME "net0"
> +#define QEMU_CMD_MEM    "--enable-kvm -m %d "\
> +                        "-object memory-backend-file,id=mem,size=%dM,"\
> +                        "mem-path=%s,share=on -numa node,memdev=mem -mem-prealloc "
> +#define QEMU_CMD_CHR    " -chardev socket,id=%s,path=%s"
> +#define QEMU_CMD_NETDEV " -device virtio-net-pci,netdev=net0 "\
> +                        " -netdev vhost-user,id=net0,chardev=%s,vhostforce "\
> +                        " -netdev user,id=n0,tftp=./,bootfile=%s "\
> +                        " -netdev socket,id=n1,fd=%d"
> +#define QEMU_CMD_NET    " -device virtio-net-pci,netdev=n0 "\
> +                        " -device virtio-net-pci,netdev=n1 "
> +
> +#define QEMU_CMD        QEMU_CMD_MEM QEMU_CMD_CHR \
> +                        QEMU_CMD_NETDEV QEMU_CMD_NET
> +
> +#define HUGETLBFS_MAGIC       0x958458f6
> +#define VUBR_SOCK "vubr.sock"
> +#define MEMSZ 1024
>  
>  static char disk[] = "tests/pxe-test-disk-XXXXXX";
>  
> +static const char *init_hugepagefs(const char *path)
> +{
> +    struct statfs fs;
> +    int ret;
> +
> +    if (access(path, R_OK | W_OK | X_OK)) {
> +        g_test_message("access on path (%s): %s\n", path, strerror(errno));
> +        return NULL;
> +    }
> +
> +    do {
> +        ret = statfs(path, &fs);
> +    } while (ret != 0 && errno == EINTR);
> +
> +    if (ret != 0) {
> +        g_test_message("statfs on path (%s): %s\n", path, strerror(errno));
> +        return NULL;
> +    }
> +
> +    if (fs.f_type != HUGETLBFS_MAGIC) {
> +        g_test_message("Warning: path not on HugeTLBFS: %s\n", path);
> +        return NULL;
> +    }
> +
> +    return path;
> +}

Why bother?  Rest of the patchset works find on any filesystem.  User
set this for some reason, let's just roll with it.

> +
> +static int vubr_create_socket(struct sockaddr_in *si_remote, int rport)
> +{
> +    int sock;
> +
> +    if (inet_aton("127.0.0.1", (struct in_addr *) &si_remote->sin_addr.s_addr)
> +            == 0) {
> +        g_test_message("inet_aton failed\n");
> +        return -1;
> +    }
> +    sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
> +    if (sock == -1) {
> +        g_test_message("socket creation failed\n");
> +        return -1;
> +    }
> +    if (connect(sock, (struct sockaddr *) si_remote, sizeof(*si_remote))) {
> +        g_test_message("connect failed: %s", strerror(errno));
> +        return -1;
> +    }
> +
> +    return sock;
> +}
> +
> +static void test_pxe_vhost_user(void)
> +{
> +    char template[] = "/tmp/vhost-user-bridge-XXXXXX";
> +    char template2[] = "/tmp/hugepages-XXXXXX";
> +    gchar * vubr_args[] = {NULL, NULL, NULL, NULL};
> +    struct sockaddr_in si_remote = {
> +        .sin_family = AF_INET,
> +        .sin_port = htons(RPORT),
> +    };
> +    const char *hugefs = NULL;
> +    const char *tmpfs2 = NULL;
> +    const char *tmpfs = NULL;
> +    const char *root = NULL;
> +    GError *error = NULL;
> +    char *vubr_binary;
> +    char *qemu_args;
> +    GPid vubr_pid;
> +    int sock = -1;
> +
> +    tmpfs = mkdtemp(template);
> +    if (!tmpfs) {
> +        g_test_message("mkdtemp on path(%s): %s\n",
> +                       template, strerror(errno));
> +    }
> +    vubr_binary = getenv("QTEST_VUBR_BINARY");
> +    g_assert(vubr_binary);
> +    vubr_args[0] = g_strdup_printf("%s", vubr_binary);
> +    vubr_args[1] = g_strdup_printf("-u");
> +    vubr_args[2] = g_strdup_printf("%s/%s", tmpfs, VUBR_SOCK);
> +    g_spawn_async(NULL, vubr_args, NULL,
> +                  G_SPAWN_SEARCH_PATH_FROM_ENVP |
> +                  G_SPAWN_SEARCH_PATH,
> +                  NULL, NULL, &vubr_pid, &error);
> +    g_assert_no_error(error);
> +
> +    hugefs = getenv("QTEST_HUGETLBFS_PATH");
> +    if (hugefs) {
> +        root = init_hugepagefs(hugefs);
> +        g_assert(root);
> +    } else {
> +        tmpfs2 = mkdtemp(template2);
> +        g_assert(tmpfs2);
> +        root = tmpfs2;
> +    }
> +
> +    sock = vubr_create_socket(&si_remote, RPORT);
> +    g_assert_cmpint(sock, !=, -1);
> +
> +    qemu_args = g_strdup_printf(QEMU_CMD, MEMSZ, MEMSZ, (root),
> +                                "char0", vubr_args[2], "char0",
> +                                disk, sock);
> +    qtest_start(qemu_args);
> +    boot_sector_test();
> +    qtest_quit(global_qtest);
> +    g_free(qemu_args);
> +    g_free(vubr_args[0]);
> +    g_free(vubr_args[1]);
> +    g_free(vubr_args[2]);
> +    g_assert_cmpint(g_remove(g_strdup_printf("%s/%s", tmpfs, VUBR_SOCK)),
> +                    ==, 0);
> +    if (tmpfs2) {
> +        g_assert_cmpint(rmdir(tmpfs2), ==, 0);
> +    }
> +    g_assert_cmpint(g_rmdir(tmpfs), ==, 0);
> +}
> +
>  static void test_pxe_one(const char *params, bool ipv6)
>  {
>      char *args;
> @@ -65,6 +202,7 @@ int main(int argc, char *argv[])
>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>          qtest_add_func("pxe/e1000", test_pxe_e1000);
>          qtest_add_func("pxe/virtio", test_pxe_virtio_pci);
> +        qtest_add_func("pxe/vhost-user", test_pxe_vhost_user);
>      } else if (strcmp(arch, "ppc64") == 0) {
>          qtest_add_func("pxe/virtio", test_pxe_virtio_pci);
>          qtest_add_func("pxe/spapr-vlan", test_pxe_spapr_vlan);
> -- 
> 2.9.4

Re: [Qemu-devel] [PATCH 4/4] tests/pxe-test: add testcase using vhost-user-bridge

[Jens Freimann]

On Tue, Jul 25, 2017 at 01:06:51AM +0300, Michael S. Tsirkin wrote:
>On Fri, Jul 21, 2017 at 11:55:53AM +0200, Jens Freimann wrote:
>> From: Jens Freimann <jfreiman@redhat.com>
>>
>> Add a PXE testcase tunneling traffic through vhost-user-bridge process.
>> Create a vhost-user-bridge  process and connect it to qemu via a socket.
>>
>> Signed-off-by: Jens Freimann <jfreimann@redhat.com>
>> ---
>>  tests/Makefile.include |   4 +-
>>  tests/pxe-test.c       | 140 ++++++++++++++++++++++++++++++++++++++++++++++++-
>>  2 files changed, 142 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index 7af278d..e40550c 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -705,7 +705,8 @@ tests/boot-order-test$(EXESUF): tests/boot-order-test.o $(libqos-obj-y)
>>  tests/boot-serial-test$(EXESUF): tests/boot-serial-test.o $(libqos-obj-y)
>>  tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o \
>>  	tests/boot-sector.o tests/acpi-utils.o $(libqos-obj-y)
>> -tests/pxe-test$(EXESUF): tests/pxe-test.o tests/boot-sector.o $(libqos-obj-y)
>> +tests/pxe-test$(EXESUF): tests/pxe-test.o tests/boot-sector.o \
>> +    tests/vhost-user-bridge$(EXESUF) $(libqos-obj-y)
>>  tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
>>  tests/ds1338-test$(EXESUF): tests/ds1338-test.o $(libqos-imx-obj-y)
>>  tests/m25p80-test$(EXESUF): tests/m25p80-test.o
>> @@ -834,6 +835,7 @@ $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
>>  	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
>>  	$(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
>>  		QTEST_QEMU_IMG=qemu-img$(EXESUF) \
>> +		QTEST_VUBR_BINARY=./tests/vhost-user-bridge$(EXESUF) \
>>  		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} \
>>  		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y) $(check-qtest-generic-y),"GTESTER","$@")
>>  	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y) $(gcov-files-generic-y); do \
>> diff --git a/tests/pxe-test.c b/tests/pxe-test.c
>> index 34282d3..d6379a8 100644
>> --- a/tests/pxe-test.c
>> +++ b/tests/pxe-test.c
>> @@ -5,7 +5,8 @@
>>   *
>>   * Authors:
>>   *  Michael S. Tsirkin <mst@redhat.com>,
>> - *  Victor Kaplansky <victork@redhat.com>
>> + *  Victor Kaplansky <victork@redhat.com>,
>> + *  Jens Freimann <jfreiman@redhat.com>
>>   *
>>   * This work is licensed under the terms of the GNU GPL, version 2 or later.
>>   * See the COPYING file in the top-level directory.
>> @@ -13,14 +14,150 @@
>>
>>  #include "qemu/osdep.h"
>>  #include <glib/gstdio.h>
>> +#include <glib.h>
>>  #include "qemu-common.h"
>>  #include "libqtest.h"
>>  #include "boot-sector.h"
>> +#include <sys/vfs.h>
>>
>> +#define LPORT 5555
>> +#define RPORT 4444
>>  #define NETNAME "net0"
>> +#define QEMU_CMD_MEM    "--enable-kvm -m %d "\
>> +                        "-object memory-backend-file,id=mem,size=%dM,"\
>> +                        "mem-path=%s,share=on -numa node,memdev=mem -mem-prealloc "
>> +#define QEMU_CMD_CHR    " -chardev socket,id=%s,path=%s"
>> +#define QEMU_CMD_NETDEV " -device virtio-net-pci,netdev=net0 "\
>> +                        " -netdev vhost-user,id=net0,chardev=%s,vhostforce "\
>> +                        " -netdev user,id=n0,tftp=./,bootfile=%s "\
>> +                        " -netdev socket,id=n1,fd=%d"
>> +#define QEMU_CMD_NET    " -device virtio-net-pci,netdev=n0 "\
>> +                        " -device virtio-net-pci,netdev=n1 "
>> +
>> +#define QEMU_CMD        QEMU_CMD_MEM QEMU_CMD_CHR \
>> +                        QEMU_CMD_NETDEV QEMU_CMD_NET
>> +
>> +#define HUGETLBFS_MAGIC       0x958458f6
>> +#define VUBR_SOCK "vubr.sock"
>> +#define MEMSZ 1024
>>
>>  static char disk[] = "tests/pxe-test-disk-XXXXXX";
>>
>> +static const char *init_hugepagefs(const char *path)
>> +{
>> +    struct statfs fs;
>> +    int ret;
>> +
>> +    if (access(path, R_OK | W_OK | X_OK)) {
>> +        g_test_message("access on path (%s): %s\n", path, strerror(errno));
>> +        return NULL;
>> +    }
>> +
>> +    do {
>> +        ret = statfs(path, &fs);
>> +    } while (ret != 0 && errno == EINTR);
>> +
>> +    if (ret != 0) {
>> +        g_test_message("statfs on path (%s): %s\n", path, strerror(errno));
>> +        return NULL;
>> +    }
>> +
>> +    if (fs.f_type != HUGETLBFS_MAGIC) {
>> +        g_test_message("Warning: path not on HugeTLBFS: %s\n", path);
>> +        return NULL;
>> +    }
>> +
>> +    return path;
>> +}
>
>Why bother?  Rest of the patchset works find on any filesystem.  User
>set this for some reason, let's just roll with it.

We do it like this for the vhost-user testcase. 
Just to be sure: you're suggesting to just get rid of
init_hugepagefs()? 

regards,
Jens 

>> +
>> +static int vubr_create_socket(struct sockaddr_in *si_remote, int rport)
>> +{
>> +    int sock;
>> +
>> +    if (inet_aton("127.0.0.1", (struct in_addr *) &si_remote->sin_addr.s_addr)
>> +            == 0) {
>> +        g_test_message("inet_aton failed\n");
>> +        return -1;
>> +    }
>> +    sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
>> +    if (sock == -1) {
>> +        g_test_message("socket creation failed\n");
>> +        return -1;
>> +    }
>> +    if (connect(sock, (struct sockaddr *) si_remote, sizeof(*si_remote))) {
>> +        g_test_message("connect failed: %s", strerror(errno));
>> +        return -1;
>> +    }
>> +
>> +    return sock;
>> +}
>> +
>> +static void test_pxe_vhost_user(void)
>> +{
>> +    char template[] = "/tmp/vhost-user-bridge-XXXXXX";
>> +    char template2[] = "/tmp/hugepages-XXXXXX";
>> +    gchar * vubr_args[] = {NULL, NULL, NULL, NULL};
>> +    struct sockaddr_in si_remote = {
>> +        .sin_family = AF_INET,
>> +        .sin_port = htons(RPORT),
>> +    };
>> +    const char *hugefs = NULL;
>> +    const char *tmpfs2 = NULL;
>> +    const char *tmpfs = NULL;
>> +    const char *root = NULL;
>> +    GError *error = NULL;
>> +    char *vubr_binary;
>> +    char *qemu_args;
>> +    GPid vubr_pid;
>> +    int sock = -1;
>> +
>> +    tmpfs = mkdtemp(template);
>> +    if (!tmpfs) {
>> +        g_test_message("mkdtemp on path(%s): %s\n",
>> +                       template, strerror(errno));
>> +    }
>> +    vubr_binary = getenv("QTEST_VUBR_BINARY");
>> +    g_assert(vubr_binary);
>> +    vubr_args[0] = g_strdup_printf("%s", vubr_binary);
>> +    vubr_args[1] = g_strdup_printf("-u");
>> +    vubr_args[2] = g_strdup_printf("%s/%s", tmpfs, VUBR_SOCK);
>> +    g_spawn_async(NULL, vubr_args, NULL,
>> +                  G_SPAWN_SEARCH_PATH_FROM_ENVP |
>> +                  G_SPAWN_SEARCH_PATH,
>> +                  NULL, NULL, &vubr_pid, &error);
>> +    g_assert_no_error(error);
>> +
>> +    hugefs = getenv("QTEST_HUGETLBFS_PATH");
>> +    if (hugefs) {
>> +        root = init_hugepagefs(hugefs);
>> +        g_assert(root);
>> +    } else {
>> +        tmpfs2 = mkdtemp(template2);
>> +        g_assert(tmpfs2);
>> +        root = tmpfs2;
>> +    }
>> +
>> +    sock = vubr_create_socket(&si_remote, RPORT);
>> +    g_assert_cmpint(sock, !=, -1);
>> +
>> +    qemu_args = g_strdup_printf(QEMU_CMD, MEMSZ, MEMSZ, (root),
>> +                                "char0", vubr_args[2], "char0",
>> +                                disk, sock);
>> +    qtest_start(qemu_args);
>> +    boot_sector_test();
>> +    qtest_quit(global_qtest);
>> +    g_free(qemu_args);
>> +    g_free(vubr_args[0]);
>> +    g_free(vubr_args[1]);
>> +    g_free(vubr_args[2]);
>> +    g_assert_cmpint(g_remove(g_strdup_printf("%s/%s", tmpfs, VUBR_SOCK)),
>> +                    ==, 0);
>> +    if (tmpfs2) {
>> +        g_assert_cmpint(rmdir(tmpfs2), ==, 0);
>> +    }
>> +    g_assert_cmpint(g_rmdir(tmpfs), ==, 0);
>> +}
>> +
>>  static void test_pxe_one(const char *params, bool ipv6)
>>  {
>>      char *args;
>> @@ -65,6 +202,7 @@ int main(int argc, char *argv[])
>>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>>          qtest_add_func("pxe/e1000", test_pxe_e1000);
>>          qtest_add_func("pxe/virtio", test_pxe_virtio_pci);
>> +        qtest_add_func("pxe/vhost-user", test_pxe_vhost_user);
>>      } else if (strcmp(arch, "ppc64") == 0) {
>>          qtest_add_func("pxe/virtio", test_pxe_virtio_pci);
>>          qtest_add_func("pxe/spapr-vlan", test_pxe_spapr_vlan);
>> --
>> 2.9.4

Re: [Qemu-devel] [PATCH 2/4] net: fix -netdev socket, fd= for UDP sockets

[Michael S. Tsirkin]

On Fri, Jul 21, 2017 at 11:55:51AM +0200, Jens Freimann wrote:
> This patch fixes -netdev socket,fd= for UDP sockets
> Currently -netdev socket,fd=<...> results in
> 
>   qemu: error: specified mcastaddr "127.0.0.1" (0x7f000001) does not
>     contain a multicast address
>   qemu-system-x86_64: -netdev
>     socket,id=n1,fd=3: Device 'socket' could not be initialized
> 
> To fix these we need to allow specifying multicast and fd arguments
> for the same netdev. With this the user can specify "-netdev
> fd=3,mcast=<IP:port>"
> 
> Fixes: 3d830459b1eccdb61b75e2712fd364012ce5a115
> Signed-off-by: Jens Freimann <jfreimann@redhat.com>

Pls Cc Jason.

> ---
>  net/socket.c | 37 ++++++++++++++++++-------------------
>  1 file changed, 18 insertions(+), 19 deletions(-)
> 
> diff --git a/net/socket.c b/net/socket.c
> index f85ef7d..18af2ab 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -320,11 +320,11 @@ static NetClientInfo net_dgram_socket_info = {
>  static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer,
>                                                  const char *model,
>                                                  const char *name,
> -                                                int fd, int is_connected)
> +                                                int fd, int is_connected,
> +                                                const char *mcast)
>  {
>      struct sockaddr_in saddr;
>      int newfd;
> -    socklen_t saddr_len = sizeof(saddr);
>      NetClientState *nc;
>      NetSocketState *s;
>  
> @@ -333,8 +333,13 @@ static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer,
>       * by ONLY ONE process: we must "clone" this dgram socket --jjo
>       */
>  
> -    if (is_connected) {
> -        if (getsockname(fd, (struct sockaddr *) &saddr, &saddr_len) == 0) {
> +    if (is_connected && mcast != NULL) {
> +            if (parse_host_port(&saddr, mcast) < 0) {
> +                fprintf(stderr,
> +                        "qemu: error: init_dgram: fd=%d failed parse_host_port()\n",
> +                        fd);
> +                goto err;
> +            }
>              /* must be bound */
>              if (saddr.sin_addr.s_addr == 0) {
>                  fprintf(stderr, "qemu: error: init_dgram: fd=%d unbound, "
> @@ -351,12 +356,6 @@ static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer,
>              dup2(newfd, fd);
>              close(newfd);
>  
> -        } else {
> -            fprintf(stderr,
> -                    "qemu: error: init_dgram: fd=%d failed getsockname(): %s\n",
> -                    fd, strerror(errno));
> -            goto err;
> -        }
>      }
>  
>      nc = qemu_new_net_client(&net_dgram_socket_info, peer, model, name);
> @@ -432,7 +431,7 @@ static NetSocketState *net_socket_fd_init_stream(NetClientState *peer,
>  
>  static NetSocketState *net_socket_fd_init(NetClientState *peer,
>                                            const char *model, const char *name,
> -                                          int fd, int is_connected)
> +                                          int fd, int is_connected, const char *mc)
>  {
>      int so_type = -1, optlen=sizeof(so_type);
>  
> @@ -445,7 +444,7 @@ static NetSocketState *net_socket_fd_init(NetClientState *peer,
>      }
>      switch(so_type) {
>      case SOCK_DGRAM:
> -        return net_socket_fd_init_dgram(peer, model, name, fd, is_connected);
> +        return net_socket_fd_init_dgram(peer, model, name, fd, is_connected, mc);
>      case SOCK_STREAM:
>          return net_socket_fd_init_stream(peer, model, name, fd, is_connected);
>      default:
> @@ -567,7 +566,7 @@ static int net_socket_connect_init(NetClientState *peer,
>              break;
>          }
>      }
> -    s = net_socket_fd_init(peer, model, name, fd, connected);
> +    s = net_socket_fd_init(peer, model, name, fd, connected, NULL);
>      if (!s)
>          return -1;
>      snprintf(s->nc.info_str, sizeof(s->nc.info_str),
> @@ -602,7 +601,7 @@ static int net_socket_mcast_init(NetClientState *peer,
>      if (fd < 0)
>          return -1;
>  
> -    s = net_socket_fd_init(peer, model, name, fd, 0);
> +    s = net_socket_fd_init(peer, model, name, fd, 0, NULL);
>      if (!s)
>          return -1;
>  
> @@ -652,7 +651,7 @@ static int net_socket_udp_init(NetClientState *peer,
>      }
>      qemu_set_nonblock(fd);
>  
> -    s = net_socket_fd_init(peer, model, name, fd, 0);
> +    s = net_socket_fd_init(peer, model, name, fd, 0, NULL);
>      if (!s) {
>          return -1;
>      }
> @@ -675,9 +674,9 @@ int net_init_socket(const Netdev *netdev, const char *name,
>      assert(netdev->type == NET_CLIENT_DRIVER_SOCKET);
>      sock = &netdev->u.socket;
>  
> -    if (sock->has_fd + sock->has_listen + sock->has_connect + sock->has_mcast +
> -        sock->has_udp != 1) {
> -        error_report("exactly one of fd=, listen=, connect=, mcast= or udp="
> +    if (sock->has_listen + sock->has_connect + sock->has_mcast +
> +        sock->has_udp > 1) {
> +        error_report("exactly one of listen=, connect=, mcast= or udp="
>                       " is required");
>          return -1;
>      }
> @@ -696,7 +695,7 @@ int net_init_socket(const Netdev *netdev, const char *name,
>              return -1;
>          }
>          qemu_set_nonblock(fd);
> -        if (!net_socket_fd_init(peer, "socket", name, fd, 1)) {
> +        if (!net_socket_fd_init(peer, "socket", name, fd, 1, sock->mcast)) {
>              return -1;
>          }
>          return 0;
> -- 
> 2.9.4

Re: [Qemu-devel] [PATCH 2/4] net: fix -netdev socket, fd= for UDP sockets

[Michael S. Tsirkin]

On Fri, Jul 21, 2017 at 11:55:51AM +0200, Jens Freimann wrote:
> This patch fixes -netdev socket,fd= for UDP sockets
> Currently -netdev socket,fd=<...> results in
> 
>   qemu: error: specified mcastaddr "127.0.0.1" (0x7f000001) does not
>     contain a multicast address
>   qemu-system-x86_64: -netdev
>     socket,id=n1,fd=3: Device 'socket' could not be initialized
> 
> To fix these we need to allow specifying multicast and fd arguments
> for the same netdev. With this the user can specify "-netdev
> fd=3,mcast=<IP:port>"
> 
> Fixes: 3d830459b1eccdb61b75e2712fd364012ce5a115
> Signed-off-by: Jens Freimann <jfreimann@redhat.com>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  net/socket.c | 37 ++++++++++++++++++-------------------
>  1 file changed, 18 insertions(+), 19 deletions(-)
> 
> diff --git a/net/socket.c b/net/socket.c
> index f85ef7d..18af2ab 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -320,11 +320,11 @@ static NetClientInfo net_dgram_socket_info = {
>  static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer,
>                                                  const char *model,
>                                                  const char *name,
> -                                                int fd, int is_connected)
> +                                                int fd, int is_connected,
> +                                                const char *mcast)
>  {
>      struct sockaddr_in saddr;
>      int newfd;
> -    socklen_t saddr_len = sizeof(saddr);
>      NetClientState *nc;
>      NetSocketState *s;
>  
> @@ -333,8 +333,13 @@ static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer,
>       * by ONLY ONE process: we must "clone" this dgram socket --jjo
>       */
>  
> -    if (is_connected) {
> -        if (getsockname(fd, (struct sockaddr *) &saddr, &saddr_len) == 0) {
> +    if (is_connected && mcast != NULL) {
> +            if (parse_host_port(&saddr, mcast) < 0) {
> +                fprintf(stderr,
> +                        "qemu: error: init_dgram: fd=%d failed parse_host_port()\n",
> +                        fd);
> +                goto err;
> +            }
>              /* must be bound */
>              if (saddr.sin_addr.s_addr == 0) {
>                  fprintf(stderr, "qemu: error: init_dgram: fd=%d unbound, "
> @@ -351,12 +356,6 @@ static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer,
>              dup2(newfd, fd);
>              close(newfd);
>  
> -        } else {
> -            fprintf(stderr,
> -                    "qemu: error: init_dgram: fd=%d failed getsockname(): %s\n",
> -                    fd, strerror(errno));
> -            goto err;
> -        }
>      }
>  
>      nc = qemu_new_net_client(&net_dgram_socket_info, peer, model, name);
> @@ -432,7 +431,7 @@ static NetSocketState *net_socket_fd_init_stream(NetClientState *peer,
>  
>  static NetSocketState *net_socket_fd_init(NetClientState *peer,
>                                            const char *model, const char *name,
> -                                          int fd, int is_connected)
> +                                          int fd, int is_connected, const char *mc)
>  {
>      int so_type = -1, optlen=sizeof(so_type);
>  
> @@ -445,7 +444,7 @@ static NetSocketState *net_socket_fd_init(NetClientState *peer,
>      }
>      switch(so_type) {
>      case SOCK_DGRAM:
> -        return net_socket_fd_init_dgram(peer, model, name, fd, is_connected);
> +        return net_socket_fd_init_dgram(peer, model, name, fd, is_connected, mc);
>      case SOCK_STREAM:
>          return net_socket_fd_init_stream(peer, model, name, fd, is_connected);
>      default:
> @@ -567,7 +566,7 @@ static int net_socket_connect_init(NetClientState *peer,
>              break;
>          }
>      }
> -    s = net_socket_fd_init(peer, model, name, fd, connected);
> +    s = net_socket_fd_init(peer, model, name, fd, connected, NULL);
>      if (!s)
>          return -1;
>      snprintf(s->nc.info_str, sizeof(s->nc.info_str),
> @@ -602,7 +601,7 @@ static int net_socket_mcast_init(NetClientState *peer,
>      if (fd < 0)
>          return -1;
>  
> -    s = net_socket_fd_init(peer, model, name, fd, 0);
> +    s = net_socket_fd_init(peer, model, name, fd, 0, NULL);
>      if (!s)
>          return -1;
>  
> @@ -652,7 +651,7 @@ static int net_socket_udp_init(NetClientState *peer,
>      }
>      qemu_set_nonblock(fd);
>  
> -    s = net_socket_fd_init(peer, model, name, fd, 0);
> +    s = net_socket_fd_init(peer, model, name, fd, 0, NULL);
>      if (!s) {
>          return -1;
>      }
> @@ -675,9 +674,9 @@ int net_init_socket(const Netdev *netdev, const char *name,
>      assert(netdev->type == NET_CLIENT_DRIVER_SOCKET);
>      sock = &netdev->u.socket;
>  
> -    if (sock->has_fd + sock->has_listen + sock->has_connect + sock->has_mcast +
> -        sock->has_udp != 1) {
> -        error_report("exactly one of fd=, listen=, connect=, mcast= or udp="
> +    if (sock->has_listen + sock->has_connect + sock->has_mcast +
> +        sock->has_udp > 1) {
> +        error_report("exactly one of listen=, connect=, mcast= or udp="
>                       " is required");
>          return -1;
>      }
> @@ -696,7 +695,7 @@ int net_init_socket(const Netdev *netdev, const char *name,
>              return -1;
>          }
>          qemu_set_nonblock(fd);
> -        if (!net_socket_fd_init(peer, "socket", name, fd, 1)) {
> +        if (!net_socket_fd_init(peer, "socket", name, fd, 1, sock->mcast)) {
>              return -1;
>          }
>          return 0;
> -- 
> 2.9.4

Re: [Qemu-devel] [PATCH 4/4] tests/pxe-test: add testcase using vhost-user-bridge

[Jens Freimann]

On Mon, Jul 24, 2017 at 02:42:43PM +0100, Stefan Hajnoczi wrote:
>On Fri, Jul 21, 2017 at 11:55:53AM +0200, Jens Freimann wrote:
>> +static const char *init_hugepagefs(const char *path)
>> +{
>> +    struct statfs fs;
>> +    int ret;
>> +
>> +    if (access(path, R_OK | W_OK | X_OK)) {
>> +        g_test_message("access on path (%s): %s\n", path, strerror(errno));
>> +        return NULL;
>> +    }
>> +
>> +    do {
>> +        ret = statfs(path, &fs);
>
>This system-call and HUGETLBFS_MAGIC are Linux-specific but I don't see
>anything in the makefile or this test code that restricts it to Linux.
>Will this break non-Linux hosts?
>
>Perhaps #ifdef __linux__ is needed.

Yes, I think so. I might get rid of it init_hugepagefs altogether
though. 

>
>> +    } while (ret != 0 && errno == EINTR);
>> +
>> +    if (ret != 0) {
>> +        g_test_message("statfs on path (%s): %s\n", path, strerror(errno));
>> +        return NULL;
>> +    }
>> +
>> +    if (fs.f_type != HUGETLBFS_MAGIC) {
>> +        g_test_message("Warning: path not on HugeTLBFS: %s\n", path);
>> +        return NULL;
>> +    }
>> +
>> +    return path;
>> +}
>> +
>> +static int vubr_create_socket(struct sockaddr_in *si_remote, int rport)
>> +{
>> +    int sock;
>> +
>> +    if (inet_aton("127.0.0.1", (struct in_addr *) &si_remote->sin_addr.s_addr)
>> +            == 0) {
>> +        g_test_message("inet_aton failed\n");
>> +        return -1;
>> +    }
>
>Or:
>
>  si_remote->sin_addr.s_addr = htonl(INADDR_LOOPBACK);

Yes, I'll change it. 

>> +    sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
>> +    if (sock == -1) {
>> +        g_test_message("socket creation failed\n");
>> +        return -1;
>> +    }
>> +    if (connect(sock, (struct sockaddr *) si_remote, sizeof(*si_remote))) {
>> +        g_test_message("connect failed: %s", strerror(errno));
>> +        return -1;
>> +    }
>> +
>> +    return sock;
>> +}
>> +
>> +static void test_pxe_vhost_user(void)
>> +{
>> +    char template[] = "/tmp/vhost-user-bridge-XXXXXX";
>> +    char template2[] = "/tmp/hugepages-XXXXXX";
>> +    gchar * vubr_args[] = {NULL, NULL, NULL, NULL};
>> +    struct sockaddr_in si_remote = {
>> +        .sin_family = AF_INET,
>> +        .sin_port = htons(RPORT),
>> +    };
>> +    const char *hugefs = NULL;
>> +    const char *tmpfs2 = NULL;
>> +    const char *tmpfs = NULL;
>> +    const char *root = NULL;
>> +    GError *error = NULL;
>> +    char *vubr_binary;
>> +    char *qemu_args;
>> +    GPid vubr_pid;
>> +    int sock = -1;
>> +
>> +    tmpfs = mkdtemp(template);
>> +    if (!tmpfs) {
>> +        g_test_message("mkdtemp on path(%s): %s\n",
>> +                       template, strerror(errno));
>> +    }
>> +    vubr_binary = getenv("QTEST_VUBR_BINARY");
>> +    g_assert(vubr_binary);
>> +    vubr_args[0] = g_strdup_printf("%s", vubr_binary);
>> +    vubr_args[1] = g_strdup_printf("-u");
>> +    vubr_args[2] = g_strdup_printf("%s/%s", tmpfs, VUBR_SOCK);
>> +    g_spawn_async(NULL, vubr_args, NULL,
>> +                  G_SPAWN_SEARCH_PATH_FROM_ENVP |
>> +                  G_SPAWN_SEARCH_PATH,
>> +                  NULL, NULL, &vubr_pid, &error);
>
>I think a few things are missing for test cleanup (especially on
>failure).
>
>From https://developer.gnome.org/glib/stable/glib-Spawning-Processes.html#g-spawn-async-with-pipes:
>
>  If child_pid is not NULL and an error does not occur then the returned
>  process reference must be closed using g_spawn_close_pid().
>
>G_SPAWN_DO_NOT_REAP_CHILD wasn't specified so glib will automatically
>reap the child but the test case will not know when the vubr process has
>terminated.
>
>A qtest_add_abrt_handler() is also missing to kill the vubr child if the
>test case terminates.

I'll add an abort handler and make sure that the vubr process is
terminated. 

Thanks for the review!

regards,
Jens 

Re: [Qemu-devel] [PATCH 0/4] tests/pxe-test: add testcase using vhost-user-bridge

[no-reply]

Hi,

This series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Type: series
Subject: [Qemu-devel] [PATCH 0/4] tests/pxe-test: add testcase using vhost-user-bridge
Message-id: 20170721095553.10717-1-jfreimann@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-quick@centos6
time make docker-test-build@min-glib
time make docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
4e60aea tests/pxe-test: add testcase using vhost-user-bridge
21e584d libvhost-user: quit when no more data received
bc0c757 net: fix -netdev socket, fd= for UDP sockets
22e423d tests/vhost-user-bridge: disable debug output by default

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-pwrch0lm/src/dtc'...
Submodule path 'dtc': checked out '558cd81bdd432769b59bff01240c44f82cfb1a9d'
  BUILD   centos6
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-pwrch0lm/src'
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPY    RUNNER
    RUN test-quick in qemu:centos6 
/usr/bin/docker-current: Error response from daemon: containerd: container did not start before the specified timeout.
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 382, in <module>
    sys.exit(main())
  File "./tests/docker/docker.py", line 379, in main
    return args.cmdobj.run(args, argv)
  File "./tests/docker/docker.py", line 237, in run
    return Docker().run(argv, args.keep, quiet=args.quiet)
  File "./tests/docker/docker.py", line 205, in run
    quiet=quiet)
  File "./tests/docker/docker.py", line 123, in _do_check
    return subprocess.check_call(self._command + cmd, **kwargs)
  File "/usr/lib64/python2.7/subprocess.py", line 186, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['docker', 'run', '--label', 'com.qemu.instance.uuid=b0088d2a72ca11e794e1525400c803e1', '-u', '0', '-t', '--rm', '--net=none', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=8', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/var/tmp/patchew-tester-tmp-pwrch0lm/src/docker-src.2017-07-27-08.54.08.1913:/var/tmp/qemu:z,ro', '-v', '/root/.cache/qemu-docker-ccache:/var/tmp/ccache:z', 'qemu:centos6', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 125
tests/docker/Makefile.include:136: recipe for target 'docker-run' failed
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory '/var/tmp/patchew-tester-tmp-pwrch0lm/src'
tests/docker/Makefile.include:168: recipe for target 'docker-run-test-quick@centos6' failed
make: *** [docker-run-test-quick@centos6] Error 2
=== OUTPUT END ===

Test command exited with code: 2


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org