Re: [Qemu-devel] [PATCH V9 1/4] mem: add share parameter to memory-backend-ram

2018-02-12 Thread Marcel Apfelbaum
Hi Eduardo,

[This is a re-send, I didn't see the response
 on the mailing list, sorry for the spam]

On 08/02/2018 23:00, Eduardo Habkost wrote:
> On Thu, Feb 01, 2018 at 10:55:08PM +0200, Marcel Apfelbaum wrote:
>> Currently only file backed memory backend can
>> be created with a "share" flag in order to allow
>> sharing guest RAM with other processes in the host.
>>
>> Add the "share" flag also to RAM Memory Backend
>> in order to allow remapping parts of the guest RAM
>> to different host virtual addresses. This is needed
>> by the RDMA devices in order to remap non-contiguous
>> QEMU virtual addresses to a contiguous virtual address range.
>>
>> Moved the "share" flag to the Host Memory base class,
>> modified phys_mem_alloc to include the new parameter
>> and a new interface memory_region_init_ram_shared_nomigrate.
>>
>> There are no functional changes if the new flag is not used.
>>
>> Signed-off-by: Marcel Apfelbaum 
> 
> Code looks correct, so:
> 
> Reviewed-by: Eduardo Habkost 
> 

Appreciated!

> But later can we please stop the explosion of memory_init_ram*()
> functions and replace them with a single function with a flags
> parameter?
> 

Sounds reasonable.


Thanks,
Marcel




Re: [Qemu-devel] [PATCH V9 1/4] mem: add share parameter to memory-backend-ram

2018-02-08 Thread Eduardo Habkost
On Thu, Feb 01, 2018 at 10:55:08PM +0200, Marcel Apfelbaum wrote:
> Currently only file backed memory backend can
> be created with a "share" flag in order to allow
> sharing guest RAM with other processes in the host.
> 
> Add the "share" flag also to RAM Memory Backend
> in order to allow remapping parts of the guest RAM
> to different host virtual addresses. This is needed
> by the RDMA devices in order to remap non-contiguous
> QEMU virtual addresses to a contiguous virtual address range.
> 
> Moved the "share" flag to the Host Memory base class,
> modified phys_mem_alloc to include the new parameter
> and a new interface memory_region_init_ram_shared_nomigrate.
> 
> There are no functional changes if the new flag is not used.
> 
> Signed-off-by: Marcel Apfelbaum 

Code looks correct, so:

Reviewed-by: Eduardo Habkost 

But later can we please stop the explosion of memory_init_ram*()
functions and replace them with a single function with a flags
parameter?

-- 
Eduardo



[Qemu-devel] [PATCH V9 1/4] mem: add share parameter to memory-backend-ram

2018-02-01 Thread Marcel Apfelbaum
Currently only file backed memory backend can
be created with a "share" flag in order to allow
sharing guest RAM with other processes in the host.

Add the "share" flag also to RAM Memory Backend
in order to allow remapping parts of the guest RAM
to different host virtual addresses. This is needed
by the RDMA devices in order to remap non-contiguous
QEMU virtual addresses to a contiguous virtual address range.

Moved the "share" flag to the Host Memory base class,
modified phys_mem_alloc to include the new parameter
and a new interface memory_region_init_ram_shared_nomigrate.

There are no functional changes if the new flag is not used.

Signed-off-by: Marcel Apfelbaum 
---
 backends/hostmem-file.c  | 25 +
 backends/hostmem-ram.c   |  4 ++--
 backends/hostmem.c   | 21 +
 exec.c   | 26 +++---
 include/exec/memory.h| 23 +++
 include/exec/ram_addr.h  |  3 ++-
 include/qemu/osdep.h |  2 +-
 include/sysemu/hostmem.h |  2 +-
 include/sysemu/kvm.h |  2 +-
 memory.c | 16 +---
 qemu-options.hx  | 10 +-
 target/s390x/kvm.c   |  4 ++--
 util/oslib-posix.c   |  4 ++--
 util/oslib-win32.c   |  2 +-
 14 files changed, 94 insertions(+), 50 deletions(-)

diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index e319ec1ad8..134b08d63a 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -31,7 +31,6 @@ typedef struct HostMemoryBackendFile HostMemoryBackendFile;
 struct HostMemoryBackendFile {
 HostMemoryBackend parent_obj;
 
-bool share;
 bool discard_data;
 char *mem_path;
 uint64_t align;
@@ -59,7 +58,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error 
**errp)
 path = object_get_canonical_path(OBJECT(backend));
 memory_region_init_ram_from_file(>mr, OBJECT(backend),
  path,
- backend->size, fb->align, fb->share,
+ backend->size, fb->align, backend->share,
  fb->mem_path, errp);
 g_free(path);
 }
@@ -86,25 +85,6 @@ static void set_mem_path(Object *o, const char *str, Error 
**errp)
 fb->mem_path = g_strdup(str);
 }
 
-static bool file_memory_backend_get_share(Object *o, Error **errp)
-{
-HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
-
-return fb->share;
-}
-
-static void file_memory_backend_set_share(Object *o, bool value, Error **errp)
-{
-HostMemoryBackend *backend = MEMORY_BACKEND(o);
-HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
-
-if (host_memory_backend_mr_inited(backend)) {
-error_setg(errp, "cannot change property value");
-return;
-}
-fb->share = value;
-}
-
 static bool file_memory_backend_get_discard_data(Object *o, Error **errp)
 {
 return MEMORY_BACKEND_FILE(o)->discard_data;
@@ -171,9 +151,6 @@ file_backend_class_init(ObjectClass *oc, void *data)
 bc->alloc = file_backend_memory_alloc;
 oc->unparent = file_backend_unparent;
 
-object_class_property_add_bool(oc, "share",
-file_memory_backend_get_share, file_memory_backend_set_share,
-_abort);
 object_class_property_add_bool(oc, "discard-data",
 file_memory_backend_get_discard_data, 
file_memory_backend_set_discard_data,
 _abort);
diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
index 38977be73e..7ddd08d370 100644
--- a/backends/hostmem-ram.c
+++ b/backends/hostmem-ram.c
@@ -28,8 +28,8 @@ ram_backend_memory_alloc(HostMemoryBackend *backend, Error 
**errp)
 }
 
 path = object_get_canonical_path_component(OBJECT(backend));
-memory_region_init_ram_nomigrate(>mr, OBJECT(backend), path,
-   backend->size, errp);
+memory_region_init_ram_shared_nomigrate(>mr, OBJECT(backend), 
path,
+   backend->size, backend->share, errp);
 g_free(path);
 }
 
diff --git a/backends/hostmem.c b/backends/hostmem.c
index ee2c2d5bfd..1daf13bd2e 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -369,6 +369,24 @@ static void set_id(Object *o, const char *str, Error 
**errp)
 backend->id = g_strdup(str);
 }
 
+static bool host_memory_backend_get_share(Object *o, Error **errp)
+{
+HostMemoryBackend *backend = MEMORY_BACKEND(o);
+
+return backend->share;
+}
+
+static void host_memory_backend_set_share(Object *o, bool value, Error **errp)
+{
+HostMemoryBackend *backend = MEMORY_BACKEND(o);
+
+if (host_memory_backend_mr_inited(backend)) {
+error_setg(errp, "cannot change property value");
+return;
+}
+backend->share = value;
+}
+
 static void
 host_memory_backend_class_init(ObjectClass *oc, void *data)
 {
@@ -399,6 +417,9 @@ host_memory_backend_class_init(ObjectClass *oc, void *data)
 host_memory_backend_get_policy,