qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] backends/hostmem-file: Add file-name property


From: Igor Mammedov
Subject: Re: [Qemu-devel] [PATCH] backends/hostmem-file: Add file-name property
Date: Tue, 27 Oct 2015 15:58:02 +0100

On Tue, 27 Oct 2015 16:53:56 +0300
Pavel Fedin <address@hidden> wrote:

> This option allows to explicitly specify file name to use with the backend.
> This is important when using it together with ivshmem in order to make it
> backed by hugetlbfs. By default filename is autogenerated using mkstemp(),
> and the file is unlink()ed after creation, effectively making it
> anonymous. This is not very useful with ivshmem because it ends up in a
> memory which cannot be accessed by something else.
How about reusing mem-path property instead of adding a new one?
if mem-path is directory use current behavior,
if it's file just use that file.
Also that would make patch less intrusive.

> 
> Signed-off-by: Pavel Fedin <address@hidden>
> Tested-by: Igor Skalkin <address@hidden>
> ---
>  backends/hostmem-file.c | 26 +++++++++++++++++++++++++-
>  exec.c                  | 36 ++++++++++++++++++++++++------------
>  include/exec/memory.h   |  3 +++
>  include/exec/ram_addr.h |  2 +-
>  memory.c                |  4 +++-
>  numa.c                  |  2 +-
>  qemu-doc.texi           |  2 +-
>  7 files changed, 58 insertions(+), 17 deletions(-)
> 
> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> index e9b6d21..557eaf1 100644
> --- a/backends/hostmem-file.c
> +++ b/backends/hostmem-file.c
> @@ -31,6 +31,7 @@ struct HostMemoryBackendFile {
>  
>      bool share;
>      char *mem_path;
> +    char *file_name;
>  };
>  
>  static void
> @@ -54,7 +55,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error 
> **errp)
>          memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
>                                   object_get_canonical_path(OBJECT(backend)),
>                                   backend->size, fb->share,
> -                                 fb->mem_path, errp);
> +                                 fb->mem_path, fb->file_name, errp);
>      }
>  #endif
>  }
> @@ -83,10 +84,31 @@ static void set_mem_path(Object *o, const char *str, 
> Error **errp)
>          error_setg(errp, "cannot change property value");
>          return;
>      }
> +
>      g_free(fb->mem_path);
>      fb->mem_path = g_strdup(str);
>  }
>  
> +static char *get_file_name(Object *o, Error **errp)
> +{
> +    HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
> +
> +    return g_strdup(fb->file_name);
> +}
> +
> +static void set_file_name(Object *o, const char *str, Error **errp)
> +{
> +    HostMemoryBackend *backend = MEMORY_BACKEND(o);
> +    HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
> +
> +    if (memory_region_size(&backend->mr)) {
> +        error_setg(errp, "cannot change property value");
> +        return;
> +    }
> +    g_free(fb->file_name);
> +    fb->file_name = g_strdup(str);
> +}
> +
>  static bool file_memory_backend_get_share(Object *o, Error **errp)
>  {
>      HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
> @@ -114,6 +136,8 @@ file_backend_instance_init(Object *o)
>                          file_memory_backend_set_share, NULL);
>      object_property_add_str(o, "mem-path", get_mem_path,
>                              set_mem_path, NULL);
> +    object_property_add_str(o, "file-name", get_file_name,
> +                            set_file_name, NULL);
>  }
>  
>  static const TypeInfo file_backend_info = {
> diff --git a/exec.c b/exec.c
> index 8af2570..6cf1a36 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1203,6 +1203,7 @@ static long gethugepagesize(const char *path, Error 
> **errp)
>  static void *file_ram_alloc(RAMBlock *block,
>                              ram_addr_t memory,
>                              const char *path,
> +                            const char *name,
>                              Error **errp)
>  {
>      char *filename;
> @@ -1240,18 +1241,29 @@ static void *file_ram_alloc(RAMBlock *block,
>              *c = '_';
>      }
>  
> -    filename = g_strdup_printf("%s/qemu_back_mem.%s.XXXXXX", path,
> -                               sanitized_name);
> -    g_free(sanitized_name);
> +    if (name) {
> +        filename = g_strdup_printf("%s/%s", path, name);
> +        fd = open(filename, O_RDWR | O_CREAT, 0644);
> +        if (fd < 0) {
> +            error_setg_errno(errp, errno,
> +                             "unable to open backing store for hugepages");
> +            g_free(filename);
> +            goto error;
> +        }
> +    } else {
> +        filename = g_strdup_printf("%s/qemu_back_mem.%s.XXXXXX", path,
> +                                   sanitized_name);
> +        g_free(sanitized_name);
>  
> -    fd = mkstemp(filename);
> -    if (fd < 0) {
> -        error_setg_errno(errp, errno,
> -                         "unable to create backing store for hugepages");
> -        g_free(filename);
> -        goto error;
> +        fd = mkstemp(filename);
> +        if (fd < 0) {
> +            error_setg_errno(errp, errno,
> +                             "unable to create backing store for hugepages");
> +            g_free(filename);
> +            goto error;
> +        }
> +        unlink(filename);
>      }
> -    unlink(filename);
>      g_free(filename);
>  
>      memory = ROUND_UP(memory, hpagesize);
> @@ -1563,7 +1575,7 @@ static ram_addr_t ram_block_add(RAMBlock *new_block, 
> Error **errp)
>  #ifdef __linux__
>  ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
>                                      bool share, const char *mem_path,
> -                                    Error **errp)
> +                                    const char *file_name, Error **errp)
>  {
>      RAMBlock *new_block;
>      ram_addr_t addr;
> @@ -1593,7 +1605,7 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, 
> MemoryRegion *mr,
>      new_block->flags = share ? RAM_SHARED : 0;
>      new_block->flags |= RAM_FILE;
>      new_block->host = file_ram_alloc(new_block, size,
> -                                     mem_path, errp);
> +                                     mem_path, file_name, errp);
>      if (!new_block->host) {
>          g_free(new_block);
>          return -1;
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 0f07159..58f56e8 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -386,6 +386,8 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr,
>   * @size: size of the region.
>   * @share: %true if memory must be mmaped with the MAP_SHARED flag
>   * @path: the path in which to allocate the RAM.
> + * @filename: name of the backing file or NULL in order to use unique
> + *            temporary file
>   * @errp: pointer to Error*, to store an error if it happens.
>   */
>  void memory_region_init_ram_from_file(MemoryRegion *mr,
> @@ -394,6 +396,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
>                                        uint64_t size,
>                                        bool share,
>                                        const char *path,
> +                                      const char *filename,
>                                        Error **errp);
>  #endif
>  
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index 3360ac5..6d27c07 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -64,7 +64,7 @@ void qemu_mutex_unlock_ramlist(void);
>  
>  ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
>                                      bool share, const char *mem_path,
> -                                    Error **errp);
> +                                    const char *file_name, Error **errp);
>  ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
>                                     MemoryRegion *mr, Error **errp);
>  ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr, Error **errp);
> diff --git a/memory.c b/memory.c
> index 2eb1597..6cb8588 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1226,13 +1226,15 @@ void memory_region_init_ram_from_file(MemoryRegion 
> *mr,
>                                        uint64_t size,
>                                        bool share,
>                                        const char *path,
> +                                      const char *filename,
>                                        Error **errp)
>  {
>      memory_region_init(mr, owner, name, size);
>      mr->ram = true;
>      mr->terminates = true;
>      mr->destructor = memory_region_destructor_ram;
> -    mr->ram_addr = qemu_ram_alloc_from_file(size, mr, share, path, errp);
> +    mr->ram_addr = qemu_ram_alloc_from_file(size, mr, share, path, filename,
> +                                            errp);
>      mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
>  }
>  #endif
> diff --git a/numa.c b/numa.c
> index e9b18f5..93e3939 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -417,7 +417,7 @@ static void allocate_system_memory_nonnuma(MemoryRegion 
> *mr, Object *owner,
>  #ifdef __linux__
>          Error *err = NULL;
>          memory_region_init_ram_from_file(mr, owner, name, ram_size, false,
> -                                         mem_path, &err);
> +                                         mem_path, NULL, &err);
>  
>          /* Legacy behavior: if allocation failed, fall back to
>           * regular RAM allocation.
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index 3126abd..5e5d77b 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -1299,7 +1299,7 @@ Instead of specifying the <shm size> using POSIX shm, 
> you may specify
>  a memory backend that has hugepage support:
>  
>  @example
> -qemu-system-i386 -object 
> memory-backend-file,size=1G,mem-path=/mnt/hugepages,id=mb1
> +qemu-system-i386 -object 
> memory-backend-file,size=1G,mem-path=/mnt/hugepages,file-name=my_ivshmem,share=on,id=mb1
>                   -device ivshmem,memdev=mb1
>  @end example
>  




reply via email to

[Prev in Thread] Current Thread [Next in Thread]