Features/QOM-QAPI integration: Difference between revisions

From QEMU
(Created page with "Separating configuration of QOM objects from their run-time state, and express the configuration as a QAPI struct. == Implementation == New input visitor function (the imple...")
 
No edit summary
 
(18 intermediate revisions by the same user not shown)
Line 1: Line 1:
Separating configuration of QOM objects from their run-time state, and express the configuration as a QAPI struct.
Separating configuration of QOM objects from their run-time state, and express the configuration as a QAPI struct.
'''Mostly obsolete.''' An actual implementation of the concepts in this page can be found at https://patchew.org/QEMU/20211103173002.209906-1-kwolf@redhat.com/ (and possibly in further revisions of that series).


== Implementation ==
== Implementation ==
Line 7: Line 9:
                                   void *opaque, Error **errp);
                                   void *opaque, Error **errp);


New ObjectClass function pointer "configure" replaces the visitor step of user_creatable_add_type
New UserCreatableClass function pointer "configure" replaces the visitor step of user_creatable_add_type
  void (*configure)(Object *obj, Visitor *v, Error **errp);
  void (*configure)(Object *obj, Visitor *v, Error **errp);


The object_configure function wraps the "configure" function pointer and falls back to properties for anything that isn't consumed by the classes:
The user_creatable_configure function wraps the "configure" function pointer and falls back to properties for anything that isn't consumed by the classes:


  bool object_configure(Object *obj, Visitor *v, Error **errp) {
  bool user_creatable_configure(Object *obj, Visitor *v, Error **errp) {
     Error *local_err = NULL;
     Error *local_err = NULL;
    /*
      * Possibly, instead of using v directly, we'd need to interpose
      * another visitor that looks up global properties?
      */
     visit_start_struct(v, NULL, NULL, 0, &local_err);
     visit_start_struct(v, NULL, NULL, 0, &local_err);
     if (local_err) {
     if (local_err) {
         goto out;
         goto out;
     }
     }
     foreach super class from Object to object_get_class(obj) {
     if (ucc->configure) {
        if (oc->configure) {
        ucc->configure(obj, v, &local_err);
            oc->configure(obj, v, &local_err);
        if (local_err) {
            if (local_err) {
            goto out;
                goto out;
            }
         }
         }
     }
     }
Line 39: Line 43:
  }
  }


user_creatable_add_type uses object_configure to set properties
user_creatable_add_type uses user_creatable_configure to set properties:
 
     obj = object_new(type);
     obj = object_new(type);
     if (!object_configure(obj, v, &local_err)) {
     if (!user_creatable_configure(obj, v, &local_err)) {
         goto out;
         goto out;
     }
     }
Line 50: Line 55:
== Conversion ==
== Conversion ==


Conversion from writable properties to options struct adds a definition of oc->configure, such as:
Conversion from writable properties to options struct starts by defining ucc->configure, such as:


  struct RngEgd {
  struct RngEgd {
     RngBackend parent;
     RngBackend parent;
 
     CharBackend chr;
     CharBackend chr;
     RngEgdOptions config;
     /*
      * Not perfect, as it causes a leak unless you free in an instance finalize function
      * or elsewhere.  However, "RngEgdProperties config" doesn't work too well either, as there
      * is no qapi_free_RngEgdProperties_members function to free config->chardev only.
      */
    RngEgdProperties *config;
  };
  };
... instance_finalize needs to call qapi_free_RngEngProperties ...


  void qapi_RngEgd_configure(Object *obj, Visitor *v, Error **errp)
  void qapi_RngEgd_configure(Object *obj, Visitor *v, Error **errp)
  {
  {
     RngEgd *s = RNG_EGD(obj);
     RngEgd *s = RNG_EGD(obj);
     visit_type_RngEgdOptions(v, NULL, &s->config, errp);
     s->config = g_new0(RngEgdProperties, 1);
    visit_type_RngEgdProperties(v, NULL, &s->config, errp);
  }
  }
  ...
  ...
  oc->configure = qapi_RngEgd_configure;
  ucc->configure = qapi_RngEgd_configure;
 
(See full-ish patch later in the page).  Default values would be handled like:
 
void qapi_MemoryBackendOptions_configure(Object *obj, Visitor *v, Error **errp)
{
    RngEgd *s = RNG_EGD(obj);
    s->config = g_new0(MemoryBackendOptions, 1);
    visit_type_MemoryBackendOptions(v, NULL, &s->config, errp);
    s->config->share = s->config->has_share ? s->config->share : false;
    ...
}
 
or at the QAPI level (i.e. the QAPI description of MemoryBackendOptions would have a default
value that would be filled in by visit_type_MemoryBackendOptions).


The qapi_RngEgd_configure function could be later generated by QAPI, for example from:
The qapi_RngEgd_configure function could be later generated by QAPI, for example from:


  { 'object': 'RngEgd',
  { 'object': 'RngEgd',
   'configuration': {
   'qom-type': 'rng-egd',    # used to go from rng-egd to RNG_EGD()
        'chardev': 'str'
  'configuration': 'RngEgdProperties'
  }
  }
  }


The above QAPI declaration could also generate a constructor function for use in C code, for example:
The above QAPI declaration could also generate a constructor function for use in C code, for example:


  RngEgd *object_new_configure(const char *type, void (*v)(Visitor *v, const char *name, void **ptr, Error **errp),
  static inline RngEgd *rng_egd_new(RngOptions *opt, Error **errp)
{
    return RNG_EGD(user_creatable_new_configure(TYPE_RNG_EGD, visit_type_RngEgdProperties, opt, errp));
}
 
where user_creatable_new_configure is:
 
Object *user_creatable_new_configure(const char *type, void (*v)(Visitor *v, const char *name, void **ptr, Error **errp),
                               void *data, Error **errp)
                               void *data, Error **errp)
  {
  {
Line 84: Line 120:
     Error *local_err = NULL;
     Error *local_err = NULL;
     Visitor *qiv, *qov;
     Visitor *qiv, *qov;
 
     qov = qobject_output_visitor_new(&ret);
     qov = qobject_output_visitor_new(&ret);
     v(qov, NULL, &data, &local_err);
     v(qov, NULL, &data, &local_err);
Line 92: Line 128:
         return NULL;
         return NULL;
     }
     }
 
     qiv = qobject_input_visitor_new(ret);
     qiv = qobject_input_visitor_new(ret);
     o = object_new(type);
     o = object_new(type);
     object_configure(o, qiv, &local_err);
     user_creatable_configure(o, qiv, &local_err);
     visit_free(qiv);
     visit_free(qiv);
     if (local_err) {
     if (local_err) {
Line 104: Line 140:
     return o;
     return o;
  }
  }


static inline RngEgd *rng_egd_new(RngOptions *opt, Error **errp)
== Properties ==
{
    return RNG_EGD(object_new_configure(TYPE_RNG_EGD, visit_type_RngEgdOptions, opt, errp));
}


Properties still exist, but they are mostly read-only.  For example, rng_egd_get_chardev remains exactly the same but rng_egd_set_chardev goes away.  They also keep their role in releasing resources on "object-del" (though it's visible mostly in devices, and not for example in rng-egd because it uses a string property for "chardev").
Properties still exist, but they are mostly read-only.  For example, rng_egd_get_chardev remains exactly the same but rng_egd_set_chardev goes away.  They also keep their role in releasing resources on "object-del" (though it's visible mostly in devices, and not for example in rng-egd because it uses a string property for "chardev").
Line 122: Line 156:
Writable 1:1 mapping to runtime state fields are probably rare enough that we can treat them the same as the third case.
Writable 1:1 mapping to runtime state fields are probably rare enough that we can treat them the same as the third case.


QAPI can express the first two.  The third must remain in C code for now.
QAPI can express the first two.  The third must remain in C code unless somebody has ideas... It is arguably the most important for introspection, but qom-list might be enough since it only applies after an object has been generated and configured.
 
== Sample backends/rng-egd.c patch ==
 
diff --git a/backends/rng-egd.c b/backends/rng-egd.c
index 4de142b9dc..7bf732d292 100644
--- a/backends/rng-egd.c
+++ b/backends/rng-egd.c
@@ -25,7 +25,7 @@ struct RngEgd {
      RngBackend parent;
 
      CharBackend chr;
-    char *chr_name;
+    RngOptions *config;
  };
 
  static void rng_egd_request_entropy(RngBackend *b, RngRequest *req)
@@ -89,16 +89,10 @@ static void rng_egd_opened(RngBackend *b, Error **errp)
      RngEgd *s = RNG_EGD(b);
      Chardev *chr;
 
-    if (s->chr_name == NULL) {
-        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-                  "chardev", "a valid character device");
-        return;
-    }
-
-    chr = qemu_chr_find(s->chr_name);
+    chr = qemu_chr_find(s->config->chardev);
      if (chr == NULL) {
          error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
-                  "Device '%s' not found", s->chr_name);
+                  "Device '%s' not found", s->config->chardev);
          return;
      }
      if (!qemu_chr_fe_init(&s->chr, chr, errp)) {
@@ -110,19 +104,6 @@ static void rng_egd_opened(RngBackend *b, Error **errp)
                              rng_egd_chr_read, NULL, NULL, s, NULL, true);
  }
 
-static void rng_egd_set_chardev(Object *obj, const char *value, Error **errp)
-{
-    RngBackend *b = RNG_BACKEND(obj);
-    RngEgd *s = RNG_EGD(b);
-
-    if (b->opened) {
-        error_setg(errp, QERR_PERMISSION_DENIED);
-    } else {
-        g_free(s->chr_name);
-        s->chr_name = g_strdup(value);
-    }
-}
-
  static char *rng_egd_get_chardev(Object *obj, Error **errp)
  {
      RngEgd *s = RNG_EGD(obj);
@@ -140,17 +121,24 @@ static void rng_egd_finalize(Object *obj)
      RngEgd *s = RNG_EGD(obj);
 
      qemu_chr_fe_deinit(&s->chr, false);
-    g_free(s->chr_name);
+    qapi_free_RngOptions(s->config);
  }
 
