All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Improve module accelerator error message
@ 2021-07-20 22:31 Jose R. Ziviani
  2021-07-20 22:31 ` [PATCH 1/2] modules: Implement new helper functions Jose R. Ziviani
  2021-07-20 22:31 ` [PATCH 2/2] qom: Improve error message in module_object_class_by_name() Jose R. Ziviani
  0 siblings, 2 replies; 6+ messages in thread
From: Jose R. Ziviani @ 2021-07-20 22:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: berrange, ehabkost, Jose R. Ziviani, richard.henderson, cfontana,
	pbonzini, kraxel

The main objective here is to fix an user issue when trying to load TCG
that was built as module, but it's not installed or found in the library
path.

For example:

$ ./qemu-system-x86_64 -machine q35 -accel tcg -kernel /boot/vmlinuz
...
ERROR:../accel/accel-softmmu.c:82:accel_init_ops_interfaces: assertion failed: (ops != NULL)
Bail out! ERROR:../accel/accel-softmmu.c:82:accel_init_ops_interfaces: assertion failed: (ops != NULL)
[1]    31964 IOT instruction (core dumped)  ./qemu-system-x86_64 ...

The new error message becomes:

$ ./qemu-system-x86_64 -machine q35 -accel tcg -kernel /boot/vmlinuz
accel-tcg-x86_64 module is missing, install the package or config the library path correctly.

Jose R. Ziviani (2):
  modules: Implement new helper functions
  qom: Improve error message in module_object_class_by_name()

 accel/accel-softmmu.c |  5 +++-
 include/qemu/module.h |  4 +++
 qom/object.c          |  9 +++++++
 util/module.c         | 57 +++++++++++++++++++++++++++++++++++++------
 4 files changed, 67 insertions(+), 8 deletions(-)

-- 
2.32.0



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] modules: Implement new helper functions
  2021-07-20 22:31 [PATCH 0/2] Improve module accelerator error message Jose R. Ziviani
@ 2021-07-20 22:31 ` Jose R. Ziviani
  2021-07-20 22:31 ` [PATCH 2/2] qom: Improve error message in module_object_class_by_name() Jose R. Ziviani
  1 sibling, 0 replies; 6+ messages in thread
From: Jose R. Ziviani @ 2021-07-20 22:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: berrange, ehabkost, Jose R. Ziviani, richard.henderson, cfontana,
	pbonzini, kraxel

The function module_load_one() fills a hash table with modules that
were successfuly loaded. However, that table is a static variable of
module_load_one(). This patch changes it and creates a function that
informs whether a given module was loaded or not.

It also creates a function that returns the module name given the
typename.

Signed-off-by: Jose R. Ziviani <jziviani@suse.de>
---
 include/qemu/module.h |  4 +++
 util/module.c         | 57 +++++++++++++++++++++++++++++++++++++------
 2 files changed, 54 insertions(+), 7 deletions(-)

diff --git a/include/qemu/module.h b/include/qemu/module.h
index 3deac0078b..f45773718d 100644
--- a/include/qemu/module.h
+++ b/include/qemu/module.h
@@ -14,6 +14,7 @@
 #ifndef QEMU_MODULE_H
 #define QEMU_MODULE_H
 
+#include <stdbool.h>
 
 #define DSO_STAMP_FUN         glue(qemu_stamp, CONFIG_STAMP)
 #define DSO_STAMP_FUN_STR     stringify(DSO_STAMP_FUN)
@@ -74,6 +75,9 @@ void module_load_qom_one(const char *type);
 void module_load_qom_all(void);
 void module_allow_arch(const char *arch);
 
