Re: [Qemu-devel] [PATCH v6 09/33] exec: allow file_ram_alloc to work on file

[Michael S. Tsirkin]

On Fri, Oct 30, 2015 at 01:56:03PM +0800, Xiao Guangrong wrote:
> Currently, file_ram_alloc() only works on directory - it creates a file
> under @path and do mmap on it
> 
> This patch tries to allow it to work on file directly, if @path is a
> directory it works as before, otherwise it treats @path as the target
> file then directly allocate memory from it
> 
> Signed-off-by: Xiao Guangrong <address@hidden>
> ---
>  exec.c | 80 
> ++++++++++++++++++++++++++++++++++++++++++------------------------
>  1 file changed, 51 insertions(+), 29 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 3ca7e50..f219010 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1174,14 +1174,60 @@ void qemu_mutex_unlock_ramlist(void)
>  }
>  
>  #ifdef __linux__
> +static bool path_is_dir(const char *path)
> +{
> +    struct stat fs;
> +
> +    return stat(path, &fs) == 0 && S_ISDIR(fs.st_mode);

This means file doesn't exist is treated as a file.
Can't figure out if that's intentional, should
be documented in any case.

> +}
> +
> +static int open_file_path(RAMBlock *block, const char *path, size_t size)
> +{
> +    char *filename;
> +    char *sanitized_name;
> +    char *c;
> +    int fd;
> +
> +    if (!path_is_dir(path)) {
> +        int flags = (block->flags & RAM_SHARED) ? O_RDWR : O_RDONLY;

Why does this make sense?

> +
> +        flags |= O_EXCL;

And why does this makes sense?

> +        return open(path, flags);
> +    }
> +
> +    /* Make name safe to use with mkstemp by replacing '/' with '_'. */
> +    sanitized_name = g_strdup(memory_region_name(block->mr));
> +    for (c = sanitized_name; *c != '\0'; c++) {
> +        if (*c == '/') {
> +            *c = '_';
> +        }
> +    }
> +    filename = g_strdup_printf("%s/qemu_back_mem.%s.XXXXXX", path,
> +                               sanitized_name);
> +    g_free(sanitized_name);
> +    fd = mkstemp(filename);
> +    if (fd >= 0) {
> +        unlink(filename);
> +        /*
> +         * ftruncate is not supported by hugetlbfs in older
> +         * hosts, so don't bother bailing out on errors.
> +         * If anything goes wrong with it under other filesystems,
> +         * mmap will fail.
> +         */
> +        if (ftruncate(fd, size)) {
> +            perror("ftruncate");
> +        }
> +    }
> +    g_free(filename);
> +
> +    return fd;
> +}
> +
>  static void *file_ram_alloc(RAMBlock *block,
>                              ram_addr_t memory,
>                              const char *path,
>                              Error **errp)
>  {
> -    char *filename;
> -    char *sanitized_name;
> -    char *c;
>      void *area;
>      int fd;
>      uint64_t pagesize;
> @@ -1211,38 +1257,14 @@ static void *file_ram_alloc(RAMBlock *block,
>          goto error;
>      }
>  
> -    /* Make name safe to use with mkstemp by replacing '/' with '_'. */
> -    sanitized_name = g_strdup(memory_region_name(block->mr));
> -    for (c = sanitized_name; *c != '\0'; c++) {
> -        if (*c == '/')
> -            *c = '_';
> -    }
> -
> -    filename = g_strdup_printf("%s/qemu_back_mem.%s.XXXXXX", path,
> -                               sanitized_name);
> -    g_free(sanitized_name);
> +    memory = ROUND_UP(memory, pagesize);
>  
> -    fd = mkstemp(filename);
> +    fd = open_file_path(block, path, memory);
>      if (fd < 0) {
>          error_setg_errno(errp, errno,
>                           "unable to create backing store for path %s", path);
> -        g_free(filename);
>          goto error;
>      }
> -    unlink(filename);
> -    g_free(filename);
> -
> -    memory = ROUND_UP(memory, pagesize);
> -
> -    /*
> -     * ftruncate is not supported by hugetlbfs in older
> -     * hosts, so don't bother bailing out on errors.
> -     * If anything goes wrong with it under other filesystems,
> -     * mmap will fail.
> -     */
> -    if (ftruncate(fd, memory)) {
> -        perror("ftruncate");
> -    }
>  
>      area = qemu_ram_mmap(fd, memory, pagesize, block->flags & RAM_SHARED);
>      if (area == MAP_FAILED) {
> -- 
> 1.8.3.1