| Message ID | 20200624131045.14512-4-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... > Hook module loading into the places where we > need it when building devices as modules. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > hw/core/qdev.c | 6 ++++-- > qdev-monitor.c | 5 +++-- > qom/qom-qmp-cmds.c | 3 ++- > softmmu/vl.c | 4 ++-- > 4 files changed, 11 insertions(+), 7 deletions(-) > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index 2131c7f951dd..9de16eae05b7 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -137,6 +137,9 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus) > */ > DeviceState *qdev_new(const char *name) > { > + if (!object_class_by_name(name)) { > + module_load_qom_one(name); > + } Curious why you don't you call module_object_class_by_name here? > return DEVICE(object_new(name)); > } > > @@ -147,10 +150,9 @@ DeviceState *qdev_new(const char *name) > */ > DeviceState *qdev_try_new(const char *name) > { > - if (!object_class_by_name(name)) { > + if (!module_object_class_by_name(name)) { > return NULL; > } > - > return DEVICE(object_new(name)); > } > > diff --git a/qdev-monitor.c b/qdev-monitor.c > index 22da107484c5..8e7a7f7bbdbc 100644 > --- a/qdev-monitor.c > +++ b/qdev-monitor.c > @@ -147,6 +147,7 @@ static void qdev_print_devinfos(bool show_no_user) > int i; > bool cat_printed; > > + module_load_qom_all(); > list = object_class_get_list_sorted(TYPE_DEVICE, false); > > for (i = 0; i <= DEVICE_CATEGORY_MAX; i++) { > @@ -215,13 +216,13 @@ static DeviceClass *qdev_get_device_class(const char **driver, Error **errp) > DeviceClass *dc; > const char *original_name = *driver; > > - oc = object_class_by_name(*driver); > + oc = module_object_class_by_name(*driver); > if (!oc) { > const char *typename = find_typename_by_alias(*driver); > > if (typename) { > *driver = typename; > - oc = object_class_by_name(*driver); > + oc = module_object_class_by_name(*driver); > } > } > > diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c > index c5249e44d020..5e2c8cbf333f 100644 > --- a/qom/qom-qmp-cmds.c > +++ b/qom/qom-qmp-cmds.c > @@ -116,6 +116,7 @@ ObjectTypeInfoList *qmp_qom_list_types(bool has_implements, > { > ObjectTypeInfoList *ret = NULL; > > + module_load_qom_all(); > object_class_foreach(qom_list_types_tramp, implements, abstract, &ret); > > return ret; > @@ -130,7 +131,7 @@ ObjectPropertyInfoList *qmp_device_list_properties(const char *typename, > ObjectPropertyIterator iter; > ObjectPropertyInfoList *prop_list = NULL; > > - klass = object_class_by_name(typename); > + klass = module_object_class_by_name(typename); > if (klass == NULL) { > error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > "Device '%s' not found", typename); > diff --git a/softmmu/vl.c b/softmmu/vl.c > index f669c06ede4a..1297815a5f5b 100644 > --- a/softmmu/vl.c > +++ b/softmmu/vl.c > @@ -1772,8 +1772,8 @@ static bool vga_interface_available(VGAInterfaceType t) > > assert(t < VGA_TYPE_MAX); > return !ti->class_names[0] || > - object_class_by_name(ti->class_names[0]) || > - object_class_by_name(ti->class_names[1]); > + module_object_class_by_name(ti->class_names[0]) || > + module_object_class_by_name(ti->class_names[1]); > } > > static const char * -- Cheers, Christophe de Dinechin (IRC c3d)
Hi, > > DeviceState *qdev_new(const char *name) > > { > > + if (!object_class_by_name(name)) { > > + module_load_qom_one(name); > > + } > > Curious why you don't you call module_object_class_by_name here? Because object_new() wants a name not an ObjectClass ... > > return DEVICE(object_new(name)); > > } take care, Gerd
On 2020-07-21 at 16:27 CEST, Gerd Hoffmann wrote... > Hi, > >> > DeviceState *qdev_new(const char *name) >> > { >> > + if (!object_class_by_name(name)) { >> > + module_load_qom_one(name); >> > + } >> >> Curious why you don't you call module_object_class_by_name here? > > Because object_new() wants a name not an ObjectClass ... I'm talking about the two lines above. if (!object_class_by_name(name)) { module_load_qom_one(name); } Thi9s code looks very similar to the code below: 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; } Both call module_load_qom_one and object_class_by_name using the name as input, so I don't see the difference (except for the order). Am I reading this wrong? > >> > return DEVICE(object_new(name)); >> > } > > take care, > Gerd -- Cheers, Christophe de Dinechin (IRC c3d)
On Wed, Jul 22, 2020 at 10:05:51AM +0200, Christophe de Dinechin wrote: > > On 2020-07-21 at 16:27 CEST, Gerd Hoffmann wrote... > > Hi, > > > >> > DeviceState *qdev_new(const char *name) > >> > { > >> > + if (!object_class_by_name(name)) { > >> > + module_load_qom_one(name); > >> > + } > >> > >> Curious why you don't you call module_object_class_by_name here? > > > > Because object_new() wants a name not an ObjectClass ... > > I'm talking about the two lines above. > > if (!object_class_by_name(name)) { > module_load_qom_one(name); > } > > Thi9s code looks very similar to the code below: > > 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; > } > > Both call module_load_qom_one and object_class_by_name using the name as > input, so I don't see the difference (except for the order). Yes, calling module_object_class_by_name then throw away the result would work too. I don't like the idea to hide the module loading though. take care, Gerd
On 2020-07-22 at 13:05 CEST, Gerd Hoffmann wrote... > On Wed, Jul 22, 2020 at 10:05:51AM +0200, Christophe de Dinechin wrote: >> >> On 2020-07-21 at 16:27 CEST, Gerd Hoffmann wrote... >> > Hi, >> > >> >> > DeviceState *qdev_new(const char *name) >> >> > { >> >> > + if (!object_class_by_name(name)) { >> >> > + module_load_qom_one(name); >> >> > + } >> >> >> >> Curious why you don't you call module_object_class_by_name here? >> > >> > Because object_new() wants a name not an ObjectClass ... >> >> I'm talking about the two lines above. >> >> if (!object_class_by_name(name)) { >> module_load_qom_one(name); >> } >> >> Thi9s code looks very similar to the code below: >> >> 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; >> } >> >> Both call module_load_qom_one and object_class_by_name using the name as >> input, so I don't see the difference (except for the order). > > Yes, calling module_object_class_by_name then throw away the result > would work too. I don't like the idea to hide the module loading > though. Why do you consider calling a function called "module_object_class_by_name" as hiding the module loading? More importantly, why is it better to have two ways to do the same thing that are slightly different? The reason for the slight difference is really unclear to me. If we later do a change to module_object_class_by_name, are there cases where we won't need the same change in qdev_new? > > take care, > Gerd -- Cheers, Christophe de Dinechin (IRC c3d)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 2131c7f951dd..9de16eae05b7 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -137,6 +137,9 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus) */ DeviceState *qdev_new(const char *name) { + if (!object_class_by_name(name)) { + module_load_qom_one(name); + } return DEVICE(object_new(name)); } @@ -147,10 +150,9 @@ DeviceState *qdev_new(const char *name) */ DeviceState *qdev_try_new(const char *name) { - if (!object_class_by_name(name)) { + if (!module_object_class_by_name(name)) { return NULL; } - return DEVICE(object_new(name)); } diff --git a/qdev-monitor.c b/qdev-monitor.c index 22da107484c5..8e7a7f7bbdbc 100644 --- a/qdev-monitor.c +++ b/qdev-monitor.c @@ -147,6 +147,7 @@ static void qdev_print_devinfos(bool show_no_user) int i; bool cat_printed; + module_load_qom_all(); list = object_class_get_list_sorted(TYPE_DEVICE, false); for (i = 0; i <= DEVICE_CATEGORY_MAX; i++) { @@ -215,13 +216,13 @@ static DeviceClass *qdev_get_device_class(const char **driver, Error **errp) DeviceClass *dc; const char *original_name = *driver; - oc = object_class_by_name(*driver); + oc = module_object_class_by_name(*driver); if (!oc) { const char *typename = find_typename_by_alias(*driver); if (typename) { *driver = typename; - oc = object_class_by_name(*driver); + oc = module_object_class_by_name(*driver); } } diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c index c5249e44d020..5e2c8cbf333f 100644 --- a/qom/qom-qmp-cmds.c +++ b/qom/qom-qmp-cmds.c @@ -116,6 +116,7 @@ ObjectTypeInfoList *qmp_qom_list_types(bool has_implements, { ObjectTypeInfoList *ret = NULL; + module_load_qom_all(); object_class_foreach(qom_list_types_tramp, implements, abstract, &ret); return ret; @@ -130,7 +131,7 @@ ObjectPropertyInfoList *qmp_device_list_properties(const char *typename, ObjectPropertyIterator iter; ObjectPropertyInfoList *prop_list = NULL; - klass = object_class_by_name(typename); + klass = module_object_class_by_name(typename); if (klass == NULL) { error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, "Device '%s' not found", typename); diff --git a/softmmu/vl.c b/softmmu/vl.c index f669c06ede4a..1297815a5f5b 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -1772,8 +1772,8 @@ static bool vga_interface_available(VGAInterfaceType t) assert(t < VGA_TYPE_MAX); return !ti->class_names[0] || - object_class_by_name(ti->class_names[0]) || - object_class_by_name(ti->class_names[1]); + module_object_class_by_name(ti->class_names[0]) || + module_object_class_by_name(ti->class_names[1]); } static const char *
Hook module loading into the places where we need it when building devices as modules. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- hw/core/qdev.c | 6 ++++-- qdev-monitor.c | 5 +++-- qom/qom-qmp-cmds.c | 3 ++- softmmu/vl.c | 4 ++-- 4 files changed, 11 insertions(+), 7 deletions(-)