+bool module_is_loaded(const char *name);
+const char *module_get_name_from_obj(const char *obj);
+
 /**
  * DOC: module info annotation macros
  *
diff --git a/util/module.c b/util/module.c
index 6bb4ad915a..f23adc65bf 100644
--- a/util/module.c
+++ b/util/module.c
@@ -119,6 +119,8 @@ static const QemuModinfo module_info_stub[] = { {
 static const QemuModinfo *module_info = module_info_stub;
 static const char *module_arch;
 
+static GHashTable *loaded_modules;
+
 void module_init_info(const QemuModinfo *info)
 {
     module_info = info;
@@ -206,13 +208,10 @@ static int module_load_file(const char *fname, bool mayfail, bool export_symbols
 out:
     return ret;
 }
-#endif
 
 bool module_load_one(const char *prefix, const char *lib_name, bool mayfail)
 {
     bool success = false;
-
-#ifdef CONFIG_MODULES
     char *fname = NULL;
 #ifdef CONFIG_MODULE_UPGRADES
     char *version_dir;
@@ -223,7 +222,6 @@ bool module_load_one(const char *prefix, const char *lib_name, bool mayfail)
     int i = 0, n_dirs = 0;
     int ret;
     bool export_symbols = false;
-    static GHashTable *loaded_modules;
     const QemuModinfo *modinfo;
     const char **sl;
 
@@ -307,12 +305,9 @@ bool module_load_one(const char *prefix, const char *lib_name, bool mayfail)
         g_free(dirs[i]);
     }
 
-#endif
     return success;
 }
 
-#ifdef CONFIG_MODULES
-
 static bool module_loaded_qom_all;
 
 void module_load_qom_one(const char *type)
@@ -377,6 +372,39 @@ void qemu_load_module_for_opts(const char *group)
     }
 }
 
+bool module_is_loaded(const char *name)
+{
+    if (!loaded_modules || !g_hash_table_contains(loaded_modules, name)) {
+        return false;
+    }
+
+    return true;
+}
+
+const char *module_get_name_from_obj(const char *obj)
+{
+    const QemuModinfo *modinfo;
+    const char **sl;
+
+    if (!obj) {
+        return NULL;
+    }
+
+    for (modinfo = module_info; modinfo->name != NULL; modinfo++) {
+        if (!modinfo->objs) {
+            continue;
+        }
+
+        for (sl = modinfo->objs; *sl != NULL; sl++) {
+            if (strcmp(obj, *sl) == 0) {
+                return modinfo->name;
+            }
+        }
+    }
+
+    return NULL;
+}
+
 #else
 
 void module_allow_arch(const char *arch) {}
@@ -384,4 +412,19 @@ void qemu_load_module_for_opts(const char *group) {}
 void module_load_qom_one(const char *type) {}
 void module_load_qom_all(void) {}
 
+bool module_load_one(const char *prefix, const char *lib_name, bool mayfail)
+{
+    return false;
+}
+
+bool module_is_loaded(const char *name)
+{
+    return false;
+}
+
+char *module_get_name_from_obj(const char *obj)
+{
+    return NULL;
+}
+
 #endif
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] qom: Improve error message in module_object_class_by_name()
  2021-07-20 22:31 [PATCH 0/2] Improve module accelerator error message Jose R. Ziviani
  2021-07-20 22:31 ` [PATCH 1/2] modules: Implement new helper functions Jose R. Ziviani
@ 2021-07-20 22:31 ` Jose R. Ziviani
  2021-07-21  7:09   ` Claudio Fontana
  2021-07-21  9:34   ` Daniel P. Berrangé
  1 sibling, 2 replies; 6+ messages in thread
From: Jose R. Ziviani @ 2021-07-20 22:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: berrange, ehabkost, Jose R. Ziviani, richard.henderson, cfontana,
	pbonzini, kraxel

module_object_class_by_name() calls module_load_qom_one if the object
is provided by a dynamically linked library. Such library might not be
available at this moment - for instance, it can be a package not yet
installed. Thus, instead of assert error messages, this patch outputs
more friendly messages.

Current error messages:
$ ./qemu-system-x86_64 -machine q35 -accel tcg -kernel /boot/vmlinuz
...
ERROR:../accel/accel-softmmu.c:82:accel_init_ops_interfaces: assertion failed: (ops != NULL)
Bail out! ERROR:../accel/accel-softmmu.c:82:accel_init_ops_interfaces: assertion failed: (ops != NULL)
[1]    31964 IOT instruction (core dumped)  ./qemu-system-x86_64 ...

New error message:
$ ./qemu-system-x86_64 -machine q35 -accel tcg -kernel /boot/vmlinuz
accel-tcg-x86_64 module is missing, install the package or config the library path correctly.

Or with other modules, when possible:
$ ./qemu-system-x86_64 -machine q35 -accel kvm -kernel /boot/vmlinuz -vga qxl                                                                     ✹
hw-display-qxl module is missing, install the package or config the library path correctly.
qemu-system-x86_64: QXL VGA not available

$ make check
...
Running test qtest-x86_64/test-filter-mirror
Running test qtest-x86_64/endianness-test
accel-qtest-x86_64 module is missing, install the package or config the library path correctly.
accel-qtest-x86_64 module is missing, install the package or config the library path correctly.
accel-qtest-x86_64 module is missing, install the package or config the library path correctly.
accel-qtest-x86_64 module is missing, install the package or config the library path correctly.
accel-qtest-x86_64 module is missing, install the package or config the library path correctly.
accel-tcg-x86_64 module is missing, install the package or config the library path correctly.
...

Signed-off-by: Jose R. Ziviani <jziviani@suse.de>
---
 accel/accel-softmmu.c | 5 ++++-
 qom/object.c          | 9 +++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/accel/accel-softmmu.c b/accel/accel-softmmu.c
index 67276e4f52..52449ac2d0 100644
--- a/accel/accel-softmmu.c
+++ b/accel/accel-softmmu.c
@@ -79,7 +79,10 @@ void accel_init_ops_interfaces(AccelClass *ac)
      * all accelerators need to define ops, providing at least a mandatory
      * non-NULL create_vcpu_thread operation.
      */
-    g_assert(ops != NULL);
+    if (ops == NULL) {
+        exit(1);
+    }
+
     if (ops->ops_init) {
         ops->ops_init(ops);
     }
diff --git a/qom/object.c b/qom/object.c
index 6a01d56546..3a170ea9df 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -10,6 +10,7 @@
  * See the COPYING file in the top-level directory.
  */
 
+#include "qemu/module.h"
 #include "qemu/osdep.h"
 #include "hw/qdev-core.h"
 #include "qapi/error.h"
@@ -1031,8 +1032,16 @@ ObjectClass *module_object_class_by_name(const char *typename)
     oc = object_class_by_name(typename);
 #ifdef CONFIG_MODULES
     if (!oc) {
+        const char *module_name = module_get_name_from_obj(typename);
         module_load_qom_one(typename);
         oc = object_class_by_name(typename);
+        if (!oc && module_name) {
+            if (!module_is_loaded(module_name)) {
+                fprintf(stderr, "%s module is missing, install the "
+                                "package or config the library path "
+                                "correctly.\n", module_name);
+            }
+        }
     }
 #endif
     return oc;
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] qom: Improve error message in module_object_class_by_name()
  2021-07-20 22:31 ` [PATCH 2/2] qom: Improve error message in module_object_class_by_name() Jose R. Ziviani
@ 2021-07-21  7:09   ` Claudio Fontana
  2021-07-21  9:34   ` Daniel P. Berrangé
  1 sibling, 0 replies; 6+ messages in thread
From: Claudio Fontana @ 2021-07-21  7:09 UTC (permalink / raw)
  To: Jose R. Ziviani, qemu-devel
  Cc: pbonzini, richard.henderson, berrange, kraxel, ehabkost

On 7/21/21 12:31 AM, Jose R. Ziviani wrote:
> module_object_class_by_name() calls module_load_qom_one if the object
> is provided by a dynamically linked library. Such library might not be
> available at this moment - for instance, it can be a package not yet
> installed. Thus, instead of assert error messages, this patch outputs
> more friendly messages.
> 
> Current error messages:
> $ ./qemu-system-x86_64 -machine q35 -accel tcg -kernel /boot/vmlinuz
> ...
> ERROR:../accel/accel-softmmu.c:82:accel_init_ops_interfaces: assertion failed: (ops != NULL)
> Bail out! ERROR:../accel/accel-softmmu.c:82:accel_init_ops_interfaces: assertion failed: (ops != NULL)
> [1]    31964 IOT instruction (core dumped)  ./qemu-system-x86_64 ...
> 
> New error message:
> $ ./qemu-system-x86_64 -machine q35 -accel tcg -kernel /boot/vmlinuz
> accel-tcg-x86_64 module is missing, install the package or config the library path correctly.
> 
> Or with other modules, when possible:
> $ ./qemu-system-x86_64 -machine q35 -accel kvm -kernel /boot/vmlinuz -vga qxl                                                                     ✹
> hw-display-qxl module is missing, install the package or config the library path correctly.
> qemu-system-x86_64: QXL VGA not available
> 
> $ make check
> ...
> Running test qtest-x86_64/test-filter-mirror
> Running test qtest-x86_64/endianness-test
> accel-qtest-x86_64 module is missing, install the package or config the library path correctly.
> accel-qtest-x86_64 module is missing, install the package or config the library path correctly.
> accel-qtest-x86_64 module is missing, install the package or config the library path correctly.
> accel-qtest-x86_64 module is missing, install the package or config the library path correctly.
> accel-qtest-x86_64 module is missing, install the package or config the library path correctly.
> accel-tcg-x86_64 module is missing, install the package or config the library path correctly.
> ...
> 
> Signed-off-by: Jose R. Ziviani <jziviani@suse.de>
> ---
>  accel/accel-softmmu.c | 5 ++++-
>  qom/object.c          | 9 +++++++++
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/accel/accel-softmmu.c b/accel/accel-softmmu.c
> index 67276e4f52..52449ac2d0 100644
> --- a/accel/accel-softmmu.c
> +++ b/accel/accel-softmmu.c
> @@ -79,7 +79,10 @@ void accel_init_ops_interfaces(AccelClass *ac)
>       * all accelerators need to define ops, providing at least a mandatory
>       * non-NULL create_vcpu_thread operation.
>       */
> -    g_assert(ops != NULL);
> +    if (ops == NULL) {
> +        exit(1);
> +    }
> +

Why?

>      if (ops->ops_init) {
>          ops->ops_init(ops);
>      }
> diff --git a/qom/object.c b/qom/object.c
> index 6a01d56546..3a170ea9df 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -10,6 +10,7 @@
>   * See the COPYING file in the top-level directory.
>   */
>  
> +#include "qemu/module.h"
>  #include "qemu/osdep.h"
>  #include "hw/qdev-core.h"
>  #include "qapi/error.h"
> @@ -1031,8 +1032,16 @@ ObjectClass *module_object_class_by_name(const char *typename)
>      oc = object_class_by_name(typename);
>  #ifdef CONFIG_MODULES
>      if (!oc) {
> +        const char *module_name = module_get_name_from_obj(typename);
>          module_load_qom_one(typename);
>          oc = object_class_by_name(typename);
> +        if (!oc && module_name) {
> +            if (!module_is_loaded(module_name)) {
> +                fprintf(stderr, "%s module is missing, install the "
> +                                "package or config the library path "
> +                                "correctly.\n", module_name);
> +            }
> +        }
>      }
>  #endif
>      return oc;
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] qom: Improve error message in module_object_class_by_name()
  2021-07-20 22:31 ` [PATCH 2/2] qom: Improve error message in module_object_class_by_name() Jose R. Ziviani
  2021-07-21  7:09   ` Claudio Fontana
@ 2021-07-21  9:34   ` Daniel P. Berrangé
  2021-07-21  9:57     ` Claudio Fontana
  1 sibling, 1 reply; 6+ messages in thread
From: Daniel P. Berrangé @ 2021-07-21  9:34 UTC (permalink / raw)
  To: Jose R. Ziviani
  Cc: ehabkost, richard.henderson, qemu-devel, cfontana, pbonzini, kraxel

On Tue, Jul 20, 2021 at 07:31:20PM -0300, Jose R. Ziviani wrote:
> module_object_class_by_name() calls module_load_qom_one if the object
> is provided by a dynamically linked library. Such library might not be
> available at this moment - for instance, it can be a package not yet
> installed. Thus, instead of assert error messages, this patch outputs
> more friendly messages.
> 
> Current error messages:
> $ ./qemu-system-x86_64 -machine q35 -accel tcg -kernel /boot/vmlinuz
> ...
> ERROR:../accel/accel-softmmu.c:82:accel_init_ops_interfaces: assertion failed: (ops != NULL)
> Bail out! ERROR:../accel/accel-softmmu.c:82:accel_init_ops_interfaces: assertion failed: (ops != NULL)
> [1]    31964 IOT instruction (core dumped)  ./qemu-system-x86_64 ...
> 
> New error message:
> $ ./qemu-system-x86_64 -machine q35 -accel tcg -kernel /boot/vmlinuz
> accel-tcg-x86_64 module is missing, install the package or config the library path correctly.
> 
> Or with other modules, when possible:
> $ ./qemu-system-x86_64 -machine q35 -accel kvm -kernel /boot/vmlinuz -vga qxl                                                                     ✹
> hw-display-qxl module is missing, install the package or config the library path correctly.
> qemu-system-x86_64: QXL VGA not available
> 
> $ make check
> ...
> Running test qtest-x86_64/test-filter-mirror
> Running test qtest-x86_64/endianness-test
> accel-qtest-x86_64 module is missing, install the package or config the library path correctly.
> accel-qtest-x86_64 module is missing, install the package or config the library path correctly.
> accel-qtest-x86_64 module is missing, install the package or config the library path correctly.
> accel-qtest-x86_64 module is missing, install the package or config the library path correctly.
> accel-qtest-x86_64 module is missing, install the package or config the library path correctly.
> accel-tcg-x86_64 module is missing, install the package or config the library path correctly.
> ...
> 
> Signed-off-by: Jose R. Ziviani <jziviani@suse.de>
> ---
>  accel/accel-softmmu.c | 5 ++++-
>  qom/object.c          | 9 +++++++++
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/accel/accel-softmmu.c b/accel/accel-softmmu.c
> index 67276e4f52..52449ac2d0 100644
> --- a/accel/accel-softmmu.c
> +++ b/accel/accel-softmmu.c
> @@ -79,7 +79,10 @@ void accel_init_ops_interfaces(AccelClass *ac)
>       * all accelerators need to define ops, providing at least a mandatory
>       * non-NULL create_vcpu_thread operation.
>       */
> -    g_assert(ops != NULL);
> +    if (ops == NULL) {
> +        exit(1);
> +    }
> +
>      if (ops->ops_init) {
>          ops->ops_init(ops);
>      }
> diff --git a/qom/object.c b/qom/object.c
> index 6a01d56546..3a170ea9df 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -10,6 +10,7 @@
>   * See the COPYING file in the top-level directory.
>   */
>  
> +#include "qemu/module.h"
>  #include "qemu/osdep.h"
>  #include "hw/qdev-core.h"
>  #include "qapi/error.h"
> @@ -1031,8 +1032,16 @@ ObjectClass *module_object_class_by_name(const char *typename)
>      oc = object_class_by_name(typename);
>  #ifdef CONFIG_MODULES
>      if (!oc) {
> +        const char *module_name = module_get_name_from_obj(typename);
>          module_load_qom_one(typename);
>          oc = object_class_by_name(typename);
> +        if (!oc && module_name) {
> +            if (!module_is_loaded(module_name)) {
> +                fprintf(stderr, "%s module is missing, install the "
> +                                "package or config the library path "
> +                                "correctly.\n", module_name);
> +            }
> +        }
>      }

Introducing a call to module_is_loaded is feels like it isn't really
addressing the root cause.

The error scenario here exists because module_load_qom_one() can
fail to load the requested module. We don't detect that failure
because module_load_qom_one() is "void" for some reason. There
are several possible causes of failure and missing file on disk
is only one of them.

This general error message about "module_name" being missing is
misleading when there are dependancies involved. "module_name"
may well exist, but something it depends on might be missing.
Or the modules might have been built from wrong QEMU and be
unable to be dlopened despite existing.

I think we need to fix module_load_qom_one() so that it can
actually report failure via an "Error **errp" parameter.
This will also entail fixing a number of methods it in turn
calls.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] qom: Improve error message in module_object_class_by_name()
  2021-07-21  9:34   ` Daniel P. Berrangé
