| Message ID | 1328884453-1067-1-git-send-email-zwu.kernel@gmail.com |
|---|---|
| State | New |
| Headers | show |
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 The only option other than abort(), is to use _Exit() which doesn't try to run cleanup handlers. Daniel
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 > > Daniel > -- > |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| > |: http://libvirt.org -o- http://virt-manager.org :| > |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| > |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
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.
On 02/10/2012 07:41 AM, Daniel P. Berrange wrote: >> @@ -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 > > The only option other than abort(), is to use _Exit() which > doesn't try to run cleanup handlers. Correct, but in that case, then you need to fflush(stderr) prior to _Exit(), or else use write() rather than fprintf(), since otherwise your attempt at a nice oom error message is lost.
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. >
On Sat, Feb 11, 2012 at 2:35 AM, Eric Blake <eblake@redhat.com> wrote: > On 02/10/2012 07:41 AM, Daniel P. Berrange wrote: > >>> @@ -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 >> >> The only option other than abort(), is to use _Exit() which >> doesn't try to run cleanup handlers. > > Correct, but in that case, then you need to fflush(stderr) prior to > _Exit(), or else use write() rather than fprintf(), since otherwise your > attempt at a nice oom error message is lost. Great, pls see next version. > > -- > Eric Blake eblake@redhat.com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org >
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. 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
On Fri, Feb 10, 2012 at 11:35:11AM -0700, Eric Blake wrote: > On 02/10/2012 07:41 AM, Daniel P. Berrange wrote: > > >> @@ -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 > > > > The only option other than abort(), is to use _Exit() which > > doesn't try to run cleanup handlers. > > Correct, but in that case, then you need to fflush(stderr) prior to > _Exit(), or else use write() rather than fprintf(), since otherwise your > attempt at a nice oom error message is lost. IIRC, stderr is not buffered, so should not need to be flushed. Daniel
On Mon, Feb 13, 2012 at 6:29 AM, Stefan Weil <sw@weilnetz.de> 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. > > 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
Stefan Weil <sw@weilnetz.de> writes: > 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. I consider abort() on OOM somewhat eccentric. abort() is for programming errors. Resource shortage is an environmental error that is sometimes (but not always) caused by a programming error. I'd rather inconvenience programmers (by making it a little bit harder to debug programming errors that cause OOM) than confuse users with inappropriate scary "crashes".
On 13 February 2012 14:04, Markus Armbruster <armbru@redhat.com> wrote: > I consider abort() on OOM somewhat eccentric. abort() is for > programming errors. Resource shortage is an environmental error that is > sometimes (but not always) caused by a programming error. > > I'd rather inconvenience programmers (by making it a little bit harder > to debug programming errors that cause OOM) than confuse users with > inappropriate scary "crashes". I think the rationale for aborting here is that you're already accepting "program just dies" behaviour for out-of-memory errors via the kernel's OOM-killer... -- PMM
> > abort can create core dumps or start a debugger which is > > useful for me and maybe other developers, too. > > I consider abort() on OOM somewhat eccentric. abort() is for > programming errors. Resource shortage is an environmental error that is > sometimes (but not always) caused by a programming error. > > I'd rather inconvenience programmers (by making it a little bit harder > to debug programming errors that cause OOM) than confuse users with > inappropriate scary "crashes". While I agree that abort() is not the most friendly failure method, I don't tthink it's worth trying to handle OOM gracefully. Once we hit OOM I'd say we're pretty much beyond hope. The best thing we can do is exist as quickly as possible. For the vast majority of systems there isn't any reason to believe things will somehow get better if we try again later. Initial guest RAM allocation is maybe a special case worth a polite error. OTOH if you're near the limit then there's a fair chance the -m allocation will succeed, but some later allocation will not. The only way to handle this rebustly is to pre-allocate all the memory we're ever going to need[1]. I don't see that happening. Paul [1] And make sure the kernel isn't lying about how much ram we can have.
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 > >
On 02/13/2012 05:16 AM, Stefan Hajnoczi wrote: > On Mon, Feb 13, 2012 at 6:29 AM, Stefan Weil<sw@weilnetz.de> wrote: >> 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. g_try_malloc glib already has a suite of functions for this. Regards, Anthony Liguori > > Stefan >
On Tue, Feb 14, 2012 at 12:42:58PM +0000, Paul Brook wrote: > > > abort can create core dumps or start a debugger which is > > > useful for me and maybe other developers, too. > > > > I consider abort() on OOM somewhat eccentric. abort() is for > > programming errors. Resource shortage is an environmental error that is > > sometimes (but not always) caused by a programming error. > > > > I'd rather inconvenience programmers (by making it a little bit harder > > to debug programming errors that cause OOM) than confuse users with > > inappropriate scary "crashes". > > While I agree that abort() is not the most friendly failure method, I don't > tthink it's worth trying to handle OOM gracefully. Once we hit OOM I'd say > we're pretty much beyond hope. The best thing we can do is exist as quickly > as possible. For the vast majority of systems there isn't any reason to > believe things will somehow get better if we try again later. > > Initial guest RAM allocation is maybe a special case worth a polite error. > OTOH if you're near the limit then there's a fair chance the -m allocation > will succeed, but some later allocation will not. > > The only way to handle this rebustly is to pre-allocate all the memory we're > ever going to need[1]. I don't see that happening. FWIW, users can already opt-in to pre-allocation if running KVM enabled QEMU -mem-path /dev/shm -mem-prealloc (or /dev/hugepages more usefully) Regards, Daniel
On 02/13/2012 08:04 AM, Markus Armbruster wrote: > Stefan Weil<sw@weilnetz.de> writes: > >> 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. > > I consider abort() on OOM somewhat eccentric. abort() is for > programming errors. Resource shortage is an environmental error that is > sometimes (but not always) caused by a programming error. > > I'd rather inconvenience programmers (by making it a little bit harder > to debug programming errors that cause OOM) than confuse users with > inappropriate scary "crashes". OOM is a going to 99% of the time be a bug in QEMU. For the rare exceptions (like a bad -m argument), we should handle those as special cases. Regards, Anthony Liguori >
> > The only way to handle this rebustly is to pre-allocate all the memory > > we're ever going to need[1]. I don't see that happening. > > FWIW, users can already opt-in to pre-allocation if running KVM enabled > QEMU > > -mem-path /dev/shm -mem-prealloc (or /dev/hugepages more usefully) No, that's something different. -mem-prealloc causes MAP_POPULATE to be passed when allocating guest ram, working around the fact that most modern implementations of mmap lie. It has no effect on how all the other memory qemu uses is allocated. Paul
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); } return ptr; } @@ -94,7 +94,7 @@ void *qemu_memalign(size_t alignment, size_t size) if (ret != 0) { fprintf(stderr, "Failed to allocate %zu B: %s\n", size, strerror(ret)); - abort(); + exit(EXIT_FAILURE); } #elif defined(CONFIG_BSD) ptr = qemu_oom_check(valloc(size)); diff --git a/oslib-win32.c b/oslib-win32.c index ce3021e..af2ff93 100644 --- a/oslib-win32.c +++ b/oslib-win32.c @@ -35,7 +35,7 @@ void *qemu_oom_check(void *ptr) { if (ptr == NULL) { fprintf(stderr, "Failed to allocate memory: %lu\n", GetLastError()); - abort(); + exit(EXIT_FAILURE); } return ptr; } @@ -45,7 +45,7 @@ void *qemu_memalign(size_t alignment, size_t size) void *ptr; if (!size) { - abort(); + exit(EXIT_FAILURE); } ptr = qemu_oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE)); trace_qemu_memalign(alignment, size, ptr);