Re: [PATCH V2] vhost-user-test: fix a memory leak
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
>
> .
>
Re: [PATCH V2] vhost-user-test: fix a memory leak
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
Re: [PATCH V2] vhost-user-test: fix a memory leak
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? Thomas
Re: [PATCH V2] vhost-user-test: fix a memory leak
On 1/13/2020 10:32 AM, Pan Nengyuan wrote: > > > On 1/12/2020 6:39 PM, Thomas Huth wrote: >> On 10/01/2020 15.07, Thomas Huth wrote: >>> On 20/12/2019 02.26, pannengy...@huawei.com wrote: From: Pan Nengyuan Spotted by ASAN. Reported-by: Euler Robot Signed-off-by: Pan Nengyuan --- Changes V2 to V1: - use a "goto cleanup", instead of duplicating the "free" functions. - free "dest_cmdline" at the end. --- >>> >>> Reviewed-by: Thomas Huth >>> >>> ... and picked up to my qtest-next tree. >> >> ... 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 ? >> Thomas >> >> . >>
Re: [PATCH V2] vhost-user-test: fix a memory leak
On 1/12/2020 6:39 PM, Thomas Huth wrote: > On 10/01/2020 15.07, Thomas Huth wrote: >> On 20/12/2019 02.26, pannengy...@huawei.com wrote: >>> From: Pan Nengyuan >>> >>> Spotted by ASAN. >>> >>> Reported-by: Euler Robot >>> Signed-off-by: Pan Nengyuan >>> --- >>> Changes V2 to V1: >>> - use a "goto cleanup", instead of duplicating the "free" functions. >>> - free "dest_cmdline" at the end. >>> --- >> >> Reviewed-by: Thomas Huth >> >> ... and picked up to my qtest-next tree. > > ... 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. > Thomas > > . >
Re: [PATCH V2] vhost-user-test: fix a memory leak
On 10/01/2020 15.07, Thomas Huth wrote:
> On 20/12/2019 02.26, pannengy...@huawei.com wrote:
>> From: Pan Nengyuan
>>
>> Spotted by ASAN.
>>
>> Reported-by: Euler Robot
>> Signed-off-by: Pan Nengyuan
>> ---
>> Changes V2 to V1:
>> - use a "goto cleanup", instead of duplicating the "free" functions.
>> - free "dest_cmdline" at the end.
>> ---
>> tests/vhost-user-test.c | 5 -
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
>> index 91ea373..dcb8617 100644
>> --- a/tests/vhost-user-test.c
>> +++ b/tests/vhost-user-test.c
>> @@ -717,7 +717,7 @@ static void test_migrate(void *obj, void *arg,
>> QGuestAllocator *alloc)
>> guint64 size;
>>
>> if (!wait_for_fds(s)) {
>> -return;
>> +goto cleanup;
>> }
>>
>> size = get_log_size(s);
>> @@ -776,8 +776,11 @@ static void test_migrate(void *obj, void *arg,
>> QGuestAllocator *alloc)
>> g_source_unref(source);
>>
>> qtest_quit(to);
>> +
>> + cleanup:
>> test_server_free(dest);
>> g_free(uri);
>> +g_string_free(dest_cmdline, true);
>> }
>>
>> static void wait_for_rings_started(TestServer *s, size_t count)
>>
>
> Reviewed-by: Thomas Huth
>
> ... and picked up to my qtest-next tree.
... 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?
Thomas
Re: [PATCH V2] vhost-user-test: fix a memory leak
On 20/12/2019 02.26, pannengy...@huawei.com wrote:
> From: Pan Nengyuan
>
> Spotted by ASAN.
>
> Reported-by: Euler Robot
> Signed-off-by: Pan Nengyuan
> ---
> Changes V2 to V1:
> - use a "goto cleanup", instead of duplicating the "free" functions.
> - free "dest_cmdline" at the end.
> ---
> tests/vhost-user-test.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
> index 91ea373..dcb8617 100644
> --- a/tests/vhost-user-test.c
> +++ b/tests/vhost-user-test.c
> @@ -717,7 +717,7 @@ static void test_migrate(void *obj, void *arg,
> QGuestAllocator *alloc)
> guint64 size;
>
> if (!wait_for_fds(s)) {
> -return;
> +goto cleanup;
> }
>
> size = get_log_size(s);
> @@ -776,8 +776,11 @@ static void test_migrate(void *obj, void *arg,
> QGuestAllocator *alloc)
> g_source_unref(source);
>
> qtest_quit(to);
> +
> + cleanup:
> test_server_free(dest);
> g_free(uri);
> +g_string_free(dest_cmdline, true);
> }
>
> static void wait_for_rings_started(TestServer *s, size_t count)
>
Reviewed-by: Thomas Huth
... and picked up to my qtest-next tree.
[PATCH V2] vhost-user-test: fix a memory leak
From: Pan Nengyuan
Spotted by ASAN.
Reported-by: Euler Robot
Signed-off-by: Pan Nengyuan
---
Changes V2 to V1:
- use a "goto cleanup", instead of duplicating the "free" functions.
- free "dest_cmdline" at the end.
---
tests/vhost-user-test.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 91ea373..dcb8617 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -717,7 +717,7 @@ static void test_migrate(void *obj, void *arg,
QGuestAllocator *alloc)
guint64 size;
if (!wait_for_fds(s)) {
-return;
+goto cleanup;
}
size = get_log_size(s);
@@ -776,8 +776,11 @@ static void test_migrate(void *obj, void *arg,
QGuestAllocator *alloc)
g_source_unref(source);
qtest_quit(to);
+
+ cleanup:
test_server_free(dest);
g_free(uri);
+g_string_free(dest_cmdline, true);
}
static void wait_for_rings_started(TestServer *s, size_t count)
--
2.7.2.windows.1
