Re: [Qemu-devel] [PATCH] oslib: make error handling more reasonable

[Stefan Hajnoczi]

On Mon, Feb 13, 2012 at 6:29 AM, Stefan Weil <address@hidden> wrote:
> Am 13.02.2012 03:37, schrieb Zhi Yong Wu:
>
>> On Fri, Feb 10, 2012 at 11:53 PM, Stefan Weil <address@hidden> wrote:
>>>
>>> Am 10.02.2012 16:13, schrieb Zhi Yong Wu:
>>>
>>>> On Fri, Feb 10, 2012 at 10:41 PM, Daniel P. Berrange
>>>> <address@hidden> wrote:
>>>>>
>>>>>
>>>>> On Fri, Feb 10, 2012 at 10:34:13PM +0800, Zhi Yong Wu wrote:
>>>>>>
>>>>>>
>>>>>> From: Zhi Yong Wu <address@hidden>
>>>>>>
>>>>>> Signed-off-by: Zhi Yong Wu <address@hidden>
>>>>>> ---
>>>>>>  oslib-posix.c |    4 ++--
>>>>>>  oslib-win32.c |    4 ++--
>>>>>>  2 files changed, 4 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/oslib-posix.c b/oslib-posix.c
>>>>>> index b6a3c7f..f978d56 100644
>>>>>> --- a/oslib-posix.c
>>>>>> +++ b/oslib-posix.c
>>>>>> @@ -80,7 +80,7 @@ void *qemu_oom_check(void *ptr)
>>>>>>  {
>>>>>>    if (ptr == NULL) {
>>>>>>        fprintf(stderr, "Failed to allocate memory: %s\n",
>>>>>> strerror(errno));
>>>>>> -        abort();
>>>>>> +        exit(EXIT_FAILURE);
>>>>>
>>>>>
>>>>>
>>>>> exit() will call any atexit()/on_exit() handlers, as well as trying
>>>>> to flush I/O streams. Any of these actions may require further
>>>>> memory allocations, which will likely fail, or worse cause this
>>>>> code to re-enter itself if an atexit() handler calls qemu_malloc
>>>>
>>>>
>>>> Nice, very reasonable.
>>>>>
>>>>>
>>>>>
>>>>> The only option other than abort(), is to use  _Exit() which
>>>>> doesn't try to run cleanup handlers.
>>>>
>>>>
>>>> I will try to send out v2
>>>
>>>
>>>
>>> Could you please explain why calling exit, _Exit or _exit is more
>>> reasonable than calling abort?
>>>
>>> abort can create core dumps or start a debugger which is
>>> useful for me and maybe other developers, too.
>>
>> pls refer to
>> http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg01270.html.
>> In the scenario, the user should not see core dump, and he perhaps
>> think that one bug exists in qemu code.
>> So we hope to use _Exit() instead of abort() here.
>
>
> So you say that you don't want a core dump just because the
> user called QEMU with -m 4000 or some other large value.
>
> Allocating RAM for the emulated machine is perhaps the only
> scenario where a core dump is indeed not reasonable. In most
> other cases, out-of-memory is an indication of a QEMU internal
> problem, so a core dump should be written.

Allocating guest memory could fail and we should give a reasonable
error and exit with a failure.  I think this might be the one case
where we *do* want to handle memory allocation NULL return.  In other
words, perhaps we should call memory allocating functions directly
here instead of using the typical QEMU abort-on-failure wrappers.

Stefan