* [Qemu-devel] [PATCH] backends/hostmem-file: Add file-name property
@ 2015-10-27 13:53 Pavel Fedin
2015-10-27 14:37 ` Eric Blake
2015-10-27 14:58 ` Igor Mammedov
0 siblings, 2 replies; 4+ messages in thread
From: Pavel Fedin @ 2015-10-27 13:53 UTC (permalink / raw)
To: qemu-devel; +Cc: 'Paolo Bonzini', 'Eduardo Habkost'
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 <p.fedin@samsung.com>
Tested-by: Igor Skalkin <i.skalkin@samsung.com>
---
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(-)
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index e9b6d21..557eaf1 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -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 = {
diff --git a/exec.c b/exec.c
index 8af2570..6cf1a36 100644
--- a/exec.c
+++ b/exec.c
@@ -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;
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 0f07159..58f56e8 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -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
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 3360ac5..6d27c07 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -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);
diff --git a/memory.c b/memory.c
index 2eb1597..6cb8588 100644
--- a/memory.c
+++ b/memory.c
@@ -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
diff --git a/numa.c b/numa.c
index e9b18f5..93e3939 100644
--- a/numa.c
+++ b/numa.c
@@ -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.
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 3126abd..5e5d77b 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,file-name=my_ivshmem,share=on,id=mb1
-device ivshmem,memdev=mb1
@end example
--
1.9.5.msysgit.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] backends/hostmem-file: Add file-name property
2015-10-27 13:53 [Qemu-devel] [PATCH] backends/hostmem-file: Add file-name property Pavel Fedin
@ 2015-10-27 14:37 ` Eric Blake
2015-10-27 14:51 ` Pavel Fedin
2015-10-27 14:58 ` Igor Mammedov
1 sibling, 1 reply; 4+ messages in thread
From: Eric Blake @ 2015-10-27 14:37 UTC (permalink / raw)
To: Pavel Fedin, qemu-devel
Cc: 'Paolo Bonzini', 'Eduardo Habkost'
[-- Attachment #1: Type: text/plain, Size: 1224 bytes --]
On 10/27/2015 07:53 AM, Pavel Fedin wrote:
> 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 <p.fedin@samsung.com>
> Tested-by: Igor Skalkin <i.skalkin@samsung.com>
> ---
> 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?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] backends/hostmem-file: Add file-name property
2015-10-27 14:37 ` Eric Blake
@ 2015-10-27 14:51 ` Pavel Fedin
0 siblings, 0 replies; 4+ messages in thread
From: Pavel Fedin @ 2015-10-27 14:51 UTC (permalink / raw)
To: 'Eric Blake', qemu-devel
Cc: 'Paolo Bonzini', 'Eduardo Habkost'
Hello!
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] backends/hostmem-file: Add file-name property
2015-10-27 13:53 [Qemu-devel] [PATCH] backends/hostmem-file: Add file-name property Pavel Fedin
2015-10-27 14:37 ` Eric Blake
@ 2015-10-27 14:58 ` Igor Mammedov
1 sibling, 0 replies; 4+ messages in thread
From: Igor Mammedov @ 2015-10-27 14:58 UTC (permalink / raw)
To: Pavel Fedin
Cc: 'Paolo Bonzini', qemu-devel, 'Eduardo Habkost'
On Tue, 27 Oct 2015 16:53:56 +0300
Pavel Fedin <p.fedin@samsung.com> wrote:
> 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.
How about reusing mem-path property instead of adding a new one?
if mem-path is directory use current behavior,
if it's file just use that file.
Also that would make patch less intrusive.
>
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> Tested-by: Igor Skalkin <i.skalkin@samsung.com>
> ---
> 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(-)
>
> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> index e9b6d21..557eaf1 100644
> --- a/backends/hostmem-file.c
> +++ b/backends/hostmem-file.c
> @@ -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 = {
> diff --git a/exec.c b/exec.c
> index 8af2570..6cf1a36 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -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;
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 0f07159..58f56e8 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -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
>
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index 3360ac5..6d27c07 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -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);
> diff --git a/memory.c b/memory.c
> index 2eb1597..6cb8588 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -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
> diff --git a/numa.c b/numa.c
> index e9b18f5..93e3939 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -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.
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index 3126abd..5e5d77b 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,file-name=my_ivshmem,share=on,id=mb1
> -device ivshmem,memdev=mb1
> @end example
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-10-27 14:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-27 13:53 [Qemu-devel] [PATCH] backends/hostmem-file: Add file-name property Pavel Fedin
2015-10-27 14:37 ` Eric Blake
2015-10-27 14:51 ` Pavel Fedin
2015-10-27 14:58 ` Igor Mammedov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).