[Qemu-devel] [PATCH v2] backends/hostmem-file: Allow to specify full pathname for backing file

[Qemu-devel] [PATCH v2] backends/hostmem-file: Allow to specify full pathname for backing file

[Pavel Fedin]

This 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.

Distinction between directory and file name is done by stat() check. If an
existing directory is given, the code keeps old behavior. Otherwise it
creates or opens a file with the given pathname.

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
Tested-by: Igor Skalkin <i.skalkin@samsung.com>
---
v1 => v2:
- Changed title to more generic one
- Do not introduce new property, check whether the given path is a
  directory instead
---
 exec.c        | 34 +++++++++++++++++++++-------------
 qemu-doc.texi |  2 +-
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/exec.c b/exec.c
index 8af2570..3238c9a 100644
--- a/exec.c
+++ b/exec.c
@@ -1205,6 +1205,7 @@ static void *file_ram_alloc(RAMBlock *block,
                             const char *path,
                             Error **errp)
 {
+    struct stat st;
     char *filename;
     char *sanitized_name;
     char *c;
@@ -1233,26 +1234,33 @@ 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 = '_';
-    }
+    if (!stat(path, &st) && S_ISDIR(st.st_mode)) {
+        /* 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);
+        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);
+        }
+        g_free(filename);
+    } else {
+        fd = open(path, O_RDWR | O_CREAT, 0644);
+    }
 
-    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);
-    g_free(filename);
 
     memory = ROUND_UP(memory, hpagesize);
 
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 3126abd..460ab71 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/my-shmem-file,id=mb1
                  -device ivshmem,memdev=mb1
 @end example
 
-- 
1.9.5.msysgit.0

Re: [Qemu-devel] [PATCH v2] backends/hostmem-file: Allow to specify full pathname for backing file

[Paolo Bonzini]

On 28/10/2015 10:54, Pavel Fedin wrote:
> This 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.
> 
> Distinction between directory and file name is done by stat() check. If an
> existing directory is given, the code keeps old behavior. Otherwise it
> creates or opens a file with the given pathname.
> 
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> Tested-by: Igor Skalkin <i.skalkin@samsung.com>
> ---
> v1 => v2:
> - Changed title to more generic one
> - Do not introduce new property, check whether the given path is a
>   directory instead
> ---
>  exec.c        | 34 +++++++++++++++++++++-------------
>  qemu-doc.texi |  2 +-
>  2 files changed, 22 insertions(+), 14 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 8af2570..3238c9a 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1205,6 +1205,7 @@ static void *file_ram_alloc(RAMBlock *block,
>                              const char *path,
>                              Error **errp)
>  {
> +    struct stat st;
>      char *filename;
>      char *sanitized_name;
>      char *c;
> @@ -1233,26 +1234,33 @@ 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 = '_';
> -    }
> +    if (!stat(path, &st) && S_ISDIR(st.st_mode)) {
> +        /* 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);
> +        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);
> +        }
> +        g_free(filename);
> +    } else {
> +        fd = open(path, O_RDWR | O_CREAT, 0644);
> +    }
>  
> -    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);
> -    g_free(filename);
>  
>      memory = ROUND_UP(memory, hpagesize);
>  
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index 3126abd..460ab71 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/my-shmem-file,id=mb1
>                   -device ivshmem,memdev=mb1
>  @end example
>  
> 

Queued, thanks.

Paolo