[PATCH v5 03/10] qdev: device module support

Gerd Hoffmann posted 10 patches 5 years, 7 months ago
[PATCH v5 03/10] qdev: device module support
Posted by Gerd Hoffmann 5 years, 7 months ago
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);
+    }
     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 *
-- 
2.18.4


Re: [PATCH v5 03/10] qdev: device module support
Posted by Christophe de Dinechin 5 years, 6 months ago
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)


Re: [PATCH v5 03/10] qdev: device module support
Posted by Gerd Hoffmann 5 years, 6 months ago
  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


Re: [PATCH v5 03/10] qdev: device module support
Posted by Christophe de Dinechin 5 years, 6 months ago
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)


Re: [PATCH v5 03/10] qdev: device module support
Posted by Gerd Hoffmann 5 years, 6 months ago
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


Re: [PATCH v5 03/10] qdev: device module support
Posted by Christophe de Dinechin 5 years, 6 months ago
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)