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 <address@hidden>
> ---
>  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