backends/hostmem-file: Add file-name property

PATCH In review Pavel Fedin 3 participants 4 messages Oct 27, 2015 Arm STM32

Share & tools

Pavel Fedin

author

2015-10-27 13:54 UTC #0 lore raw

PATCH 1/1 backends/hostmem-file: Add file-name property

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.

Signed-off-by Pavel Fedin <[email protected]>

Tested-by Igor Skalkin <[email protected]>

---
 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(-)

backends/hostmem-file.c +25 -1

index e9b6d21..557eaf1 100644
@@ -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 = {

exec.c +24 ~~-12~~

index 8af2570..6cf1a36 100644
@@ -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;

include/exec/memory.h +3 -0

index 0f07159..58f56e8 100644
@@ -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

include/exec/ram_addr.h +1 -1

index 3360ac5..6d27c07 100644
@@ -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);

memory.c +3 -1

index 2eb1597..6cb8588 100644
@@ -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

numa.c +1 -1

index e9b18f5..93e3939 100644
@@ -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.

qemu-doc.texi +1 -1

index 3126abd..5e5d77b 100644
@@ -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

Signature · 1 line

1.9.5.msysgit.0

Eric Blake

2015-10-27 14:37 UTC #1 lore raw

Quoted 9 lines · Pavel Fedin

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.
Signed-off-by Pavel Fedin <[email protected]>
Tested-by Igor Skalkin <[email protected]>

Quoted 9 lines

---
 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(-)

Is there a missing qapi change, so that the new filename attribute can also be specified by QMP command? Or do we not support hotplug of ivshmem yet?

Signature · 2 lines

Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Pavel Fedin

author

2015-10-27 14:51 UTC #2 lore raw

Hello!

Quoted 3 lines

Is there a missing qapi change, so that the new filename attribute can also be specified by QMP command? Or do we not support hotplug of ivshmem yet?

To tell the truth, i don't know, and we currently do not test ivshmem with hotplug here, neither i heard that we would want it. And i actually even don't know how hotplug works in qemu. I discovered an issue and fixed it - this simple. Without this fix ivshmem + hostmem-file is unusable because the file cannot be located, therefore the memory cannot be shared. Could this (possibly) missing thing be added afterwards, if necessary? Anyway, my fix does not break anything (i hope).

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Igor Mammedov

2015-10-27 14:58 UTC #3 lore raw

In reply to Pavel Fedin

Quoted 6 lines · On Tue, 27 Oct 2015 16:53:56 +0300 Pavel Fedin <[email protected]> wrote: