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

[Anthony Liguori]

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 <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.

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