Hello Gerd, Paolo,
this commit from 2020, together with others in the same series, contains a flaw
that now comes back to bite.
From commit 28457744c345ca4ccb58c984c9552e9c5955a9de ("module: qom module
support") :
On 6/24/20 15:10, Gerd Hoffmann wrote:
Add support for qom types provided by modules. For starters use a
manually maintained list which maps qom type to module and prefix.
Two load functions are added: One to load the module for a specific
type, and one to load all modules (needed for object/device lists as
printed by -- for example -- qemu -device help).
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
include/qemu/module.h | 2 ++
util/module.c | 55 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 57 insertions(+)
diff --git a/include/qemu/module.h b/include/qemu/module.h
index 011ae1ae7605..9121a475c1b6 100644
--- a/include/qemu/module.h
+++ b/include/qemu/module.h
@@ -70,5 +70,7 @@ void register_dso_module_init(void (*fn)(void),
module_init_type type);
void module_call_init(module_init_type type);
bool module_load_one(const char *prefix, const char *lib_name);
+void module_load_qom_one(const char *type);
+void module_load_qom_all(void);
See the "+void" just below the "bool". We have now hidden all the return values
of module_load_one,
so now we cannot figure out if a load was successful or not, and the fact that
module_load_one returns bool is now pointless, at least for qom.
As a pattern, I think this is rarely a good idea.
#endif
diff --git a/util/module.c b/util/module.c
index e48d9aacc05a..ee560a4b4269 100644
--- a/util/module.c
+++ b/util/module.c
@@ -245,3 +245,58 @@ bool module_load_one(const char *prefix, const char
*lib_name)
#endif
return success;
}
+
+/*
+ * Building devices and other qom objects modular is mostly useful in
+ * case they have dependencies to external shared libraries, so we can
+ * cut down the core qemu library dependencies. Which is the case for
+ * only a very few devices & objects.
+ *
+ * So with the expectation that this will be rather the exception than
+ * to rule and the list will not gain that many entries go with a
+ * simple manually maintained list for now.
+ */
+static struct {
+ const char *type;
+ const char *prefix;
+ const char *module;
+} const qom_modules[] = {
+};
+
+static bool module_loaded_qom_all;
+
+void module_load_qom_one(const char *type)
+{
+ int i;
+
+ if (module_loaded_qom_all) {
+ return;
+ }
+ for (i = 0; i < ARRAY_SIZE(qom_modules); i++) {
+ if (strcmp(qom_modules[i].type, type) == 0) {
+ module_load_one(qom_modules[i].prefix,
+ qom_modules[i].module);
+ return;
return value lost.
Should we not at least trace something, warn something?
Maybe we want to do it in module_load_one?
+ }
+ }
+}
+
+void module_load_qom_all(void)
+{
+ int i;
+
+ if (module_loaded_qom_all) {
+ return;
+ }
+ for (i = 0; i < ARRAY_SIZE(qom_modules); i++) {
+ if (i > 0 && (strcmp(qom_modules[i - 1].module,
+ qom_modules[i].module) == 0 &&
+ strcmp(qom_modules[i - 1].prefix,
+ qom_modules[i].prefix) == 0)) {
+ /* one module implementing multiple types -> load only once */
+ continue;
+ }
+ module_load_one(qom_modules[i].prefix, qom_modules[i].module);
return value lost.
+ }
+ module_loaded_qom_all = true;
+}
Pair this with the next commit,
commit 0f8198f1b2f3c33df2381c412ad8d8fd219b90b2 ("object: qom module support") :
which goes:
commit 0f8198f1b2f3c33df2381c412ad8d8fd219b90b2
Author: Gerd Hoffmann <kraxel@redhat.com>
Date: Wed Jun 24 15:10:37 2020 +0200
object: qom module support
Little helper function to load modules on demand. In most cases adding
module loading support for devices and other objects is just
s/object_class_by_name/module_object_class_by_name/ in the right spot.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200624131045.14512-3-kraxel@redhat.com
diff --git a/include/qom/object.h b/include/qom/object.h
index 94a61ccc3f..51f188137f 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -994,6 +994,18 @@ bool object_class_is_abstract(ObjectClass *klass);
*/
ObjectClass *object_class_by_name(const char *typename);
+/**
+ * module_object_class_by_name:
+ * @typename: The QOM typename to obtain the class for.
+ *
+ * For objects which might be provided by a module. Behaves like
+ * object_class_by_name, but additionally tries to load the module
Note the use of the word "tries" in the comment.
+ * needed in case the class is not available.
+ *
+ * Returns: The class for @typename or %NULL if not found.
+ */
+ObjectClass *module_object_class_by_name(const char *typename);
+
void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque),
const char *implements_type, bool include_abstract,
void *opaque);
diff --git a/qom/object.c b/qom/object.c
index 6ece96bc2b..34daaf1280 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -985,6 +985,20 @@ ObjectClass *object_class_by_name(const char *typename)
return type->class;
}
+ObjectClass *module_object_class_by_name(const char *typename)
+{
+ ObjectClass *oc;
+
+ oc = object_class_by_name(typename);
+#ifdef CONFIG_MODULES
+ if (!oc) {
+ module_load_qom_one(typename);
+ oc = object_class_by_name(typename);
now here we fail during object_class_by_name() with oc that is NULL. So I guess
we can evince that module_load_qom_one failed from here.
+ }
+#endif
+ return oc;
+}
+
ObjectClass *object_class_get_parent(ObjectClass *class)
{
TypeImpl *type = type_get_parent(class->type);
We should be able to tell if a load is successful or not I think, but before
that,
I think we are currently inconsistent, and do not report enough in terms of
tracing and errors to the user
as to the reason that a module load fails.
Either we add tracing to module_load_one, and make it so that module_load_one
also returns void,
or we change module_load_qom_one to also return bool.
Note, qdev also makes use of this facility.
Thoughts?