We are interested by the coredump of the child, not the qtest
parent. If the child generated a coredump, simply call
exit(EXIT_FAILURE) in the parent to avoid overwriting the
child coredump.

Fixes: 71a268a5fd ("tests/libqtest: Improve kill_qemu()")
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/qtest/libqtest.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 49075b55a1..bd85d01145 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -173,7 +173,12 @@ static void kill_qemu(QTestState *s)
         fprintf(stderr, "%s:%d: kill_qemu() detected QEMU death "
                 "from signal %d (%s)%s\n",
                 __FILE__, __LINE__, sig, signame, dump);
-        abort();
+        if (WCOREDUMP(wstatus)) {
+            /* Preserve child coredump */
+            exit(1);
+        } else {
+            abort();
+        }
     }
 }
 
-- 
2.21.3

On 07/07/2020 05.19, Philippe Mathieu-Daudé wrote:
> We are interested by the coredump of the child, not the qtest
> parent. If the child generated a coredump, simply call
> exit(EXIT_FAILURE) in the parent to avoid overwriting the
> child coredump.
> 
> Fixes: 71a268a5fd ("tests/libqtest: Improve kill_qemu()")
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  tests/qtest/libqtest.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 49075b55a1..bd85d01145 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -173,7 +173,12 @@ static void kill_qemu(QTestState *s)
>          fprintf(stderr, "%s:%d: kill_qemu() detected QEMU death "
>                  "from signal %d (%s)%s\n",
>                  __FILE__, __LINE__, sig, signame, dump);
> -        abort();
> +        if (WCOREDUMP(wstatus)) {
> +            /* Preserve child coredump */
> +            exit(1);
> +        } else {
> +            abort();
> +        }
>      }
>  }

Would it maybe rather make sense to always use exit(1) unconditionally here?

 Thomas

On 07/07/20 11:03, Thomas Huth wrote:
>> +++ b/tests/qtest/libqtest.c
>> @@ -173,7 +173,12 @@ static void kill_qemu(QTestState *s)
>>          fprintf(stderr, "%s:%d: kill_qemu() detected QEMU death "
>>                  "from signal %d (%s)%s\n",
>>                  __FILE__, __LINE__, sig, signame, dump);
>> -        abort();
>> +        if (WCOREDUMP(wstatus)) {
>> +            /* Preserve child coredump */
>> +            exit(1);
>> +        } else {
>> +            abort();
>> +        }
>>      }
>>  }
> Would it maybe rather make sense to always use exit(1) unconditionally here?

But why is it a problem to overwrite the child core dump?  Aren't both
stashed away if you use the core.PID name as is common?

Paolo

On 7/7/20 11:29 AM, Paolo Bonzini wrote:
> On 07/07/20 11:03, Thomas Huth wrote:
>>> +++ b/tests/qtest/libqtest.c
>>> @@ -173,7 +173,12 @@ static void kill_qemu(QTestState *s)
>>>          fprintf(stderr, "%s:%d: kill_qemu() detected QEMU death "
>>>                  "from signal %d (%s)%s\n",
>>>                  __FILE__, __LINE__, sig, signame, dump);
>>> -        abort();
>>> +        if (WCOREDUMP(wstatus)) {
>>> +            /* Preserve child coredump */
>>> +            exit(1);
>>> +        } else {
>>> +            abort();
>>> +        }
>>>      }
>>>  }
>> Would it maybe rather make sense to always use exit(1) unconditionally here?
> 
> But why is it a problem to overwrite the child core dump?  Aren't both
> stashed away if you use the core.PID name as is common?

I'm not sure what you mean. Without this patch, the coredump I get
is qtest parent, coredumpctl list the child but the coredump is
unavailable. With this patch I don't get the uninteresting (for my
uses) qtest parent but the child coredump. I'm using Fedora 30,
I don't remember changing the coredumpctl default config. Travis-CI
is based on Ubuntu.

> 
> Paolo
> 
>