Re: [Qemu-devel] [PATCH v6 28/28] hostmem: use object id for memory region name with >= 4.0

[Igor Mammedov <imammedo@redhat.com>]

On Thu, 13 Dec 2018 01:48:50 +0400
Marc-André Lureau <marcandre.lureau@redhat.com> wrote:

> hostmem-file and hostmem-memfd use the whole object path for the
> memory region name, and hostname-ram uses only the path component (the
> object id, or canonical path basename):
> 
> 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
> 
> 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
> 
> For consistency, change to use object id for -file and -memfd as well
> with >= 4.0.
> 
> Having a consistent naming allows to migrate to different hostmem
> backends.

With neat below fixed:

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/sysemu/hostmem.h |  3 ++-
>  backends/hostmem-file.c  |  8 ++++----
>  backends/hostmem-memfd.c |  2 +-
>  backends/hostmem-ram.c   |  9 ++++-----
>  backends/hostmem.c       | 36 ++++++++++++++++++++++++++++++++++++
>  hw/core/machine.c        | 12 +++++++++++-
>  6 files changed, 58 insertions(+), 12 deletions(-)
> 
> diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
> index 6e6bd2c1cb..a023b372a4 100644
> --- a/include/sysemu/hostmem.h
> +++ b/include/sysemu/hostmem.h
> @@ -53,7 +53,7 @@ struct HostMemoryBackend {
>  
>      /* protected */
>      uint64_t size;
> -    bool merge, dump;
> +    bool merge, dump, use_canonical_path;
>      bool prealloc, force_prealloc, is_mapped, share;
>      DECLARE_BITMAP(host_nodes, MAX_NODES + 1);
>      HostMemPolicy policy;
> @@ -67,5 +67,6 @@ MemoryRegion *host_memory_backend_get_memory(HostMemoryBackend *backend);
>  void host_memory_backend_set_mapped(HostMemoryBackend *backend, bool mapped);
>  bool host_memory_backend_is_mapped(HostMemoryBackend *backend);
>  size_t host_memory_backend_pagesize(HostMemoryBackend *memdev);
> +char *host_memory_backend_get_name(HostMemoryBackend *backend);
>  
>  #endif
> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> index 78f058dee2..7a34e25c43 100644
> --- a/backends/hostmem-file.c
> +++ b/backends/hostmem-file.c
> @@ -43,7 +43,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>  {
>      HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
>  #ifdef CONFIG_POSIX
> -    gchar *path;
> +    gchar *name;
>  #endif
>  
>      if (!backend->size) {
> @@ -58,14 +58,14 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>      error_setg(errp, "-mem-path not supported on this host");
>  #else
>      backend->force_prealloc = mem_prealloc;
> -    path = object_get_canonical_path(OBJECT(backend));
> +    name = host_memory_backend_get_name(backend);
>      memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
> -                                     path,
> +                                     name,
>                                       backend->size, fb->align,
>                                       (backend->share ? RAM_SHARED : 0) |
>                                       (fb->is_pmem ? RAM_PMEM : 0),
>                                       fb->mem_path, errp);
> -    g_free(path);
> +    g_free(name);
>  #endif
>  }
>  
> diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
> index 2eb9c827a5..98c9bf3240 100644
> --- a/backends/hostmem-memfd.c
> +++ b/backends/hostmem-memfd.c
> @@ -53,7 +53,7 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>          return;
>      }
>  
> -    name = object_get_canonical_path(OBJECT(backend));
> +    name = host_memory_backend_get_name(backend);
>      memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend),
>                                     name, backend->size,
>                                     backend->share, fd, errp);
> diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
> index 7ddd08d370..24b65d9ae3 100644
> --- a/backends/hostmem-ram.c
> +++ b/backends/hostmem-ram.c
> @@ -16,21 +16,20 @@
>  
>  #define TYPE_MEMORY_BACKEND_RAM "memory-backend-ram"
>  
> -
>  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 = host_memory_backend_get_name(backend);
> +    memory_region_init_ram_shared_nomigrate(&backend->mr, OBJECT(backend), name,
>                             backend->size, backend->share, errp);
> -    g_free(path);
> +    g_free(name);
>  }
>  
>  static void
> diff --git a/backends/hostmem.c b/backends/hostmem.c
> index af800284e0..ff67e39c83 100644
> --- a/backends/hostmem.c
> +++ b/backends/hostmem.c
> @@ -28,6 +28,16 @@ QEMU_BUILD_BUG_ON(HOST_MEM_POLICY_BIND != MPOL_BIND);
>  QEMU_BUILD_BUG_ON(HOST_MEM_POLICY_INTERLEAVE != MPOL_INTERLEAVE);
>  #endif
>  
> +char *
> +host_memory_backend_get_name(HostMemoryBackend *self)

for consistency s/self/backend/


> +{
> +    if (!self->use_canonical_path) {
> +        return object_get_canonical_path_component(OBJECT(self));
> +    }
> +
> +    return object_get_canonical_path(OBJECT(self));
> +}
> +
>  static void
>  host_memory_backend_get_size(Object *obj, Visitor *v, const char *name,
>                               void *opaque, Error **errp)
> @@ -247,6 +257,11 @@ static void host_memory_backend_init(Object *obj)
>      backend->prealloc = mem_prealloc;
>  }
>  
> +static void host_memory_backend_post_init(Object *obj)
> +{
> +    object_apply_compat_props(obj);
> +}
> +
>  bool host_memory_backend_mr_inited(HostMemoryBackend *backend)
>  {
>      /*
> @@ -395,6 +410,23 @@ static void host_memory_backend_set_share(Object *o, bool value, Error **errp)
>      backend->share = value;
>  }
>  
> +static bool
> +host_memory_backend_get_use_canonical_path(Object *obj, Error **errp)
> +{
> +    HostMemoryBackend *self = MEMORY_BACKEND(obj);
> +
> +    return self->use_canonical_path;
> +}
> +
> +static void
> +host_memory_backend_set_use_canonical_path(Object *obj, bool value,
> +                                           Error **errp)
> +{
> +    HostMemoryBackend *self = MEMORY_BACKEND(obj);
> +
> +    self->use_canonical_path = value;
> +}
> +
>  static void
>  host_memory_backend_class_init(ObjectClass *oc, void *data)
>  {
> @@ -441,6 +473,9 @@ host_memory_backend_class_init(ObjectClass *oc, void *data)
>          &error_abort);
>      object_class_property_set_description(oc, "share",
>          "Mark the memory as private to QEMU or shared", &error_abort);
> +    object_class_property_add_bool(oc, "x-use-canonical-path-for-ramblock-id",
> +        host_memory_backend_get_use_canonical_path,
> +        host_memory_backend_set_use_canonical_path, &error_abort);
>  }
>  
>  static const TypeInfo host_memory_backend_info = {
> @@ -451,6 +486,7 @@ static const TypeInfo host_memory_backend_info = {
>      .class_init = host_memory_backend_class_init,
>      .instance_size = sizeof(HostMemoryBackend),
>      .instance_init = host_memory_backend_init,
> +    .instance_post_init = host_memory_backend_post_init,
>      .interfaces = (InterfaceInfo[]) {
>          { TYPE_USER_CREATABLE },
>          { }
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 7570408caa..cba79ef7ff 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -23,7 +23,17 @@
>  #include "sysemu/qtest.h"
>  #include "hw/pci/pci.h"
>  
> -GlobalProperty hw_compat_3_1[] = {};
> +GlobalProperty hw_compat_3_1[] = {
> +    {
> +        .driver   = "memory-backend-file",
> +        .property = "x-use-canonical-path-for-ramblock-id",
> +        .value    = "true",
> +    },{
> +        .driver   = "memory-backend-memfd",
> +        .property = "x-use-canonical-path-for-ramblock-id",
> +        .value    = "true",
> +    },
> +};
>  const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);
>  
>  GlobalProperty hw_compat_3_0[] = {};