From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41209) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zr5hS-0003f6-2o for qemu-devel@nongnu.org; Tue, 27 Oct 2015 10:58:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zr5hO-00083k-SJ for qemu-devel@nongnu.org; Tue, 27 Oct 2015 10:58:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36472) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zr5hO-00083P-JR for qemu-devel@nongnu.org; Tue, 27 Oct 2015 10:58:06 -0400 Date: Tue, 27 Oct 2015 15:58:02 +0100 From: Igor Mammedov Message-ID: <20151027155802.0f106106@nial.brq.redhat.com> In-Reply-To: <002701d110be$ed01e4d0$c705ae70$@samsung.com> References: <002701d110be$ed01e4d0$c705ae70$@samsung.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] backends/hostmem-file: Add file-name property List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pavel Fedin Cc: 'Paolo Bonzini' , qemu-devel@nongnu.org, 'Eduardo Habkost' On Tue, 27 Oct 2015 16:53:56 +0300 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. 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 > Tested-by: Igor Skalkin > --- > 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 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 >