hw/core/qdev.c | 23 ++--------------------- include/qom/object.h | 12 ++++++++++++ qom/object.c | 25 ++++++++++++++++++++++++- 3 files changed, 38 insertions(+), 22 deletions(-)
The module resolving in qdev_new() is specific to QOM, not to
QDev. Extract the code as a new QOM object_try_new() helper so
it can be reused by non-QDev code.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
RFC because I'm wonder if we can't find a better name...
Also, should we refactor object_initialize() similarly,
having object_try_initialize(..., Error *)?
---
hw/core/qdev.c | 23 ++---------------------
include/qom/object.h | 12 ++++++++++++
qom/object.c | 25 ++++++++++++++++++++++++-
3 files changed, 38 insertions(+), 22 deletions(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index d759c4602c..3a076dcc7f 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -147,31 +147,12 @@ bool qdev_set_parent_bus(DeviceState *dev, BusState *bus, Error **errp)
DeviceState *qdev_new(const char *name)
{
- ObjectClass *oc = object_class_by_name(name);
-#ifdef CONFIG_MODULES
- if (!oc) {
- int rv = module_load_qom(name, &error_fatal);
- if (rv > 0) {
- oc = object_class_by_name(name);
- } else {
- error_report("could not find a module for type '%s'", name);
- exit(1);
- }
- }
-#endif
- if (!oc) {
- error_report("unknown type '%s'", name);
- abort();
- }
- return DEVICE(object_new(name));
+ return DEVICE(object_try_new(name, &error_fatal));
}
DeviceState *qdev_try_new(const char *name)
{
- if (!module_object_class_by_name(name)) {
- return NULL;
- }
- return DEVICE(object_new(name));
+ return DEVICE(object_try_new(name, NULL));
}
static QTAILQ_HEAD(, DeviceListener) device_listeners
diff --git a/include/qom/object.h b/include/qom/object.h
index ef7258a5e1..27059cafb7 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -565,6 +565,18 @@ Object *object_new_with_class(ObjectClass *klass);
*/
Object *object_new(const char *typename);
+/**
+ * object_try_new: Try to create an object on the heap
+ * @typename: The name of the type of the object to instantiate.
+ * @errp: pointer to Error object.
+ *
+ * This is like object_new(), except it returns %NULL when type @typename
+ * does not exist, rather than asserting.
+ *
+ * Returns: The newly allocated and instantiated object, or %NULL.
+ */
+Object *object_try_new(const char *typename, Error **errp);
+
/**
* object_new_with_props:
* @typename: The name of the type of the object to instantiate.
diff --git a/qom/object.c b/qom/object.c
index e25f1e96db..13070393ef 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -747,14 +747,37 @@ Object *object_new_with_class(ObjectClass *klass)
return object_new_with_type(klass->type);
}
-Object *object_new(const char *typename)
+
+Object *object_try_new(const char *typename, Error **errp)
{
TypeImpl *ti = type_get_by_name(typename);
+#ifdef CONFIG_MODULES
+ if (!ti) {
+ int rv = module_load_qom(typename, errp);
+ if (rv) {
+ error_prepend(errp, "could not find a module for type '%s': ",
+ typename);
+ return NULL;
+ }
+ ti = type_get_by_name(typename);
+ }
+#endif
+ if (!ti) {
+ error_setg(errp, "unknown type '%s'", typename);
+ return NULL;
+ }
+
return object_new_with_type(ti);
}
+Object *object_new(const char *typename)
+{
+ return object_try_new(typename, &error_fatal);
+}
+
+
Object *object_new_with_props(const char *typename,
Object *parent,
const char *id,
--
2.38.1
Hi Philippe, On 1/9/23 12:31, Philippe Mathieu-Daudé wrote: > The module resolving in qdev_new() is specific to QOM, not to > QDev. Extract the code as a new QOM object_try_new() helper so > it can be reused by non-QDev code. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > RFC because I'm wonder if we can't find a better name... > > Also, should we refactor object_initialize() similarly, > having object_try_initialize(..., Error *)? > --- > hw/core/qdev.c | 23 ++--------------------- > include/qom/object.h | 12 ++++++++++++ > qom/object.c | 25 ++++++++++++++++++++++++- > 3 files changed, 38 insertions(+), 22 deletions(-) > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index d759c4602c..3a076dcc7f 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -147,31 +147,12 @@ bool qdev_set_parent_bus(DeviceState *dev, BusState *bus, Error **errp) > > DeviceState *qdev_new(const char *name) > { > - ObjectClass *oc = object_class_by_name(name); > -#ifdef CONFIG_MODULES > - if (!oc) { > - int rv = module_load_qom(name, &error_fatal); > - if (rv > 0) { > - oc = object_class_by_name(name); > - } else { > - error_report("could not find a module for type '%s'", name); > - exit(1); > - } > - } > -#endif > - if (!oc) { > - error_report("unknown type '%s'", name); > - abort(); > - } > - return DEVICE(object_new(name)); > + return DEVICE(object_try_new(name, &error_fatal)); for clarity, why this last line change? The new object_new() implementation does exactly this, ie: { return object_try_new(typename, &error_fatal); } and from a reader perspective, when calling qdev_new() - as opposed to qdev_try_new() - we want the object to be created, there is no "try". It is not a functional thing, just something that would seem clearer to me. > } > > DeviceState *qdev_try_new(const char *name) > { > - if (!module_object_class_by_name(name)) { > - return NULL; > - } > - return DEVICE(object_new(name)); > + return DEVICE(object_try_new(name, NULL)); > } Ok. Here it make sense for me to see "try_new", as we are trying to create a qdev. > > static QTAILQ_HEAD(, DeviceListener) device_listeners > diff --git a/include/qom/object.h b/include/qom/object.h > index ef7258a5e1..27059cafb7 100644 > --- a/include/qom/object.h > +++ b/include/qom/object.h > @@ -565,6 +565,18 @@ Object *object_new_with_class(ObjectClass *klass); > */ > Object *object_new(const char *typename); > > +/** > + * object_try_new: Try to create an object on the heap > + * @typename: The name of the type of the object to instantiate. > + * @errp: pointer to Error object. > + * > + * This is like object_new(), except it returns %NULL when type @typename > + * does not exist, rather than asserting. > + * > + * Returns: The newly allocated and instantiated object, or %NULL. Does it make sense to warn or document that in order to make any error fatal, those special values can be passed? I know it is documented in include/qapi/error.h in pages of text, but from reading this function alone, one would think that try_new is never fatal, which in some way is true, but also isn't.. When is the actual error propagation and check for the special value error_fatal and error_abort happening? Is it worthwhile documenting it here, especially for someone investigating the use of qdev_new() ? > + */ > +Object *object_try_new(const char *typename, Error **errp); > + > /** > * object_new_with_props: > * @typename: The name of the type of the object to instantiate. > diff --git a/qom/object.c b/qom/object.c > index e25f1e96db..13070393ef 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -747,14 +747,37 @@ Object *object_new_with_class(ObjectClass *klass) > return object_new_with_type(klass->type); > } > > -Object *object_new(const char *typename) > + > +Object *object_try_new(const char *typename, Error **errp) > { > TypeImpl *ti = type_get_by_name(typename); > > +#ifdef CONFIG_MODULES > + if (!ti) { > + int rv = module_load_qom(typename, errp); > + if (rv) { > + error_prepend(errp, "could not find a module for type '%s': ", > + typename); > + return NULL; > + } > + ti = type_get_by_name(typename); > + } > +#endif > + if (!ti) { > + error_setg(errp, "unknown type '%s'", typename); > + return NULL; > + } > + > return object_new_with_type(ti); > } > > > +Object *object_new(const char *typename) > +{ > + return object_try_new(typename, &error_fatal); > +} > + > + > Object *object_new_with_props(const char *typename, > Object *parent, > const char *id,
© 2016 - 2024 Red Hat, Inc.