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

[Pan Nengyuan]

On 1/16/2020 9:29 PM, Thomas Huth wrote:
> 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?

Ok, I agree with it.

Thanks.

> 
>  Thomas
> 
> .
>