hostmem-file and hostmem-memfd use the whole object path for the
memory region name, but hostname-ram uses only the path component (the
basename):

qemu -m 1024 -object memory-backend-ram,id=mem,size=1G -numa node,memdev=mem -monitor stdio
(qemu) info ramblock
              Block Name    PSize              Offset               Used              Total
                     mem    4 KiB  0x0000000000000000 0x0000000040000000 0x0000000040000000

qemu -m 1024 -object memory-backend-file,id=mem,size=1G,mem-path=/tmp/foo -numa node,memdev=mem -monitor stdio
(qemu) info ramblock
              Block Name    PSize              Offset               Used              Total
            /objects/mem    4 KiB  0x0000000000000000 0x0000000040000000 0x0000000040000000

qemu -m 1024 -object memory-backend-memfd,id=mem,size=1G -numa node,memdev=mem -monitor stdio
(qemu) info ramblock
              Block Name    PSize              Offset               Used              Total
            /objects/mem    4 KiB  0x0000000000000000 0x0000000040000000 0x0000000040000000

Use the whole path name with >= 3.1. Having a consistent naming allow
to migrate to different hostmem backends.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/hw/compat.h    |  6 +++++-
 backends/hostmem-ram.c | 47 ++++++++++++++++++++++++++++++++++++++----
 2 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/include/hw/compat.h b/include/hw/compat.h
index 6f4d5fc647..8ce7a7057b 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -2,7 +2,11 @@
 #define HW_COMPAT_H
 
 #define HW_COMPAT_3_0 \
-    /* empty */
+    {\
+        .driver   = "memory-backend-ram",\
+        .property = "x-component-name",\
+        .value    = "true",\
+    },
 
 #define HW_COMPAT_2_12 \
     {\
diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
index 7ddd08d370..a9eb99cf1b 100644
--- a/backends/hostmem-ram.c
+++ b/backends/hostmem-ram.c
@@ -16,21 +16,56 @@
 
 #define TYPE_MEMORY_BACKEND_RAM "memory-backend-ram"
 
+typedef struct RamMemoryBackend {
+    HostMemoryBackend parent;
+
+    bool component_name;
+} RamMemoryBackend;
+
+#define RAM_BACKEND(obj) \
+    OBJECT_CHECK(RamMemoryBackend, (obj), TYPE_MEMORY_BACKEND_RAM)
+
+static char *
+ram_backend_get_name(RamMemoryBackend *self)
+{
+    /* < 3.1 use the component as memory region name */
+    if (self->component_name) {
+        return object_get_canonical_path_component(OBJECT(self));
+    }
+
+    return object_get_canonical_path(OBJECT(self));
+}
 
 static void
 ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
 {
-    char *path;
+    char *name;
 
     if (!backend->size) {
         error_setg(errp, "can't create backend with size 0");
         return;
     }
 
-    path = object_get_canonical_path_component(OBJECT(backend));
-    memory_region_init_ram_shared_nomigrate(&backend->mr, OBJECT(backend), path,
+    name = ram_backend_get_name(RAM_BACKEND(backend));
+    memory_region_init_ram_shared_nomigrate(&backend->mr, OBJECT(backend), name,
                            backend->size, backend->share, errp);
-    g_free(path);
+    g_free(name);
+}
+
+static bool
+ram_backend_get_component_name(Object *obj, Error **errp)
+{
+    RamMemoryBackend *self = RAM_BACKEND(obj);
+
+    return self->component_name;
+}
+
+static void
+ram_backend_set_component_name(Object *obj, bool value, Error **errp)
+{
+    RamMemoryBackend *self = RAM_BACKEND(obj);
+
+    self->component_name = value;
 }
 
 static void
@@ -39,11 +74,15 @@ ram_backend_class_init(ObjectClass *oc, void *data)
     HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc);
 
     bc->alloc = ram_backend_memory_alloc;
+    object_class_property_add_bool(oc, "x-component-name",
+        ram_backend_get_component_name,
+        ram_backend_set_component_name, &error_abort);
 }
 
 static const TypeInfo ram_backend_info = {
     .name = TYPE_MEMORY_BACKEND_RAM,
     .parent = TYPE_MEMORY_BACKEND,
+    .instance_size = sizeof(RamMemoryBackend),
     .class_init = ram_backend_class_init,
 };
 
-- 
2.19.0.rc1

On Wed, 12 Sep 2018 16:55:31 +0400
Marc-André Lureau <marcandre.lureau@redhat.com> wrote:

> hostmem-file and hostmem-memfd use the whole object path

maybe add something along the lines:

consisting from user supplied 'id' and ...

> for the
> memory region name, but hostname-ram uses only the path component (the
> basename):

so path component would mean a concrete thing.

> 
> qemu -m 1024 -object memory-backend-ram,id=mem,size=1G -numa node,memdev=mem -monitor stdio
> (qemu) info ramblock
>               Block Name    PSize              Offset               Used              Total
>                      mem    4 KiB  0x0000000000000000 0x0000000040000000 0x0000000040000000
> 
> qemu -m 1024 -object memory-backend-file,id=mem,size=1G,mem-path=/tmp/foo -numa node,memdev=mem -monitor stdio
> (qemu) info ramblock
>               Block Name    PSize              Offset               Used              Total
>             /objects/mem    4 KiB  0x0000000000000000 0x0000000040000000 0x0000000040000000
> 
> qemu -m 1024 -object memory-backend-memfd,id=mem,size=1G -numa node,memdev=mem -monitor stdio
> (qemu) info ramblock
>               Block Name    PSize              Offset               Used              Total
>             /objects/mem    4 KiB  0x0000000000000000 0x0000000040000000 0x0000000040000000
> 
> Use the whole path name with >= 3.1. Having a consistent naming allow
> to migrate to different hostmem backends.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/hw/compat.h    |  6 +++++-
>  backends/hostmem-ram.c | 47 ++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 48 insertions(+), 5 deletions(-)
> 
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 6f4d5fc647..8ce7a7057b 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -2,7 +2,11 @@
>  #define HW_COMPAT_H
>  
>  #define HW_COMPAT_3_0 \
> -    /* empty */
> +    {\
> +        .driver   = "memory-backend-ram",\
> +        .property = "x-component-name",\
> +        .value    = "true",\
> +    },
>  
>  #define HW_COMPAT_2_12 \
>      {\
> diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
> index 7ddd08d370..a9eb99cf1b 100644
> --- a/backends/hostmem-ram.c
> +++ b/backends/hostmem-ram.c
> @@ -16,21 +16,56 @@
>  
>  #define TYPE_MEMORY_BACKEND_RAM "memory-backend-ram"
>  
> +typedef struct RamMemoryBackend {
> +    HostMemoryBackend parent;
> +
> +    bool component_name;
> +} RamMemoryBackend;
> +
> +#define RAM_BACKEND(obj) \
> +    OBJECT_CHECK(RamMemoryBackend, (obj), TYPE_MEMORY_BACKEND_RAM)
> +
> +static char *
> +ram_backend_get_name(RamMemoryBackend *self)
> +{
> +    /* < 3.1 use the component as memory region name */
I'd drop comment here, it belongs more to HW_COMPAT_...

> +    if (self->component_name) {
> +        return object_get_canonical_path_component(OBJECT(self));
> +    }
> +
> +    return object_get_canonical_path(OBJECT(self));
> +}
>  
>  static void
>  ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>  {
> -    char *path;
> +    char *name;
>  
>      if (!backend->size) {
>          error_setg(errp, "can't create backend with size 0");
>          return;
>      }
>  
> -    path = object_get_canonical_path_component(OBJECT(backend));
> -    memory_region_init_ram_shared_nomigrate(&backend->mr, OBJECT(backend), path,
> +    name = ram_backend_get_name(RAM_BACKEND(backend));
> +    memory_region_init_ram_shared_nomigrate(&backend->mr, OBJECT(backend), name,
>                             backend->size, backend->share, errp);
> -    g_free(path);
> +    g_free(name);
> +}
> +
> +static bool
> +ram_backend_get_component_name(Object *obj, Error **errp)
> +{
> +    RamMemoryBackend *self = RAM_BACKEND(obj);
> +
> +    return self->component_name;
> +}
> +
> +static void
> +ram_backend_set_component_name(Object *obj, bool value, Error **errp)
> +{
> +    RamMemoryBackend *self = RAM_BACKEND(obj);
> +
> +    self->component_name = value;
>  }
>  
>  static void
> @@ -39,11 +74,15 @@ ram_backend_class_init(ObjectClass *oc, void *data)
>      HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc);
>  
>      bc->alloc = ram_backend_memory_alloc;
> +    object_class_property_add_bool(oc, "x-component-name",
If we would convert initial RAM to backend/frontend model,
we would actually need to keep current ramblock naming scheme for 
ram_backend and even do this /i.e. non path variant/ for other
backends to keep migration stream compatible.

How about adding this property to generic memory-backend and
naming it something like 'x-use-canonical-path-for-ramblock-id'

I wonder if there is a point to use path for ramblock at all,
considering that it is single name space for all backends anyways.
Maybe by default we should just use 'id' everywhere?

> +        ram_backend_get_component_name,
> +        ram_backend_set_component_name, &error_abort);

 + object_class_property_set_description(...)

>  }
>  
>  static const TypeInfo ram_backend_info = {
>      .name = TYPE_MEMORY_BACKEND_RAM,
>      .parent = TYPE_MEMORY_BACKEND,
> +    .instance_size = sizeof(RamMemoryBackend),
>      .class_init = ram_backend_class_init,
>  };
>  

* Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> hostmem-file and hostmem-memfd use the whole object path for the
> memory region name, but hostname-ram uses only the path component (the
> basename):
> 
> qemu -m 1024 -object memory-backend-ram,id=mem,size=1G -numa node,memdev=mem -monitor stdio
> (qemu) info ramblock
>               Block Name    PSize              Offset               Used              Total
>                      mem    4 KiB  0x0000000000000000 0x0000000040000000 0x0000000040000000
> 
> qemu -m 1024 -object memory-backend-file,id=mem,size=1G,mem-path=/tmp/foo -numa node,memdev=mem -monitor stdio
> (qemu) info ramblock
>               Block Name    PSize              Offset               Used              Total
>             /objects/mem    4 KiB  0x0000000000000000 0x0000000040000000 0x0000000040000000
> 
> qemu -m 1024 -object memory-backend-memfd,id=mem,size=1G -numa node,memdev=mem -monitor stdio
> (qemu) info ramblock
>               Block Name    PSize              Offset               Used              Total
>             /objects/mem    4 KiB  0x0000000000000000 0x0000000040000000 0x0000000040000000
> 
> Use the whole path name with >= 3.1. Having a consistent naming allow
> to migrate to different hostmem backends.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/hw/compat.h    |  6 +++++-
>  backends/hostmem-ram.c | 47 ++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 48 insertions(+), 5 deletions(-)
> 
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 6f4d5fc647..8ce7a7057b 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -2,7 +2,11 @@
>  #define HW_COMPAT_H
>  
>  #define HW_COMPAT_3_0 \
> -    /* empty */
> +    {\
> +        .driver   = "memory-backend-ram",\
> +        .property = "x-component-name",\
> +        .value    = "true",\
> +    },

I don't think this needs to be an 'x-' - I think we originally used
those just for eXperimental things.

However, that's a minor nit, so:


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

>  #define HW_COMPAT_2_12 \
>      {\
> diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
> index 7ddd08d370..a9eb99cf1b 100644
> --- a/backends/hostmem-ram.c
> +++ b/backends/hostmem-ram.c
> @@ -16,21 +16,56 @@
>  
>  #define TYPE_MEMORY_BACKEND_RAM "memory-backend-ram"
>  
> +typedef struct RamMemoryBackend {
> +    HostMemoryBackend parent;
> +
> +    bool component_name;
> +} RamMemoryBackend;
> +
> +#define RAM_BACKEND(obj) \
> +    OBJECT_CHECK(RamMemoryBackend, (obj), TYPE_MEMORY_BACKEND_RAM)
> +
> +static char *
> +ram_backend_get_name(RamMemoryBackend *self)
> +{
> +    /* < 3.1 use the component as memory region name */
> +    if (self->component_name) {
> +        return object_get_canonical_path_component(OBJECT(self));
> +    }
> +
> +    return object_get_canonical_path(OBJECT(self));
> +}
>  
>  static void
>  ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>  {
> -    char *path;
> +    char *name;
>  
>      if (!backend->size) {
>          error_setg(errp, "can't create backend with size 0");
>          return;
>      }
>  
> -    path = object_get_canonical_path_component(OBJECT(backend));
> -    memory_region_init_ram_shared_nomigrate(&backend->mr, OBJECT(backend), path,
> +    name = ram_backend_get_name(RAM_BACKEND(backend));
> +    memory_region_init_ram_shared_nomigrate(&backend->mr, OBJECT(backend), name,
>                             backend->size, backend->share, errp);
> -    g_free(path);
> +    g_free(name);
> +}
> +
> +static bool
> +ram_backend_get_component_name(Object *obj, Error **errp)
> +{
> +    RamMemoryBackend *self = RAM_BACKEND(obj);
> +
> +    return self->component_name;
> +}
> +
> +static void
> +ram_backend_set_component_name(Object *obj, bool value, Error **errp)
> +{
> +    RamMemoryBackend *self = RAM_BACKEND(obj);
> +
> +    self->component_name = value;
>  }
>  
>  static void
> @@ -39,11 +74,15 @@ ram_backend_class_init(ObjectClass *oc, void *data)
>      HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc);
>  
>      bc->alloc = ram_backend_memory_alloc;
> +    object_class_property_add_bool(oc, "x-component-name",
> +        ram_backend_get_component_name,
> +        ram_backend_set_component_name, &error_abort);
>  }
>  
>  static const TypeInfo ram_backend_info = {
>      .name = TYPE_MEMORY_BACKEND_RAM,
>      .parent = TYPE_MEMORY_BACKEND,
> +    .instance_size = sizeof(RamMemoryBackend),
>      .class_init = ram_backend_class_init,
>  };
>  
> -- 
> 2.19.0.rc1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK