[Qemu-devel] [PATCH v2 0/3] fix query-memdev not repporting IDs of memory backends

[Qemu-devel] [PATCH v2 0/3] fix query-memdev not repporting IDs of memory backends

[Igor Mammedov]

Changelog:
    since v1:
        * fix mistakes in commit messages/change them as suggested
        * extend 3/3 commit message to explain why it's ok to use 'id'
        * 3/3 set 'id' property directly instead of injecting it back
          into qdict.


Series is a couple of preparratory cleanups which simplify fix
and a fix itself.

Before fix HMP 'info memdevs' for CLI:
qemu-system-x86_64 -object memory-backend-ram,id=mem0,size=1G
outputs:

  memory backend: 0
    size:  1073741824
    merge: true
    dump: true
    prealloc: false
    policy: default
    host nodes: 128

after fix:

  memory backend: mem0
    size:  1073741824
    merge: true
    dump: true
    prealloc: false
    policy: default
    host nodes: 128

it should help to avoid remembering hotplugged IDs as they
could be queried back via HMP/QMP interface.

CC: ehabkost@redhat.com
CC: armbru@redhat.com
CC: dgilbert@redhat.com
CC: eblake@redhat.com
CC: afaerber@suse.de
CC: pbonzini@redhat.com

Igor Mammedov (3):
  qom: remove unused header
  reuse user_creatable_add_opts() instead of user_creatable_add() in
    monitor
  fix qmp/hmp query-memdev not reporting IDs of memory backends

 include/qom/object_interfaces.h | 17 ---------
 include/sysemu/hostmem.h        |  1 +
 backends/hostmem.c              | 26 ++++++++++++++
 docs/qmp-commands.txt           |  1 +
 hmp.c                           | 10 ++----
 numa.c                          |  3 ++
 qapi-schema.json                |  3 ++
 qom/object_interfaces.c         | 78 ++++++++++++-----------------------------
 8 files changed, 59 insertions(+), 80 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH v2 1/3] qom: remove unused header

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 qom/object_interfaces.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index ded4d84..4b880d0 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -3,7 +3,6 @@
 #include "qom/object_interfaces.h"
 #include "qemu/module.h"
 #include "qapi-visit.h"
-#include "qapi/qobject-output-visitor.h"
 #include "qapi/opts-visitor.h"
 
 void user_creatable_complete(Object *obj, Error **errp)
-- 
2.7.4

[Qemu-devel] [PATCH v2 2/3] reuse user_creatable_add_opts() instead of user_creatable_add() in monitor

[Igor Mammedov]

Simplify code by dropping ~57LOC by merging user_creatable_add()
into user_creatable_add_opts() and using the later from monitor.
Along with it allocate opts_visitor_new() once in user_creatable_add_opts().

As result we have one less API func and a more readable/simple
user_creatable_add_opts() vs user_creatable_add().

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 include/qom/object_interfaces.h | 17 ----------
 hmp.c                           |  5 +--
 qom/object_interfaces.c         | 71 ++++++++++-------------------------------
 3 files changed, 18 insertions(+), 75 deletions(-)

diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index 8b17f4d..fdd7603 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -76,23 +76,6 @@ void user_creatable_complete(Object *obj, Error **errp);
 bool user_creatable_can_be_deleted(UserCreatable *uc, Error **errp);
 
 /**
- * user_creatable_add:
- * @qdict: the object definition
- * @v: the visitor
- * @errp: if an error occurs, a pointer to an area to store the error
- *
- * Create an instance of the user creatable object whose type
- * is defined in @qdict by the 'qom-type' field, placing it
- * in the object composition tree with name provided by the
- * 'id' field. The remaining fields in @qdict are used to
- * initialize the object properties.
- *
- * Returns: the newly created object or NULL on error
- */
-Object *user_creatable_add(const QDict *qdict,
-                           Visitor *v, Error **errp);
-
-/**
  * user_creatable_add_type:
  * @type: the object type name
  * @id: the unique ID for the object
diff --git a/hmp.c b/hmp.c
index b869617..e7bead5 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1808,7 +1808,6 @@ void hmp_object_add(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;
     QemuOpts *opts;
-    Visitor *v;
     Object *obj = NULL;
 
     opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, &err);
@@ -1817,9 +1816,7 @@ void hmp_object_add(Monitor *mon, const QDict *qdict)
         return;
     }
 
-    v = opts_visitor_new(opts);
-    obj = user_creatable_add(qdict, v, &err);
-    visit_free(v);
+    obj = user_creatable_add_opts(opts, &err);
     qemu_opts_del(opts);
 
     if (err) {
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 4b880d0..9b4155a 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -34,57 +34,6 @@ bool user_creatable_can_be_deleted(UserCreatable *uc, Error **errp)
     }
 }
 
-
-Object *user_creatable_add(const QDict *qdict,
-                           Visitor *v, Error **errp)
-{
-    char *type = NULL;
-    char *id = NULL;
-    Object *obj = NULL;
-    Error *local_err = NULL;
-    QDict *pdict;
-
-    pdict = qdict_clone_shallow(qdict);
-
-    visit_start_struct(v, NULL, NULL, 0, &local_err);
-    if (local_err) {
-        goto out;
-    }
-
-    qdict_del(pdict, "qom-type");
-    visit_type_str(v, "qom-type", &type, &local_err);
-    if (local_err) {
-        goto out_visit;
-    }
-
-    qdict_del(pdict, "id");
-    visit_type_str(v, "id", &id, &local_err);
-    if (local_err) {
-        goto out_visit;
-    }
-    visit_check_struct(v, &local_err);
-    if (local_err) {
-        goto out_visit;
-    }
-
-    obj = user_creatable_add_type(type, id, pdict, v, &local_err);
-
-out_visit:
-    visit_end_struct(v, NULL);
-
-out:
-    QDECREF(pdict);
-    g_free(id);
-    g_free(type);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        object_unref(obj);
-        return NULL;
-    }
-    return obj;
-}
-
-
 Object *user_creatable_add_type(const char *type, const char *id,
                                 const QDict *qdict,
                                 Visitor *v, Error **errp)
@@ -157,13 +106,27 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error **errp)
 {
     Visitor *v;
     QDict *pdict;
-    Object *obj = NULL;
+    Object *obj;
+    const char *id = qemu_opts_id(opts);
+    const char *type = qemu_opt_get(opts, "qom-type");
+
+    if (!type) {
+        error_setg(errp, QERR_MISSING_PARAMETER, "qom-type");
+        return NULL;
+    }
+    if (!id) {
+        error_setg(errp, QERR_MISSING_PARAMETER, "id");
+        return NULL;
+    }
 
-    v = opts_visitor_new(opts);
     pdict = qemu_opts_to_qdict(opts, NULL);
+    qdict_del(pdict, "qom-type");
+    qdict_del(pdict, "id");
 
-    obj = user_creatable_add(pdict, v, errp);
+    v = opts_visitor_new(opts);
+    obj = user_creatable_add_type(type, id, pdict, v, errp);
     visit_free(v);
+
     QDECREF(pdict);
     return obj;
 }
-- 
2.7.4

[Qemu-devel] [PATCH v2 3/3] fix qmp/hmp query-memdev not reporting IDs of memory backends

[Igor Mammedov]

Considering 'id' is mandatory for user_creatable objects/backends
and user_creatable_add_type() always has it as an argument
regardless of where from it is called CLI/monitor or QMP,
Fix issue by adding 'id' property to hostmem backends and
set it in user_creatable_add_type() for every object that
implements 'id' property. Then later at query-memdev time
get 'id' from object directly.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 include/sysemu/hostmem.h |  1 +
 backends/hostmem.c       | 26 ++++++++++++++++++++++++++
 docs/qmp-commands.txt    |  1 +
 hmp.c                    |  5 +----
 numa.c                   |  3 +++
 qapi-schema.json         |  3 +++
 qom/object_interfaces.c  |  6 ++++++
 7 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
index 678232a..ecae0cf 100644
--- a/include/sysemu/hostmem.h
+++ b/include/sysemu/hostmem.h
@@ -52,6 +52,7 @@ struct HostMemoryBackend {
     Object parent;
 
     /* protected */
+    char *id;
     uint64_t size;
     bool merge, dump;
     bool prealloc, force_prealloc, is_mapped;
diff --git a/backends/hostmem.c b/backends/hostmem.c
index 4256d24..7f5de70 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -348,6 +348,24 @@ host_memory_backend_can_be_deleted(UserCreatable *uc, Error **errp)
     }
 }
 
+static char *get_id(Object *o, Error **errp)
+{
+    HostMemoryBackend *backend = MEMORY_BACKEND(o);
+
+    return g_strdup(backend->id);
+}
+
+static void set_id(Object *o, const char *str, Error **errp)
+{
+    HostMemoryBackend *backend = MEMORY_BACKEND(o);
+
+    if (backend->id) {
+        error_setg(errp, "cannot change property value");
+        return;
+    }
+    backend->id = g_strdup(str);
+}
+
 static void
 host_memory_backend_class_init(ObjectClass *oc, void *data)
 {
@@ -377,6 +395,13 @@ host_memory_backend_class_init(ObjectClass *oc, void *data)
         HostMemPolicy_lookup,
         host_memory_backend_get_policy,
         host_memory_backend_set_policy, &error_abort);
+    object_class_property_add_str(oc, "id", get_id, set_id, &error_abort);
+}
+
+static void host_memory_backend_finalize(Object *o)
+{
+    HostMemoryBackend *backend = MEMORY_BACKEND(o);
+    g_free(backend->id);
 }
 
 static const TypeInfo host_memory_backend_info = {
@@ -387,6 +412,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_finalize = host_memory_backend_finalize,
     .interfaces = (InterfaceInfo[]) {
         { TYPE_USER_CREATABLE },
         { }
diff --git a/docs/qmp-commands.txt b/docs/qmp-commands.txt
index abf210a..18db4cd 100644
--- a/docs/qmp-commands.txt
+++ b/docs/qmp-commands.txt
@@ -3529,6 +3529,7 @@ Example (1):
          "policy": "bind"
        },
        {
+         "id": "mem1",
          "size": 536870912,
          "merge": false,
          "dump": true,
diff --git a/hmp.c b/hmp.c
index e7bead5..8522efe 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2078,13 +2078,11 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
     MemdevList *m = memdev_list;
     Visitor *v;
     char *str;
-    int i = 0;
-
 
     while (m) {
         v = string_output_visitor_new(false, &str);
         visit_type_uint16List(v, NULL, &m->value->host_nodes, NULL);
-        monitor_printf(mon, "memory backend: %d\n", i);
+        monitor_printf(mon, "memory backend: %s\n", m->value->id);
         monitor_printf(mon, "  size:  %" PRId64 "\n", m->value->size);
         monitor_printf(mon, "  merge: %s\n",
                        m->value->merge ? "true" : "false");
@@ -2100,7 +2098,6 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
         g_free(str);
         visit_free(v);
         m = m->next;
-        i++;
     }
 
     monitor_printf(mon, "\n");
diff --git a/numa.c b/numa.c
index 9c09e45..f5fc7da 100644
--- a/numa.c
+++ b/numa.c
@@ -518,6 +518,9 @@ static int query_memdev(Object *obj, void *opaque)
 
         m->value = g_malloc0(sizeof(*m->value));
 
+        m->value->id = object_property_get_str(obj, "id", NULL);
+        m->value->has_id = !!m->value->id;
+
         m->value->size = object_property_get_int(obj, "size",
                                                  &error_abort);
         m->value->merge = object_property_get_bool(obj, "merge",
diff --git a/qapi-schema.json b/qapi-schema.json
index a0d3b5d..88eb2b9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4453,6 +4453,8 @@
 #
 # Information about memory backend
 #
+# @id: #optional backend's ID if backend has 'id' property (since 2.9)
+#
 # @size: memory backend size
 #
 # @merge: enables or disables memory merge support
@@ -4469,6 +4471,7 @@
 ##
 { 'struct': 'Memdev',
   'data': {
+    '*id':        'str',
     'size':       'size',
     'merge':      'bool',
     'dump':       'bool',
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 9b4155a..03a95c3 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -62,6 +62,12 @@ Object *user_creatable_add_type(const char *type, const char *id,
 
     assert(qdict);
     obj = object_new(type);
+    if (object_property_find(obj, "id", NULL)) {
+        object_property_set_str(obj, id, "id", &local_err);
+        if (local_err) {
+            goto out;
+        }
+    }
     visit_start_struct(v, NULL, NULL, 0, &local_err);
     if (local_err) {
         goto out;
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 3/3] fix qmp/hmp query-memdev not reporting IDs of memory backends

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1022 bytes --]

On 01/10/2017 06:53 AM, Igor Mammedov wrote:
> Considering 'id' is mandatory for user_creatable objects/backends
> and user_creatable_add_type() always has it as an argument
> regardless of where from it is called CLI/monitor or QMP,
> Fix issue by adding 'id' property to hostmem backends and
> set it in user_creatable_add_type() for every object that
> implements 'id' property. Then later at query-memdev time
> get 'id' from object directly.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  include/sysemu/hostmem.h |  1 +
>  backends/hostmem.c       | 26 ++++++++++++++++++++++++++
>  docs/qmp-commands.txt    |  1 +
>  hmp.c                    |  5 +----
>  numa.c                   |  3 +++
>  qapi-schema.json         |  3 +++
>  qom/object_interfaces.c  |  6 ++++++
>  7 files changed, 41 insertions(+), 4 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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 --]

Re: [Qemu-devel] [PATCH v2 0/3] fix query-memdev not repporting IDs of memory backends

[Eduardo Habkost]

On Tue, Jan 10, 2017 at 01:53:12PM +0100, Igor Mammedov wrote:
> 
> Changelog:
>     since v1:
>         * fix mistakes in commit messages/change them as suggested
>         * extend 3/3 commit message to explain why it's ok to use 'id'
>         * 3/3 set 'id' property directly instead of injecting it back
>           into qdict.
> 
> 
> Series is a couple of preparratory cleanups which simplify fix
> and a fix itself.
> 
> Before fix HMP 'info memdevs' for CLI:
> qemu-system-x86_64 -object memory-backend-ram,id=mem0,size=1G
> outputs:
> 
>   memory backend: 0
>     size:  1073741824
>     merge: true
>     dump: true
>     prealloc: false
>     policy: default
>     host nodes: 128
> 
> after fix:
> 
>   memory backend: mem0
>     size:  1073741824
>     merge: true
>     dump: true
>     prealloc: false
>     policy: default
>     host nodes: 128
> 
> it should help to avoid remembering hotplugged IDs as they
> could be queried back via HMP/QMP interface.
> 

Applied to machine-next. Thanks!

-- 
Eduardo