[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V2] test-filter-mirror: pass UNIX domain socket
From: |
Zhang Chen |
Subject: |
Re: [Qemu-devel] [PATCH V2] test-filter-mirror: pass UNIX domain socket through fd |
Date: |
Thu, 31 Jan 2019 10:25:04 +0800 |
On Wed, Jan 30, 2019 at 11:14 AM Jason Wang <address@hidden> wrote:
> The tests tries to let qemu server mode to process the connection
> which turns out to be racy after commit 8258292e18c3 ("monitor: Remove
> "x-oob", offer capability "oob" unconditionally"). This is because the
> filter may try to mirror the packets before UNIX socket object is
> ready (connected was set to true) from the view of qemu. In this case
> the packet will be dropped silently.
>
> Fixing this by passing pre-connected socket created by socketpair() to
> qemu through fd.
>
> Cc: Peter Maydell <address@hidden>
> Cc: Li Zhijian <address@hidden>
> Cc: Peter Xu <address@hidden>
> Cc: Dr. David Alan Gilbert <address@hidden>
> Cc: Zhang Chen <address@hidden>
> Cc: Markus Armbruster <address@hidden>
> Cc: Daniel P. Berrange <address@hidden>
> Signed-off-by: Jason Wang <address@hidden>
>
Looks good for me.
Reviewed-by: Zhang Chen <address@hidden>
By the way: Do we need add ASCII diagram to explain data flow in this file
or commit log?
Thanks
Zhang Chen
> ---
> Changes from v1:
> - close the socket pairs in the end of test
> ---
> tests/test-filter-mirror.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/tests/test-filter-mirror.c b/tests/test-filter-mirror.c
> index 7ab2aed8a0..d942c74a3f 100644
> --- a/tests/test-filter-mirror.c
> +++ b/tests/test-filter-mirror.c
> @@ -21,10 +21,9 @@
>
> static void test_mirror(void)
> {
> - int send_sock[2], recv_sock;
> + int send_sock[2], recv_sock[2];
> uint32_t ret = 0, len = 0;
> char send_buf[] = "Hello! filter-mirror~";
> - char sock_path[] = "filter-mirror.XXXXXX";
> char *recv_buf;
> uint32_t size = sizeof(send_buf);
> size = htonl(size);
> @@ -38,18 +37,15 @@ static void test_mirror(void)
> ret = socketpair(PF_UNIX, SOCK_STREAM, 0, send_sock);
> g_assert_cmpint(ret, !=, -1);
>
> - ret = mkstemp(sock_path);
> + ret = socketpair(PF_UNIX, SOCK_STREAM, 0, recv_sock);
> g_assert_cmpint(ret, !=, -1);
>
> qts = qtest_initf(
> "-netdev socket,id=qtest-bn0,fd=%d "
> "-device %s,netdev=qtest-bn0,id=qtest-e0 "
> - "-chardev socket,id=mirror0,path=%s,server,nowait "
> + "-chardev socket,id=mirror0,fd=%d "
> "-object
> filter-mirror,id=qtest-f0,netdev=qtest-bn0,queue=tx,outdev=mirror0 "
> - , send_sock[1], devstr, sock_path);
> -
> - recv_sock = unix_connect(sock_path, NULL);
> - g_assert_cmpint(recv_sock, !=, -1);
> + , send_sock[1], devstr, recv_sock[1]);
>
> struct iovec iov[] = {
> {
> @@ -67,18 +63,20 @@ static void test_mirror(void)
> g_assert_cmpint(ret, ==, sizeof(send_buf) + sizeof(size));
> close(send_sock[0]);
>
> - ret = qemu_recv(recv_sock, &len, sizeof(len), 0);
> + ret = qemu_recv(recv_sock[0], &len, sizeof(len), 0);
> g_assert_cmpint(ret, ==, sizeof(len));
> len = ntohl(len);
>
> g_assert_cmpint(len, ==, sizeof(send_buf));
> recv_buf = g_malloc(len);
> - ret = qemu_recv(recv_sock, recv_buf, len, 0);
> + ret = qemu_recv(recv_sock[0], recv_buf, len, 0);
> g_assert_cmpstr(recv_buf, ==, send_buf);
>
> g_free(recv_buf);
> - close(recv_sock);
> - unlink(sock_path);
> + close(send_sock[0]);
> + close(send_sock[1]);
> + close(recv_sock[0]);
> + close(recv_sock[1]);
> qtest_quit(qts);
> }
>
> --
> 2.17.1
>
>