+void rng_egd_configure(Object *obj, Visitor *v, Error **errp)
+{
+    RngEgd *s = RNG_EGD(obj);
+    s->config = g_new(RngEgdProperties, 1);
+    visit_type_RngEgdProperties(v, NULL, &s->config, errp);
+}
+
  static void rng_egd_class_init(ObjectClass *klass, void *data)
  {
      RngBackendClass *rbc = RNG_BACKEND_CLASS(klass);
 
+    klass->configure = rng_egd_configure;
      rbc->request_entropy = rng_egd_request_entropy;
      rbc->opened = rng_egd_opened;
-    object_class_property_add_str(klass, "chardev",
-                                  rng_egd_get_chardev, rng_egd_set_chardev);
+    object_class_property_add_str(klass, "chardev", rng_egd_get_chardev, NULL);
  }
 
  static const TypeInfo rng_egd_info = {

Latest revision as of 21:28, 3 November 2021

Separating configuration of QOM objects from their run-time state, and express the configuration as a QAPI struct.

Mostly obsolete. An actual implementation of the concepts in this page can be found at https://patchew.org/QEMU/20211103173002.209906-1-kwolf@redhat.com/ (and possibly in further revisions of that series).

Implementation

New input visitor function (the implementation in QObjectInputVisitor visits QLIST_FIRST(&qiv->stack)->h)

bool visit_struct_foreach_member(Visitor *v, void (*cb)(void *opaque, const char *name, Visitor *v, Error **errp),
                                 void *opaque, Error **errp);

New UserCreatableClass function pointer "configure" replaces the visitor step of user_creatable_add_type

void (*configure)(Object *obj, Visitor *v, Error **errp);

The user_creatable_configure function wraps the "configure" function pointer and falls back to properties for anything that isn't consumed by the classes:

bool user_creatable_configure(Object *obj, Visitor *v, Error **errp) {
    Error *local_err = NULL;
    /*
     * Possibly, instead of using v directly, we'd need to interpose
     * another visitor that looks up global properties?
     */
    visit_start_struct(v, NULL, NULL, 0, &local_err);
    if (local_err) {
        goto out;
    }
    if (ucc->configure) {
        ucc->configure(obj, v, &local_err);
        if (local_err) {
            goto out;
        }
    }
    if (!visit_struct_foreach_member(v, object_property_set, obj, errp)) {
        return false;
    }
out:
    if (local_err) {
        error_propagate(errp, local_err);
    } else {
        visit_check_struct(v, &local_err);
    }
    visit_end_struct(v, NULL);
    return local_err != NULL;
}

user_creatable_add_type uses user_creatable_configure to set properties:

    obj = object_new(type);
    if (!user_creatable_configure(obj, v, &local_err)) {
        goto out;
    }
    if (id != NULL) {
        ...
    }

Conversion

Conversion from writable properties to options struct starts by defining ucc->configure, such as:

struct RngEgd {
    RngBackend parent;

    CharBackend chr;

    /*
     * Not perfect, as it causes a leak unless you free in an instance finalize function
     * or elsewhere.  However, "RngEgdProperties config" doesn't work too well either, as there
     * is no qapi_free_RngEgdProperties_members function to free config->chardev only.
     */
    RngEgdProperties *config;
};

... instance_finalize needs to call qapi_free_RngEngProperties ...
void qapi_RngEgd_configure(Object *obj, Visitor *v, Error **errp)
{
    RngEgd *s = RNG_EGD(obj);
    s->config = g_new0(RngEgdProperties, 1);
    visit_type_RngEgdProperties(v, NULL, &s->config, errp);
}
...
ucc->configure = qapi_RngEgd_configure;

(See full-ish patch later in the page). Default values would be handled like:

void qapi_MemoryBackendOptions_configure(Object *obj, Visitor *v, Error **errp)
{
    RngEgd *s = RNG_EGD(obj);
    s->config = g_new0(MemoryBackendOptions, 1);
    visit_type_MemoryBackendOptions(v, NULL, &s->config, errp);

    s->config->share = s->config->has_share ? s->config->share : false;
    ...
}

or at the QAPI level (i.e. the QAPI description of MemoryBackendOptions would have a default value that would be filled in by visit_type_MemoryBackendOptions).

The qapi_RngEgd_configure function could be later generated by QAPI, for example from:

{ 'object': 'RngEgd',
  'qom-type': 'rng-egd',    # used to go from rng-egd to RNG_EGD()
  'configuration': 'RngEgdProperties'
}


The above QAPI declaration could also generate a constructor function for use in C code, for example:

static inline RngEgd *rng_egd_new(RngOptions *opt, Error **errp)
{
    return RNG_EGD(user_creatable_new_configure(TYPE_RNG_EGD, visit_type_RngEgdProperties, opt, errp));
}

where user_creatable_new_configure is:

Object *user_creatable_new_configure(const char *type, void (*v)(Visitor *v, const char *name, void **ptr, Error **errp),
                             void *data, Error **errp)
{
    Object *o;
    QObject *ret = NULL;
    Error *local_err = NULL;
    Visitor *qiv, *qov;

    qov = qobject_output_visitor_new(&ret);
    v(qov, NULL, &data, &local_err);
    visit_free(qov);
    if (local_err) {
        error_propagate(errp, local_err);
        return NULL;
    }

    qiv = qobject_input_visitor_new(ret);
    o = object_new(type);
    user_creatable_configure(o, qiv, &local_err);
    visit_free(qiv);
    if (local_err) {
        error_propagate(errp, local_err);
        object_unref(o);
        return NULL;
    }
    return o;
}

Properties

Properties still exist, but they are mostly read-only. For example, rng_egd_get_chardev remains exactly the same but rng_egd_set_chardev goes away. They also keep their role in releasing resources on "object-del" (though it's visible mostly in devices, and not for example in rng-egd because it uses a string property for "chardev").

Properties are _not_ exposed to QAPI (not yet at least). Properties will be of one of three kinds:

  • read-only 1:1 mapping to configuration fields (e.g. rng-random.filename)
  • read-only 1:1 mapping to runtime state fields (e.g. rng-egd.chardev)
  • writable, with side effects (e.g. throttle-group.limits)

Writable 1:1 mapping to runtime state fields are probably rare enough that we can treat them the same as the third case.

QAPI can express the first two. The third must remain in C code unless somebody has ideas... It is arguably the most important for introspection, but qom-list might be enough since it only applies after an object has been generated and configured.

Sample backends/rng-egd.c patch

diff --git a/backends/rng-egd.c b/backends/rng-egd.c
index 4de142b9dc..7bf732d292 100644
--- a/backends/rng-egd.c
+++ b/backends/rng-egd.c
@@ -25,7 +25,7 @@ struct RngEgd {
     RngBackend parent;
 
     CharBackend chr;
-    char *chr_name;
+    RngOptions *config;
 };
 
 static void rng_egd_request_entropy(RngBackend *b, RngRequest *req)
@@ -89,16 +89,10 @@ static void rng_egd_opened(RngBackend *b, Error **errp)
     RngEgd *s = RNG_EGD(b);
     Chardev *chr;
 
-    if (s->chr_name == NULL) {
-        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-                   "chardev", "a valid character device");
-        return;
-    }
-
-    chr = qemu_chr_find(s->chr_name);
+    chr = qemu_chr_find(s->config->chardev);
     if (chr == NULL) {
         error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
-                  "Device '%s' not found", s->chr_name);
+                  "Device '%s' not found", s->config->chardev);
         return;
     }
     if (!qemu_chr_fe_init(&s->chr, chr, errp)) {
@@ -110,19 +104,6 @@ static void rng_egd_opened(RngBackend *b, Error **errp)
                              rng_egd_chr_read, NULL, NULL, s, NULL, true);
 }
 
