[Qemu-devel] [PATCH] Remove ill-advised fallback when we cannot allocate from -mem-path

[Qemu-devel] [PATCH] Remove ill-advised fallback when we cannot allocate from -mem-path

[David Gibson]

When the -mem-path option is specified we try to allocate memory for guest
RAM from mmap()ing files in the given path rather than from anonymous
memory as usual.  If we are unable to allocate memory from the path, we
fall back to anonymous memory, with a rather cryptic error message.

This fallback is a bad idea - if the user specified a backing path, they
did so for a reason and falling back automatically could lead to confusing
results.

Under Book3S-HV KVM on PowerPC the failure is particularly bad, because the
page size of the backing file limits what page sizes can be supported in the
guest.  We have code to automatically advertise the available page sizes to
the guest, but this will give the wrong information in the case of a fall
back to anonymous memory, because by the time we're producing that infor
there's no easy way to determine that such a fallback occurred.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 exec.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/exec.c b/exec.c
index feb4795..544a090 100644
--- a/exec.c
+++ b/exec.c
@@ -2425,7 +2425,6 @@ static void *file_ram_alloc(RAMBlock *block,
     area = mmap(0, memory, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
 #endif
     if (area == MAP_FAILED) {
-        perror("file_ram_alloc: can't mmap RAM pages");
         close(fd);
         return (NULL);
     }
@@ -2528,8 +2527,9 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
 #if defined (__linux__) && !defined(TARGET_S390X)
             new_block->host = file_ram_alloc(new_block, size, mem_path);
             if (!new_block->host) {
-                new_block->host = qemu_vmalloc(size);
-                qemu_madvise(new_block->host, size, QEMU_MADV_MERGEABLE);
+                fprintf(stderr, "Unable to map %llu bytes of RAM from requested"
+                        " backing path %s\n", (unsigned long long)size, mem_path);
+                exit(1);
             }
 #else
             fprintf(stderr, "-mem-path option unsupported\n");
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH] Remove ill-advised fallback when we cannot allocate from -mem-path

[David Gibson]

Marcelo,

Anthony pointed out that you explicitly added this fallback in
618a568da4eda5c2e41fc4e58059546806afff6b.  I'd like to know your
reasoning as to why - as noted below this causes problems for us, and
a situation where the fallback is desirable is not clear to me.

On Wed, Aug 01, 2012 at 02:41:05PM +1000, David Gibson wrote:
> When the -mem-path option is specified we try to allocate memory for guest
> RAM from mmap()ing files in the given path rather than from anonymous
> memory as usual.  If we are unable to allocate memory from the path, we
> fall back to anonymous memory, with a rather cryptic error message.
> 
> This fallback is a bad idea - if the user specified a backing path, they
> did so for a reason and falling back automatically could lead to confusing
> results.
> 
> Under Book3S-HV KVM on PowerPC the failure is particularly bad, because the
> page size of the backing file limits what page sizes can be supported in the
> guest.  We have code to automatically advertise the available page sizes to
> the guest, but this will give the wrong information in the case of a fall
> back to anonymous memory, because by the time we're producing that infor
> there's no easy way to determine that such a fallback occurred.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  exec.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index feb4795..544a090 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2425,7 +2425,6 @@ static void *file_ram_alloc(RAMBlock *block,
>      area = mmap(0, memory, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
>  #endif
>      if (area == MAP_FAILED) {
> -        perror("file_ram_alloc: can't mmap RAM pages");
>          close(fd);
>          return (NULL);
>      }
> @@ -2528,8 +2527,9 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
>  #if defined (__linux__) && !defined(TARGET_S390X)
>              new_block->host = file_ram_alloc(new_block, size, mem_path);
>              if (!new_block->host) {
> -                new_block->host = qemu_vmalloc(size);
> -                qemu_madvise(new_block->host, size, QEMU_MADV_MERGEABLE);
> +                fprintf(stderr, "Unable to map %llu bytes of RAM from requested"
> +                        " backing path %s\n", (unsigned long long)size, mem_path);
> +                exit(1);
>              }
>  #else
>              fprintf(stderr, "-mem-path option unsupported\n");

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson