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

[Vladimir Sementsov-Ogievskiy]

On 30.10.2015 08:56, 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);
> +}
> +
> +static int open_file_path(RAMBlock *block, const char *path, size_t size)

I think the name should be more descriptive, as it is very special 
function in common exec.c and it doesn't "just open file path'.. May be 
open_ram_file_path


> +{
> +    char *filename;
> +    char *sanitized_name;
> +    char *c;
> +    int fd;
> +
> +    if (!path_is_dir(path)) {
> +        int flags = (block->flags & RAM_SHARED) ? O_RDWR : O_RDONLY;
> +
> +        flags |= O_EXCL;
> +        return open(path, flags);
> +    }

Was not there old scenarios when path is file? statfs will  success for 
any file withing mounted filesystem.

Also, may be we shouldn't warn about "Memory is not allocated from 
HugeTlbfs.\n" in case of !path_is_dir ?

> +
> +    /* 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) {


--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.