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

[Anthony Liguori <anthony@codemonkey.ws>]

On 02/13/2012 12:29 AM, Stefan Weil wrote:
> Am 13.02.2012 03:37, schrieb Zhi Yong Wu:
>> On Fri, Feb 10, 2012 at 11:53 PM, Stefan Weil <sw@weilnetz.de> wrote:
>>> Am 10.02.2012 16:13, schrieb Zhi Yong Wu:
>>>
>>>> On Fri, Feb 10, 2012 at 10:41 PM, Daniel P. Berrange
>>>> <berrange@redhat.com> wrote:
>>>>>
>>>>> On Fri, Feb 10, 2012 at 10:34:13PM +0800, Zhi Yong Wu wrote:
>>>>>>
>>>>>> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>>>>>>
>>>>>> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>>>>>> ---
>>>>>> 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.

Then use g_try_malloc() when allocating ram and give a nice error message.

Normal malloc failures should call abort().

Regards,

Anthony Liguori

>
> 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.
>
> I therefore suggest to restrict any modification to the handling
> of -m. In that case you could even improve the error message by
> telling the user how much memory would be possible.
> Simply call the allocating function with decreasing values until
> it no longer fails.
>
> Regards,
> Stefan Weil
>
>