| Message ID | 20200624131045.14512-3-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | build some devices as modules. | expand |
On 2020-06-24 at 15:10 CEST, Gerd Hoffmann wrote... > 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> > --- > include/qom/object.h | 12 ++++++++++++ > qom/object.c | 14 ++++++++++++++ > 2 files changed, 26 insertions(+) > > diff --git a/include/qom/object.h b/include/qom/object.h > index 94a61ccc3fe8..51f188137f1f 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 > + * 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 6ece96bc2bfc..34daaf1280f5 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); > + } > +#endif I'm wondering if there is any reason to only trigger the module load when you don't find the object class. You could simply call module_load_qom_one under #ifdef CONFIG_MODULES. Performance wise, I don't think this makes much of a difference, and it simplifies the logical flow IMO. > + return oc; > +} > + > ObjectClass *object_class_get_parent(ObjectClass *class) > { > TypeImpl *type = type_get_parent(class->type); -- Cheers, Christophe de Dinechin (IRC c3d)
Hi, > > +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); > > + } > > +#endif > > I'm wondering if there is any reason to only trigger the module load when > you don't find the object class. You could simply call module_load_qom_one > under #ifdef CONFIG_MODULES. > > Performance wise, I don't think this makes much of a difference, and it > simplifies the logical flow IMO. I expect the common case is that the object class is found and there is rarely a need to actually load a module. take care, Gerd
On Mon, Jul 20, 2020 at 04:20:55PM +0200, Christophe de Dinechin wrote: > > On 2020-06-24 at 15:10 CEST, Gerd Hoffmann wrote... > > 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> > > --- > > include/qom/object.h | 12 ++++++++++++ > > qom/object.c | 14 ++++++++++++++ > > 2 files changed, 26 insertions(+) > > > > diff --git a/include/qom/object.h b/include/qom/object.h > > index 94a61ccc3fe8..51f188137f1f 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 > > + * 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 6ece96bc2bfc..34daaf1280f5 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); > > + } > > +#endif > > I'm wondering if there is any reason to only trigger the module load when > you don't find the object class. You could simply call module_load_qom_one > under #ifdef CONFIG_MODULES. > > Performance wise, I don't think this makes much of a difference, and it > simplifies the logical flow IMO. I wouldn't make that assumption about performance - module_load_qom_one() does alot of work, and there are places where object / class creation is performance critical. We might not happen to trigger them now with the current set of modules, but we can easily do so in future. Regards, Daniel
On 2020-06-24 at 15:10 CEST, Gerd Hoffmann wrote... > 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> > --- > include/qom/object.h | 12 ++++++++++++ > qom/object.c | 14 ++++++++++++++ > 2 files changed, 26 insertions(+) > > diff --git a/include/qom/object.h b/include/qom/object.h > index 94a61ccc3fe8..51f188137f1f 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 > + * 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 6ece96bc2bfc..34daaf1280f5 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) Re-reading this, it occurred to me that typename is a keyword in C++. I don't know if we want to make it a rule to not use C++ keywords just in case we would some day compile that code with a C++ compiler. > +{ > + ObjectClass *oc; > + > + oc = object_class_by_name(typename); > +#ifdef CONFIG_MODULES > + if (!oc) { > + module_load_qom_one(typename); > + oc = object_class_by_name(typename); > + } > +#endif > + return oc; > +} > + > ObjectClass *object_class_get_parent(ObjectClass *class) > { > TypeImpl *type = type_get_parent(class->type); -- Cheers, Christophe de Dinechin (IRC c3d)
> > +ObjectClass *module_object_class_by_name(const char *typename) > > Re-reading this, it occurred to me that typename is a keyword in C++. > > I don't know if we want to make it a rule to not use C++ keywords just in > case we would some day compile that code with a C++ compiler. If we want do this we have to touch lots of places anyway, using typename as variable is pretty common in qom code ... take care, Gerd
diff --git a/include/qom/object.h b/include/qom/object.h index 94a61ccc3fe8..51f188137f1f 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 + * 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 6ece96bc2bfc..34daaf1280f5 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); + } +#endif + return oc; +} + ObjectClass *object_class_get_parent(ObjectClass *class) { TypeImpl *type = type_get_parent(class->type);
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> --- include/qom/object.h | 12 ++++++++++++ qom/object.c | 14 ++++++++++++++ 2 files changed, 26 insertions(+)