-static void rng_egd_set_chardev(Object *obj, const char *value, Error **errp)
-{
-    RngBackend *b = RNG_BACKEND(obj);
-    RngEgd *s = RNG_EGD(b);
-
-    if (b->opened) {
-        error_setg(errp, QERR_PERMISSION_DENIED);
-    } else {
-        g_free(s->chr_name);
-        s->chr_name = g_strdup(value);
-    }
-}
-
 static char *rng_egd_get_chardev(Object *obj, Error **errp)
 {
     RngEgd *s = RNG_EGD(obj);
@@ -140,17 +121,24 @@ static void rng_egd_finalize(Object *obj)
     RngEgd *s = RNG_EGD(obj);
 
     qemu_chr_fe_deinit(&s->chr, false);
-    g_free(s->chr_name);
+    qapi_free_RngOptions(s->config);
 }
 
+void rng_egd_configure(Object *obj, Visitor *v, Error **errp)
+{
+    RngEgd *s = RNG_EGD(obj);
+    s->config = g_new(RngEgdProperties, 1);
+    visit_type_RngEgdProperties(v, NULL, &s->config, errp);
+}
+
  static void rng_egd_class_init(ObjectClass *klass, void *data)
 {
     RngBackendClass *rbc = RNG_BACKEND_CLASS(klass);
 
+    klass->configure = rng_egd_configure;
     rbc->request_entropy = rng_egd_request_entropy;
     rbc->opened = rng_egd_opened;
-    object_class_property_add_str(klass, "chardev",
-                                  rng_egd_get_chardev, rng_egd_set_chardev);
+    object_class_property_add_str(klass, "chardev", rng_egd_get_chardev, NULL);
 }
 
 static const TypeInfo rng_egd_info = {