Re: [Qemu-devel] [PATCH RFC 4/4] add -keep-mem-path-files option

[Blue Swirl]

On Tue, Jun 26, 2012 at 8:51 PM, Eduardo Habkost <address@hidden> wrote:
> This make QEMU create files inside the -mem-path directory using
> more predictable names, and not remove them afterwards.
>
> This allow (for example) users or management layers to use numactl
> later, to set NUMA policy for the guest RAM.
>
> Signed-off-by: Eduardo Habkost <address@hidden>
> ---
>  cpu-all.h       |    1 +
>  exec.c          |   27 ++++++++++++++++++++++++++-
>  qemu-options.hx |   12 ++++++++++++
>  vl.c            |    5 +++++
>  4 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/cpu-all.h b/cpu-all.h
> index 2beed5a..acd59ff 100644
> --- a/cpu-all.h
> +++ b/cpu-all.h
> @@ -490,6 +490,7 @@ typedef struct RAMList {
>  extern RAMList ram_list;
>
>  extern const char *mem_path;
> +extern bool keep_mem_path_files;
>  extern bool mem_prealloc;
>
>  /* Flags stored in the low bits of the TLB virtual address.  These are
> diff --git a/exec.c b/exec.c
> index dcbe4e1..504e19f 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2377,6 +2377,28 @@ static int get_temp_fd(const char *path)
>     return fd;
>  }
>
> +/* Return FD to RAM block file, using the memory region name as filename
> + */
> +static int open_ramblock_file(RAMBlock *block, const char *path)
> +{
> +    int fd;
> +    char *filename;
> +
> +    if (asprintf(&filename, "%s/%s", path, block->mr->name) == -1) {

asprintf() uses plain malloc() which is not tracked by our tracing
system. Please use g_malloc() and snprintf() or maybe
g_string_printf().

> +        return -1;
> +    }
> +
> +    fd = open(filename, O_RDWR|O_CREAT);
> +    if (fd < 0) {
> +        perror("unable to open backing store for hugepages");
> +        free(filename);

This needs to be converted to g_free() then.

> +        return -1;
> +    }
> +    free(filename);

Ditto.

> +
> +    return fd;
> +}
> +
>  static void *file_ram_alloc(RAMBlock *block,
>                             size_t length,
>                             const char *path)
> @@ -2402,7 +2424,10 @@ static void *file_ram_alloc(RAMBlock *block,
>         return NULL;
>     }
>
> -    fd = get_temp_fd(path);
> +    if (keep_mem_path_files)

Missing braces needed by our CODING_STYLE.

> +        fd = open_ramblock_file(block, path);
> +    else
> +        fd = get_temp_fd(path);
>     if (fd < 0) {
>         return NULL;
>     }
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 8b66264..f2eb237 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -399,6 +399,18 @@ STEXI
>  Allocate guest RAM from a temporarily created file in @var{path}.
>  ETEXI
>
> +DEF("keep-mem-path-files", HAS_ARG, QEMU_OPTION_keep_mempath_files,
> +    "-keep-mem-path-files  Keep files created in -mem-path\n", QEMU_ARCH_ALL)
> +STEXI
> address@hidden -keep-mem-path-files
> +Create the files for -mem-path using the memory region names, and don't 
> remove
> +them afterwards.
> +
> +This allows further fine-tuning of NUMA policy for memory regions using
> +numactl.
> +ETEXI
> +
> +
>  #ifdef MAP_POPULATE
>  DEF("mem-prealloc", 0, QEMU_OPTION_mem_prealloc,
>     "-mem-prealloc   preallocate guest memory (use with -mem-path)\n",
> diff --git a/vl.c b/vl.c
> index 4e3403c..1dc595f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -177,6 +177,8 @@ int display_remote = 0;
>  const char* keyboard_layout = NULL;
>  ram_addr_t ram_size;
>  const char *mem_path = NULL;
> +bool keep_mem_path_files = false; /* Keep files created at mem_path.
> +                                   * use memory region names as filename */
>  #ifdef MAP_POPULATE
>  bool mem_prealloc = false; /* force preallocation of physical target memory 
> */
>  #endif
> @@ -2671,6 +2673,9 @@ int main(int argc, char **argv, char **envp)
>             case QEMU_OPTION_mempath:
>                 mem_path = optarg;
>                 break;
> +            case QEMU_OPTION_keep_mempath_files:
> +                keep_mem_path_files = true;
> +                break;
>  #ifdef MAP_POPULATE
>             case QEMU_OPTION_mem_prealloc:
>                 mem_prealloc = true;
> --
> 1.7.10.4
>
>