Re: [Qemu-devel] [PATCH] test-filter-mirror: pass UNIX domain socket through fd

[Jason Wang <jasowang@redhat.com>]

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