Re: [PATCH V2] vhost-user-test: fix a memory leak

[Thomas Huth]

On 15/01/2020 10.13, Thomas Huth wrote:
> On 15/01/2020 04.10, Pan Nengyuan wrote:
>>
>> On 1/13/2020 10:32 AM, Pan Nengyuan wrote:
>>>
>>> On 1/12/2020 6:39 PM, Thomas Huth wrote:
> [...]
>>>> ... and now I had to unqueue the patch again. It is reproducibly causing
>>>> one of the gitlab CI pipelines to fail with a timeout, e.g.:
>>>>
>>>>  https://gitlab.com/huth/qemu/-/jobs/400101552
>>>>
>>>> Not sure what is going on here, though, there is no obvious error
>>>> message in the output... this needs some more investigation... do you
>>>> have a gitlab account and could have a look?
>>>>
>>>
>>> OK, I will register a account and have a look.
>>>
>>
>> I'm sorry, I build and test with the same params, but I can't reproduce it.
>> Could you add "V=1 or V=2" params to get more information ?
> 
> It seems to hang forever in qos-test
> /arm/virt/virtio-mmio/virtio-bus/virtio-net-device/virtio-net/virtio-net-tests/announce-self
> :
> 
>  https://gitlab.com/huth/qemu/-/jobs/403472594
> 
> It's completely weird, I also added some fprintf statements:
> 
>  https://gitlab.com/huth/qemu/commit/8ae76c0cf37cf46d26620dd
> 
> ... but none of them show up in the output of the test run... so I'm
> currently completely puzzled what might be going wrong here... Any other
> ideas what we could try here?

I tried to add some more fprintfs here and there to see where it hangs,
but I did not succeed to get any further.

However, the CI build succeeds with this fix instead:

diff a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
--- a/tests/qtest/vhost-user-test.c
+++ b/tests/qtest/vhost-user-test.c
@@ -707,9 +707,9 @@ static void test_read_guest_mem(void *obj, void
*arg, QGuestAllocator *alloc)
 static void test_migrate(void *obj, void *arg, QGuestAllocator *alloc)
 {
     TestServer *s = arg;
-    TestServer *dest = test_server_new("dest");
-    GString *dest_cmdline = g_string_new(qos_get_current_command_line());
-    char *uri = g_strdup_printf("%s%s", "unix:", dest->mig_path);
+    TestServer *dest;
+    GString *dest_cmdline;
+    char *uri;
     QTestState *to;
     GSource *source;
     QDict *rsp;
@@ -720,6 +720,10 @@ static void test_migrate(void *obj, void *arg,
QGuestAllocator *alloc)
         return;
     }

+    dest = test_server_new("dest");
+    dest_cmdline = g_string_new(qos_get_current_command_line());
+    uri = g_strdup_printf("%s%s", "unix:", dest->mig_path);
+
     size = get_log_size(s);
     g_assert_cmpint(size, ==, (256 * 1024 * 1024) / (VHOST_LOG_PAGE * 8));

@@ -778,6 +782,7 @@ static void test_migrate(void *obj, void *arg,
QGuestAllocator *alloc)
     qtest_quit(to);
     test_server_free(dest);
     g_free(uri);
+    g_string_free(dest_cmdline, true);
 }


Here's a build with that patch that succeeded:

 https://gitlab.com/huth/qemu/-/jobs/405357307

So if you agree with that patch, I think we should simply use that
version instead, ok?

 Thomas