[v5,03/10] qdev: device module support
diff mbox series

Message ID 20200624131045.14512-4-kraxel@redhat.com
State New
Headers show
Series
  • build some devices as modules.
Related show

Commit Message

Gerd Hoffmann June 24, 2020, 1:10 p.m. UTC
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(-)

Comments

Christophe de Dinechin July 20, 2020, 2:25 p.m. UTC | #1
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)
Gerd Hoffmann July 21, 2020, 2:27 p.m. UTC | #2
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
Christophe de Dinechin July 22, 2020, 8:05 a.m. UTC | #3
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)
Gerd Hoffmann July 22, 2020, 11:05 a.m. UTC | #4
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
Christophe de Dinechin July 22, 2020, 2:39 p.m. UTC | #5
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)

Patch
diff mbox series

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 *