Re: [Qemu-devel] [PATCH v3 3/9] exec: split qemu_ram_alloc_from_file()

[Eduardo Habkost]

On Fri, Jun 02, 2017 at 06:12:23PM +0400, Marc-André Lureau wrote:
> Add qemu_ram_alloc_from_fd(), which can be use to allocate ramblock from
> fd only.
> 
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  include/exec/ram_addr.h |  3 +++
>  exec.c                  | 45 ++++++++++++++++++++++++++++++---------------
>  2 files changed, 33 insertions(+), 15 deletions(-)
> 
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index 140efa840c..73d1bea8b6 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -65,6 +65,9 @@ unsigned long last_ram_page(void);
>  RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
>                                     bool share, const char *mem_path,
>                                     Error **errp);
> +RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr,
> +                                 bool share, int fd,
> +                                 Error **errp);
>  RAMBlock *qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
>                                    MemoryRegion *mr, Error **errp);
>  RAMBlock *qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr, Error **errp);
> diff --git a/exec.c b/exec.c
> index 97e01d75f0..8cd3153450 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1915,14 +1915,12 @@ static void ram_block_add(RAMBlock *new_block, Error 
> **errp)
>  }
>  
>  #ifdef __linux__
> -RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
> -                                   bool share, const char *mem_path,
> -                                   Error **errp)
> +RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr,
> +                                 bool share, int fd,
> +                                 Error **errp)
>  {
>      RAMBlock *new_block;
>      Error *local_err = NULL;
> -    int fd;
> -    bool created;
>      int64_t file_size;
>  
>      if (xen_enabled()) {
> @@ -1947,18 +1945,12 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, 
> MemoryRegion *mr,
>          return NULL;
>      }
>  
> -    fd = file_ram_open(mem_path, memory_region_name(mr), &created, errp);
> -    if (fd < 0) {
> -        return NULL;
> -    }
> -

I like this, because it gets us one step closer to moving the
file creation/opening logic in file_ram_open() to hostmem-file.c.
Then both memfd and hostmem-file could use
memory_region_init_ram_from_fd(), and we won't need
memory_region_init_ram_from_file() anymore.


>      size = HOST_PAGE_ALIGN(size);
>      file_size = get_file_size(fd);
>      if (file_size > 0 && file_size < size) {
>          error_setg(errp, "backing store %s size 0x%" PRIx64
>                     " does not match 'size' option 0x" RAM_ADDR_FMT,
>                     mem_path, file_size, size);
> -        close(fd);
>          return NULL;
>      }
>  
> @@ -1969,10 +1961,6 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, 
> MemoryRegion *mr,
>      new_block->flags = share ? RAM_SHARED : 0;
>      new_block->host = file_ram_alloc(new_block, size, fd, !file_size, errp);
>      if (!new_block->host) {
> -        if (created) {
> -            unlink(mem_path);
> -        }
> -        close(fd);
>          g_free(new_block);
>          return NULL;
>      }
> @@ -1984,6 +1972,33 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, 
> MemoryRegion *mr,
>          return NULL;
>      }
>      return new_block;
> +
> +}
> +
> +
> +RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
> +                                   bool share, const char *mem_path,
> +                                   Error **errp)
> +{
> +    int fd;
> +    bool created;
> +    RAMBlock *block;
> +
> +    fd = file_ram_open(mem_path, memory_region_name(mr), &created, errp);
> +    if (fd < 0) {
> +        return NULL;
> +    }
> +
> +    block = qemu_ram_alloc_from_fd(size, mr, share, fd, errp);
> +    if (!block) {
> +        if (created) {
> +            unlink(mem_path);
> +        }
> +        close(fd);
> +        return NULL;
> +    }
> +
> +    return block;
>  }
>  #endif
>  
> -- 
> 2.13.0.91.g00982b8dd
> 

-- 
Eduardo