Re: [Qemu-devel] [PATCH v3 for 1.6 7/8] exec: Don't abort when we can't allocate guest memory

[Andreas Färber]

Am 31.07.2013 15:11, schrieb Markus Armbruster:
> We abort() on memory allocation failure.  abort() is appropriate for
> programming errors.  Maybe most memory allocation failures are
> programming errors, maybe not.  But guest memory allocation failure
> isn't, and aborting when the user asks for more memory than we can
> provide is not nice.  exit(1) instead, and do it in just one place, so
> the error message is consistent.
> 
> Tested-by: Christian Borntraeger <address@hidden>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  exec.c             | 5 +++++
>  target-s390x/kvm.c | 6 +-----
>  util/oslib-posix.c | 4 +---
>  util/oslib-win32.c | 5 +----
>  4 files changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 231d04e..0cfca3a 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1162,6 +1162,11 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, 
> void *host,
>          }
>          if (!new_block->host) {
>              new_block->host = phys_mem_alloc(size);
> +            if (!new_block->host) {
> +                fprintf(stderr, "Cannot set up guest memory '%s': %s\n",
> +                        new_block->mr->name, strerror(errno));

This could use error_report() while at it, but still

Reviewed-by: Andreas Färber <address@hidden>

Thought I had ack'ed it long ago, but I guess something minor changed.

Cheers,
Andreas

> +                exit(1);
> +            }
>              memory_try_enable_merging(new_block->host, size);
>          }
>      }
> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> index b5351e6..c4a0fdd 100644
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -331,11 +331,7 @@ static void *legacy_s390_alloc(ram_addr_t size)
>      mem = mmap((void *) 0x800000000ULL, size,
>                 PROT_EXEC|PROT_READ|PROT_WRITE,
>                 MAP_SHARED | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
> -    if (mem == MAP_FAILED) {
> -        fprintf(stderr, "Allocating RAM failed\n");
> -        abort();
> -    }
> -    return mem;
> +    return mem == MAP_FAILED ? NULL : mem;
>  }
>  
>  int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 3dc8b1b..253bc3d 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -112,9 +112,7 @@ void *qemu_anon_ram_alloc(size_t size)
>      size_t offset = QEMU_ALIGN_UP((uintptr_t)ptr, align) - (uintptr_t)ptr;
>  
>      if (ptr == MAP_FAILED) {
> -        fprintf(stderr, "Failed to allocate %zu B: %s\n",
> -                size, strerror(errno));
> -        abort();
> +        return NULL;
>      }
>  
>      ptr += offset;
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index 961fbf5..983b7a2 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -65,10 +65,7 @@ void *qemu_anon_ram_alloc(size_t size)
>      /* FIXME: this is not exactly optimal solution since VirtualAlloc
>         has 64Kb granularity, but at least it guarantees us that the
>         memory is page aligned. */
> -    if (!size) {
> -        abort();
> -    }
> -    ptr = qemu_oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, 
> PAGE_READWRITE));
> +    ptr = VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE);
>      trace_qemu_anon_ram_alloc(size, ptr);
>      return ptr;
>  }
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg