[Qemu-devel] [RFC PATCH] generalize QOM path resolution

[Paolo Bonzini]

Right now, resolving a string to an object is not generic to QOM,
but rather it is entirely embedded in qdev (the Device class).
This embryo patch generalizes the concept adding a resolve_path
class method, and get_canonical_path instance method, to Object.
Link properties use the type to direct sets to the right resolve_path
method, while the qom-{get,set,list} commands get a class argument.

This is needed to have different namespaces for devices, host drives,
host chardevs, etc. and to make block/chardev/etc.  properties be simply
links (after QOMification).  That's distant, but we should get it
right before assumptions about global pathnames to pervade the code.

The patch is not even compiled, and absolutely not in shape for getting
in (but really, the only major hurdle for it is really Anthony's part 3
and pushing properties up in the hierarchy).  The usual questions about
this being the right thing to do already apply though.  And also: should
QOM class names be part of the ABI/API?

Signed-off-by: Paolo Bonzini <address@hidden>
---
 hw/qdev.c             |   34 +++++++++++++++++++++++++---------
 include/qemu/object.h |    7 +++++++
 qapi-schema.json      |    6 +++---
 qmp.c                 |   26 ++++++++++++++++++++++----
 qom/object.c          |   15 +++++++++++++++
 roms/SLOF             |    2 +-
 roms/seabios          |    2 +-
 7 files changed, 74 insertions(+), 18 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index a8c24de..1b8c1cd 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -43,6 +43,8 @@ DeviceInfo *device_info_list;
 static BusState *qbus_find_recursive(BusState *bus, const char *name,
                                      const BusInfo *info);
 static BusState *qbus_find(const char *path);
+static Object *device_resolve_path(const char *path, bool *ambiguous);
+static gchar *device_get_canonical_path(Object *obj);
 
 /* Register a new device type.  */
 static void qdev_subclass_init(ObjectClass *klass, void *data)
@@ -51,6 +53,8 @@ static void qdev_subclass_init(ObjectClass *klass, void *data)
 
     dc->info = data;
     dc->reset = dc->info->reset;
+    dc->parent_class.resolve_path = device_resolve_path;
+    dc->parent_class.get_canonical_path = device_get_canonical_path;
 
     /* Poison to try to detect future uses */
     dc->info->reset = NULL;
@@ -1329,11 +1333,11 @@ void qdev_property_add_child(DeviceState *dev, const 
char *name,
 static void qdev_get_link_property(DeviceState *dev, Visitor *v, void *opaque,
                                    const char *name, Error **errp)
 {
-    DeviceState **child = opaque;
+    Object **child = opaque;
     gchar *path;
 
     if (*child) {
-        path = qdev_get_canonical_path(*child);
+        path = object_get_canonical_path(*child);
         visit_type_str(v, &path, name, errp);
         g_free(path);
     } else {
@@ -1345,7 +1349,7 @@ static void qdev_get_link_property(DeviceState *dev, 
Visitor *v, void *opaque,
 static void qdev_set_link_property(DeviceState *dev, Visitor *v, void *opaque,
                                    const char *name, Error **errp)
 {
-    DeviceState **child = opaque;
+    Object **child = opaque;
     bool ambiguous = false;
     const char *type;
     char *path;
@@ -1359,16 +1363,16 @@ static void qdev_set_link_property(DeviceState *dev, 
Visitor *v, void *opaque,
     }
 
     if (strcmp(path, "") != 0) {
-        DeviceState *target;
+        Object *target;
 
-        target = qdev_resolve_path(path, &ambiguous);
+        target = object_resolve_path(type, path, &ambiguous);
         if (target) {
             gchar *target_type;
 
             target_type = g_strdup_printf("link<%s>", 
object_get_typename(OBJECT(target)));
             if (strcmp(target_type, type) == 0) {
                 *child = target;
-                qdev_ref(target);
+                qdev_ref(DEVICE(target));
             } else {
                 error_set(errp, QERR_INVALID_PARAMETER_TYPE, name, type);
             }
@@ -1400,10 +1404,11 @@ void qdev_property_add_link(DeviceState *dev, const 
char *name,
     g_free(full_type);
 }
 
-gchar *qdev_get_canonical_path(DeviceState *dev)
+gchar *device_get_canonical_path(Object *obj)
 {
     DeviceState *root = qdev_get_root();
     char *newpath = NULL, *path = NULL;
+    DeviceState *dev = DEVICE(obj);
 
     while (dev != root) {
         DeviceProperty *prop = NULL;
@@ -1438,6 +1443,11 @@ gchar *qdev_get_canonical_path(DeviceState *dev)
     return newpath;
 }
 
+gchar *qdev_get_canonical_path(DeviceState *dev)
+{
+    return object_get_canonical_path(OBJECT(dev));
+}
+
 static DeviceState *qdev_resolve_abs_path(DeviceState *parent,
                                           gchar **parts,
                                           int index)
@@ -1510,7 +1520,7 @@ static DeviceState *qdev_resolve_partial_path(DeviceState 
*parent,
     return dev;
 }
 
-DeviceState *qdev_resolve_path(const char *path, bool *ambiguous)
+Object *device_resolve_path(const char *path, bool *ambiguous)
 {
     bool partial_path = true;
     DeviceState *dev;
@@ -1537,7 +1547,13 @@ DeviceState *qdev_resolve_path(const char *path, bool 
*ambiguous)
 
     g_strfreev(parts);
 
-    return dev;
+    return OBJECT(dev);
+}
+
+DeviceState *qdev_resolve_path(const char *path, bool *ambiguous)
+{
+    Object *obj = object_resolve_path(TYPE_DEVICE, path, ambiguous);
+    return DEVICE(obj);
 }
 
 typedef struct StringProperty
diff --git a/include/qemu/object.h b/include/qemu/object.h
index ba37850..33728e3 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -122,6 +122,9 @@ typedef struct InterfaceInfo InterfaceInfo;
  */
 struct ObjectClass
 {
+    Object *(*resolve_path)(const char *path, bool *ambiguous);
+    gchar *(*get_canonical_path)(Object *obj);
+
     /*< private >*/
     Type type;
 };
@@ -420,6 +423,10 @@ ObjectClass *object_class_dynamic_cast_assert(ObjectClass 
*klass,
 ObjectClass *object_class_dynamic_cast(ObjectClass *klass,
                                        const char *typename);
 
+Object *object_resolve_path(const char *typename, const char *path,
+                            bool *ambiguous);
+gchar *object_get_canonical_path(Object *obj);
+
 /**
  * object_class_get_name:
  * @klass: The class to obtain the QOM typename for.
diff --git a/qapi-schema.json b/qapi-schema.json
index 80debe6..a47f689 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1249,7 +1249,7 @@
 #        releases.
 ##
 { 'command': 'qom-list',
-  'data': { 'path': 'str' },
+  'data': { '*class': 'str', 'path': 'str' },
   'returns': [ 'DevicePropertyInfo' ] }
 
 ##
@@ -1287,7 +1287,7 @@
 # Notes: This command is experimental and may change syntax in future releases.
 ##
 { 'command': 'qom-get',
-  'data': { 'path': 'str', 'property': 'str' },
+  'data': { '*class': 'str', 'path': 'str', 'property': 'str' },
   'returns': 'visitor',
   'gen': 'no' }
 
@@ -1308,7 +1308,7 @@
 # Notes: This command is experimental and may change syntax in future releases.
 ##
 { 'command': 'qom-set',
-  'data': { 'path': 'str', 'property': 'str', 'value': 'visitor' },
+  'data': { '*class': 'str', 'path': 'str', 'property': 'str', 'value': 
'visitor' },
   'gen': 'no' }
 
 ##
diff --git a/qmp.c b/qmp.c
index 1222b6c..a869af8 100644
--- a/qmp.c
+++ b/qmp.c
@@ -164,14 +164,20 @@ void qmp_cont(Error **errp)
     vm_start();
 }
 
-DevicePropertyInfoList *qmp_qom_list(const char *path, Error **errp)
+DevicePropertyInfoList *qmp_qom_list(const char *class, const char *path,
+                                    Error **errp)
 {
+    Object *obj;
     DeviceState *dev;
     bool ambiguous = false;
     DevicePropertyInfoList *props = NULL;
     DeviceProperty *prop;
 
-    dev = qdev_resolve_path(path, &ambiguous);
+    if (!class) {
+        class = TYPE_DEVICE;
+    }
+    obj = object_resolve_path(class, path, &ambiguous);
+    dev = DEVICE(obj);
     if (dev == NULL) {
         error_set(errp, QERR_DEVICE_NOT_FOUND, path);
         return NULL;
@@ -194,14 +200,20 @@ DevicePropertyInfoList *qmp_qom_list(const char *path, 
Error **errp)
 /* FIXME: teach qapi about how to pass through Visitors */
 int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret)
 {
+    const char *class = qdict_get_str(qdict, "class");
     const char *path = qdict_get_str(qdict, "path");
     const char *property = qdict_get_str(qdict, "property");
     QObject *value = qdict_get(qdict, "value");
     Error *local_err = NULL;
+    Object *obj;
     QmpInputVisitor *mi;
     DeviceState *dev;
 
-    dev = qdev_resolve_path(path, NULL);
+    if (!class) {
+        class = TYPE_DEVICE;
+    }
+    obj = object_resolve_path(class, path, NULL);
+    dev = DEVICE(obj);
     if (!dev) {
         error_set(&local_err, QERR_DEVICE_NOT_FOUND, path);
         goto out;
@@ -224,13 +236,19 @@ out:
 
 int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret)
 {
+    const char *class = qdict_get_str(qdict, "class");
     const char *path = qdict_get_str(qdict, "path");
     const char *property = qdict_get_str(qdict, "property");
     Error *local_err = NULL;
     QmpOutputVisitor *mo;
+    Object *obj;
     DeviceState *dev;
 
-    dev = qdev_resolve_path(path, NULL);
+    if (!class) {
+        class = TYPE_DEVICE;
+    }
+    obj = object_resolve_path(class, path, NULL);
+    dev = DEVICE(obj);
     if (!dev) {
         error_set(&local_err, QERR_DEVICE_NOT_FOUND, path);
         goto out;
diff --git a/qom/object.c b/qom/object.c
index a12895f..fd8f3f9 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -488,3 +488,18 @@ void object_class_foreach(void (*fn)(ObjectClass *klass, 
void *opaque),
 
     g_hash_table_foreach(type_table_get(), object_class_foreach_tramp, &data);
 }
+
+gchar *object_get_canonical_path(Object *obj)
+{
+    return obj->class->get_canonical_path(obj);
+}
+
+Object *object_resolve_path(const char *typename, const char *path,
+                            bool *ambiguous)
+{
+    ObjectClass *klass = object_class_by_name(typename);
+    if (!klass->resolve_path) {
+        return NULL;
+    }
+    return klass->resolve_path(path, ambiguous);
+}
diff --git a/roms/SLOF b/roms/SLOF
index ab062ff..32e3430 160000
--- a/roms/SLOF
+++ b/roms/SLOF
@@ -1 +1 @@
-Subproject commit ab062ff3b37c39649f2b0d94ed607adc6f6b3c7d
+Subproject commit 32e3430c018ceb8413cb808477449d1968c42497
diff --git a/roms/seabios b/roms/seabios
index 80d11e8..8e30147 160000
--- a/roms/seabios
+++ b/roms/seabios
@@ -1 +1 @@
-Subproject commit 80d11e8577bf03e98f2eb1b0cb3a281ab2879c9e
+Subproject commit 8e301472e324b6d6496d8b4ffc66863e99d7a505
-- 
1.7.7.6