Features/QOM-QAPI integration

From QEMU
Revision as of 21:28, 3 November 2021 by Paolo Bonzini (talk | contribs)
(diff) ← Older revision | Latest revision (diff) | Newer revision → (diff)

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 = {