@ 2021-07-21  9:57     ` Claudio Fontana
  0 siblings, 0 replies; 6+ messages in thread
From: Claudio Fontana @ 2021-07-21  9:57 UTC (permalink / raw)
  To: Daniel P. Berrangé, Jose R. Ziviani
  Cc: pbonzini, richard.henderson, qemu-devel, ehabkost, kraxel

On 7/21/21 11:34 AM, Daniel P. Berrangé wrote:
> On Tue, Jul 20, 2021 at 07:31:20PM -0300, Jose R. Ziviani wrote:
>> module_object_class_by_name() calls module_load_qom_one if the object
>> is provided by a dynamically linked library. Such library might not be
>> available at this moment - for instance, it can be a package not yet
>> installed. Thus, instead of assert error messages, this patch outputs
>> more friendly messages.
>>
>> Current error messages:
>> $ ./qemu-system-x86_64 -machine q35 -accel tcg -kernel /boot/vmlinuz
>> ...
>> ERROR:../accel/accel-softmmu.c:82:accel_init_ops_interfaces: assertion failed: (ops != NULL)
>> Bail out! ERROR:../accel/accel-softmmu.c:82:accel_init_ops_interfaces: assertion failed: (ops != NULL)
>> [1]    31964 IOT instruction (core dumped)  ./qemu-system-x86_64 ...
>>
>> New error message:
>> $ ./qemu-system-x86_64 -machine q35 -accel tcg -kernel /boot/vmlinuz
>> accel-tcg-x86_64 module is missing, install the package or config the library path correctly.
>>
>> Or with other modules, when possible:
>> $ ./qemu-system-x86_64 -machine q35 -accel kvm -kernel /boot/vmlinuz -vga qxl                                                                     ✹
>> hw-display-qxl module is missing, install the package or config the library path correctly.
>> qemu-system-x86_64: QXL VGA not available
>>
>> $ make check
>> ...
>> Running test qtest-x86_64/test-filter-mirror
>> Running test qtest-x86_64/endianness-test
>> accel-qtest-x86_64 module is missing, install the package or config the library path correctly.
>> accel-qtest-x86_64 module is missing, install the package or config the library path correctly.
>> accel-qtest-x86_64 module is missing, install the package or config the library path correctly.
>> accel-qtest-x86_64 module is missing, install the package or config the library path correctly.
>> accel-qtest-x86_64 module is missing, install the package or config the library path correctly.
>> accel-tcg-x86_64 module is missing, install the package or config the library path correctly.
>> ...
>>
>> Signed-off-by: Jose R. Ziviani <jziviani@suse.de>
>> ---
>>  accel/accel-softmmu.c | 5 ++++-
>>  qom/object.c          | 9 +++++++++
>>  2 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/accel/accel-softmmu.c b/accel/accel-softmmu.c
>> index 67276e4f52..52449ac2d0 100644
>> --- a/accel/accel-softmmu.c
>> +++ b/accel/accel-softmmu.c
>> @@ -79,7 +79,10 @@ void accel_init_ops_interfaces(AccelClass *ac)
>>       * all accelerators need to define ops, providing at least a mandatory
>>       * non-NULL create_vcpu_thread operation.
>>       */
>> -    g_assert(ops != NULL);
>> +    if (ops == NULL) {
>> +        exit(1);
>> +    }
>> +
>>      if (ops->ops_init) {
>>          ops->ops_init(ops);
>>      }
>> diff --git a/qom/object.c b/qom/object.c
>> index 6a01d56546..3a170ea9df 100644
>> --- a/qom/object.c
>> +++ b/qom/object.c
>> @@ -10,6 +10,7 @@
>>   * See the COPYING file in the top-level directory.
>>   */
>>  
>> +#include "qemu/module.h"
>>  #include "qemu/osdep.h"
>>  #include "hw/qdev-core.h"
>>  #include "qapi/error.h"
>> @@ -1031,8 +1032,16 @@ ObjectClass *module_object_class_by_name(const char *typename)
>>      oc = object_class_by_name(typename);
>>  #ifdef CONFIG_MODULES
>>      if (!oc) {
>> +        const char *module_name = module_get_name_from_obj(typename);
>>          module_load_qom_one(typename);
>>          oc = object_class_by_name(typename);
>> +        if (!oc && module_name) {
>> +            if (!module_is_loaded(module_name)) {
>> +                fprintf(stderr, "%s module is missing, install the "
>> +                                "package or config the library path "
>> +                                "correctly.\n", module_name);
>> +            }
>> +        }
>>      }
> 
> Introducing a call to module_is_loaded is feels like it isn't really
> addressing the root cause.
> 
> The error scenario here exists because module_load_qom_one() can
> fail to load the requested module. We don't detect that failure
> because module_load_qom_one() is "void" for some reason. There
> are several possible causes of failure and missing file on disk
> is only one of them.
> 
> This general error message about "module_name" being missing is
> misleading when there are dependancies involved. "module_name"
> may well exist, but something it depends on might be missing.
> Or the modules might have been built from wrong QEMU and be
> unable to be dlopened despite existing.
> 
> I think we need to fix module_load_qom_one() so that it can
> actually report failure via an "Error **errp" parameter.

Sounds right to me.

> This will also entail fixing a number of methods it in turn
> calls.
> 
> 
> Regards,
> Daniel
> 

Thanks,

Claudio


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-07-21 10:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-20 22:31 [PATCH 0/2] Improve module accelerator error message Jose R. Ziviani
2021-07-20 22:31 ` [PATCH 1/2] modules: Implement new helper functions Jose R. Ziviani
2021-07-20 22:31 ` [PATCH 2/2] qom: Improve error message in module_object_class_by_name() Jose R. Ziviani
2021-07-21  7:09   ` Claudio Fontana
2021-07-21  9:34   ` Daniel P. Berrangé
2021-07-21  9:57     ` Claudio Fontana

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.