* [Qemu-devel] [PATCH] test-filter-mirror: pass UNIX domain socket through fd
@ 2019-01-28 4:11 Jason Wang
2019-01-28 10:30 ` Daniel P. Berrangé
0 siblings, 1 reply; 3+ messages in thread
From: Jason Wang @ 2019-01-28 4:11 UTC (permalink / raw)
To: qemu-devel, jasowang
Cc: Peter Maydell, Li Zhijian, Peter Xu, Dr . David Alan Gilbert,
Zhang Chen, Markus Armbruster, Daniel P . Berrange
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 <peter.maydell@linaro.org>
Cc: Li Zhijian <lizhijian@cn.fujitsu.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Zhang Chen <zhangckid@gmail.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
tests/test-filter-mirror.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/tests/test-filter-mirror.c b/tests/test-filter-mirror.c
index 7ab2aed8a0..3c3d1f8961 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,16 @@ 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);
qtest_quit(qts);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] test-filter-mirror: pass UNIX domain socket through fd
2019-01-28 4:11 [Qemu-devel] [PATCH] test-filter-mirror: pass UNIX domain socket through fd Jason Wang
@ 2019-01-28 10:30 ` Daniel P. Berrangé
2019-01-29 2:52 ` Jason Wang
0 siblings, 1 reply; 3+ messages in thread
From: Daniel P. Berrangé @ 2019-01-28 10:30 UTC (permalink / raw)
To: Jason Wang
Cc: qemu-devel, Peter Maydell, Li Zhijian, Peter Xu,
Dr . David Alan Gilbert, Zhang Chen, Markus Armbruster
On Mon, Jan 28, 2019 at 12:11:59PM +0800, Jason Wang 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 <peter.maydell@linaro.org>
> Cc: Li Zhijian <lizhijian@cn.fujitsu.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Cc: Zhang Chen <zhangckid@gmail.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Daniel P. Berrange <berrange@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> tests/test-filter-mirror.c | 18 ++++++------------
> 1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/tests/test-filter-mirror.c b/tests/test-filter-mirror.c
> index 7ab2aed8a0..3c3d1f8961 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,16 @@ 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);
You're leaking recv_sock[0] and recv_sock[1] now. For that matter it
seems send_sock[0] & send_sock[1] are already both leaked too.
> - unlink(sock_path);
> qtest_quit(qts);
> }
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] test-filter-mirror: pass UNIX domain socket through fd
2019-01-28 10:30 ` Daniel P. Berrangé
@ 2019-01-29 2:52 ` Jason Wang
0 siblings, 0 replies; 3+ messages in thread
From: Jason Wang @ 2019-01-29 2:52 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Peter Maydell, Li Zhijian, Markus Armbruster, qemu-devel,
Peter Xu, Dr . David Alan Gilbert, Zhang Chen
On 2019/1/28 下午6:30, Daniel P. Berrangé wrote:
> On Mon, Jan 28, 2019 at 12:11:59PM +0800, Jason Wang 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 <peter.maydell@linaro.org>
>> Cc: Li Zhijian <lizhijian@cn.fujitsu.com>
>> Cc: Peter Xu <peterx@redhat.com>
>> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> Cc: Zhang Chen <zhangckid@gmail.com>
>> Cc: Markus Armbruster <armbru@redhat.com>
>> Cc: Daniel P. Berrange <berrange@redhat.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> tests/test-filter-mirror.c | 18 ++++++------------
>> 1 file changed, 6 insertions(+), 12 deletions(-)
>>
>> diff --git a/tests/test-filter-mirror.c b/tests/test-filter-mirror.c
>> index 7ab2aed8a0..3c3d1f8961 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,16 @@ 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);
> You're leaking recv_sock[0] and recv_sock[1] now. For that matter it
> seems send_sock[0] & send_sock[1] are already both leaked too.
Will fix in V2.
Thanks
>> - unlink(sock_path);
>> qtest_quit(qts);
>> }
> Regards,
> Daniel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-01-29 2:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-28 4:11 [Qemu-devel] [PATCH] test-filter-mirror: pass UNIX domain socket through fd Jason Wang
2019-01-28 10:30 ` Daniel P. Berrangé
2019-01-29 2:52 ` Jason Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).